[JSC] Move the CheckTierUp function calls out of the main path
Created attachment 278820 [details] Patch
Conf#1 Conf#2 3d-cube 4.9750+-0.0183 ^ 4.9216+-0.0172 ^ definitely 1.0109x faster 3d-morph 5.0505+-0.0241 ? 5.0733+-0.0230 ? 3d-raytrace 5.3486+-0.0268 5.3325+-0.0193 access-binary-trees 2.1137+-0.0118 ? 2.1173+-0.0128 ? access-fannkuch 5.8436+-0.0222 ! 5.8900+-0.0228 ! definitely 1.0079x slower access-nbody 2.5166+-0.0112 2.5166+-0.0113 access-nsieve 2.9611+-0.0119 2.9548+-0.0111 bitops-3bit-bits-in-byte 1.1139+-0.0057 ? 1.1150+-0.0052 ? bitops-bits-in-byte 2.7480+-0.0127 ^ 2.5887+-0.0131 ^ definitely 1.0615x faster bitops-bitwise-and 1.9873+-0.0075 ? 1.9899+-0.0084 ? bitops-nsieve-bits 3.0469+-0.0113 ! 3.0874+-0.0177 ! definitely 1.0133x slower controlflow-recursive 2.3395+-0.0124 2.3363+-0.0104 crypto-aes 4.3709+-0.0236 ? 4.3782+-0.0252 ? crypto-md5 2.4169+-0.0105 2.4102+-0.0120 crypto-sha1 2.2562+-0.0137 ? 2.2636+-0.0145 ? date-format-tofte 6.5461+-0.0253 6.5453+-0.0264 date-format-xparb 4.8131+-0.0263 4.7939+-0.0233 math-cordic 2.8892+-0.0084 ^ 2.8247+-0.0128 ^ definitely 1.0228x faster math-partial-sums 3.7110+-0.0206 ? 3.7177+-0.0236 ? math-spectral-norm 1.9969+-0.0120 1.9814+-0.0069 regexp-dna 6.5614+-0.0364 6.5493+-0.0349 string-base64 4.2191+-0.0257 ? 4.2558+-0.0311 ? string-fasta 5.6710+-0.0185 5.6655+-0.0172 string-tagcloud 8.5184+-0.0336 ? 8.5624+-0.0362 ? string-unpack-code 18.8462+-0.0945 ? 18.9783+-0.0952 ? string-validate-input 4.1754+-0.0199 4.1531+-0.0188 <arithmetic> 4.5014+-0.0053 4.5001+-0.0054 might be 1.0003x faster
Comment on attachment 278820 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278820&action=review > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:5249 > - m_jit.setupArgumentsWithExecState(TrustedImm32(bytecodeIndex)); > - appendCallSetResult(triggerOSREntryNow, tempGPR); > - MacroAssembler::Jump dontEnter = m_jit.branchTestPtr(MacroAssembler::Zero, tempGPR); > - m_jit.emitRestoreCalleeSaves(); > - m_jit.jump(tempGPR); > - dontEnter.link(&m_jit); > - silentFillAllRegisters(tempGPR); > - > - done.link(&m_jit); > + > + addSlowPathGenerator([=]() { > + forceOSREntry.link(&m_jit); > + overflowedCounter.link(&m_jit); > + > + silentSpill(savePlans); > + m_jit.setupArgumentsWithExecState(TrustedImm32(bytecodeIndex)); > + appendCallSetResult(triggerOSREntryNow, tempGPR); > + > + if (savePlans.isEmpty()) > + m_jit.branchTestPtr(MacroAssembler::Zero, tempGPR).linkTo(toNextOperation, &m_jit); > + else { > + MacroAssembler::Jump osrEnter = m_jit.branchTestPtr(MacroAssembler::NonZero, tempGPR); > + silentFill(savePlans); > + osrEnter.link(&m_jit); > + } > + m_jit.emitRestoreCalleeSaves(); > + m_jit.jump(tempGPR); > + }); This does not look right. In the old code, of tempGPR is Zero, we do not enter. In the new code, I don't see a path to not enter. I think you're missing a jump to toNextOperation after the silentFill.
Created attachment 278846 [details] Patch
Comment on attachment 278846 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=278846&action=review > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:5246 > + MacroAssembler::Jump osrEnter = m_jit.branchTestPtr(MacroAssembler::NonZero, tempGPR); > + silentFill(savePlans); > + osrEnter.link(&m_jit); > + m_jit.jump().linkTo(toNextOperation, &m_jit); Still wrong. The jump needs to be before the osrEnter label linkage.
Created attachment 278856 [details] Patch
Comment on attachment 278856 [details] Patch r=me
Comment on attachment 278856 [details] Patch Clearing flags on attachment: 278856 Committed r200897: <http://trac.webkit.org/changeset/200897>
All reviewed patches have been landed. Closing bug.