[JSC] Merge op_check_traps into op_enter and op_loop_hint
Created attachment 377782 [details] Patch
Created attachment 377785 [details] Patch
Comment on attachment 377785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377785&action=review > Source/JavaScriptCore/jit/JITOpcodes.cpp:1034 > +#if ENABLE(DFG_JIT) > + // Emit the JIT optimization check: > if (canBeOptimized()) { > addSlowCase(branchAdd32(PositiveOrZero, TrustedImm32(Options::executionCounterIncrementForLoop()), > AbsoluteAddress(m_codeBlock->addressOfJITExecuteCounter()))); > } > +#endif > + // Check traps. > + addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_vm->needTrapHandlingAddress()))); I think we want to check traps before we check for optimization. This is because if we OSR exit due to a trap, we want to handle the trap first. Otherwise, there's a chance we'll be caught in a optimize + OSR exit loop. In practice, this might not happen, but I think it is more correct to check traps first. > Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:1019 > + emitEnterOptimizationCheck(); > + > + // Check traps. > + addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_vm->needTrapHandlingAddress()))); Ditto here. Let's check traps first before checking for optimizing. > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1678 > checkSwitchToJITForLoop() > - dispatch() > -end) > - > - > -llintOp(op_check_traps, OpCheckTraps, macro (unused, unused, dispatch) > + # CheckTraps. > loadp CodeBlock[cfr], t1 > loadp CodeBlock::m_vm[t1], t1 > - loadb VM::m_traps+VMTraps::m_needTrapHandling[t1], t0 > - btpnz t0, .handleTraps > + btpnz VM::m_traps + VMTraps::m_needTrapHandling[t1], .handleTraps Ditto here. Let's check traps first. This will be meaningful if we do OSR exit to LLInt later.
Comment on attachment 377785 [details] Patch Attachment 377785 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12986566 Number of test failures exceeded the failure limit.
Created attachment 377786 [details] Archive of layout-test-results from ews212 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews212 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 377785 [details] Patch Attachment 377785 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/12986568 New failing tests: stress/regress-189227-watchdog-on-infinite-loop.js.dfg-eager-no-cjit-validate stress/call-link-info-osrexit-repatch.js.ftl-eager stress/regress-189227-watchdog-on-infinite-loop.js.eager-jettison-no-cjit stress/regress-189227-watchdog-on-infinite-loop.js.ftl-no-cjit-validate-sampling-profiler stress/regress-189227-watchdog-on-infinite-loop.js.ftl-eager-no-cjit stress/regress-189227-watchdog-on-infinite-loop.js.ftl-no-cjit-b3o0 stress/regress-189227-watchdog-on-infinite-loop.js.ftl-eager-no-cjit-b3o1 stress/regress-189227-watchdog-on-infinite-loop.js.no-cjit-collect-continuously stress/regress-189227-watchdog-on-infinite-loop.js.no-cjit-validate-phases stress/regress-189227-watchdog-on-infinite-loop.js.ftl-no-cjit-no-inline-validate stress/regress-189227-watchdog-on-infinite-loop.js.ftl-no-cjit-no-put-stack-validate
Comment on attachment 377785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377785&action=review >> Source/JavaScriptCore/jit/JITOpcodes.cpp:1034 >> + addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_vm->needTrapHandlingAddress()))); > > I think we want to check traps before we check for optimization. This is because if we OSR exit due to a trap, we want to handle the trap first. Otherwise, there's a chance we'll be caught in a optimize + OSR exit loop. In practice, this might not happen, but I think it is more correct to check traps first. OK, fixed. >> Source/JavaScriptCore/jit/JITOpcodes32_64.cpp:1019 >> + addSlowCase(branchTest8(NonZero, AbsoluteAddress(m_vm->needTrapHandlingAddress()))); > > Ditto here. Let's check traps first before checking for optimizing. Fixed. >> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1678 >> + btpnz VM::m_traps + VMTraps::m_needTrapHandling[t1], .handleTraps > > Ditto here. Let's check traps first. This will be meaningful if we do OSR exit to LLInt later. Fixed.
Created attachment 377798 [details] Patch
Created attachment 377799 [details] Patch
Comment on attachment 377799 [details] Patch Attachment 377799 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12987339 Number of test failures exceeded the failure limit.
Created attachment 377801 [details] Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 377799 [details] Patch Attachment 377799 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/12987334 New failing tests: stress/call-link-info-osrexit-repatch.js.ftl-eager
Created attachment 377809 [details] Patch
Comment on attachment 377809 [details] Patch Attachment 377809 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/12988577 New failing tests: stress/call-link-info-osrexit-repatch.js.ftl-eager
Created attachment 377812 [details] Patch
Created attachment 377816 [details] Patch
Comment on attachment 377816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377816&action=review Nice work. r=me > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:727 > + callTrapHandler(.throwHandler) Why not also callTrapHandler(_llint_throw_from_slow_path_trampoline) here like you did in llintOp(op_loop_hint)? > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:730 > +.throwHandler: > + jmp _llint_throw_from_slow_path_trampoline Not needed if you callTrapHandler(_llint_throw_from_slow_path_trampoline) above.
Comment on attachment 377816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377816&action=review Thanks! >> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:727 >> + callTrapHandler(.throwHandler) > > Why not also callTrapHandler(_llint_throw_from_slow_path_trampoline) here like you did in llintOp(op_loop_hint)? Right. Fixed.
Created attachment 377830 [details] Patch for landing
Committed r249372: <https://trac.webkit.org/changeset/249372>
<rdar://problem/54939747>
I'll roll-out this patch for now since JetStream2/Basic shows ~10% regression. My guess is that this regression is not inherently related to this patch: changing size of bytecodes and hitting some spot. But I'm not sure, I'll revisit this after generator optimization is done.
Committed r249523: <https://trac.webkit.org/changeset/249523>