And replaced by inlining of finally blocks wherever we would have done jsr. That makes the code easier to compile, should have no effect on performance or memory usage, and should allow us to kill off a lot of code. Not to mention, it will preempt any bugs from OSR during execution of a finally block. LLInt uses the virtual PC for the subroutine return, while the JIT uses the machine PC.
Created attachment 135240 [details] the patch
Comment on attachment 135240 [details] the patch Reviewed by Geoff in person.
Created attachment 135495 [details] the patch The previous patch failed to save/restore the correct state for finally inlining. For example: try { switch (foo) { case blah: return bar; } } finally { break; } Since the finally block was emitted inline in the return, the break would break us out of the switch. Other similarly horrible things would happen with scope depth and the finally depth, etc.
Comment on attachment 135495 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=135495&action=review emitComplexJumpScopes is maybe not my favorite function. r=me > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2177 > + while (m_labelScopes.size() > finallyContext.labelScopesSize) > + m_labelScopes.removeLast(); m_labelScopes.shrink(finallyContext.labelScopesSize) should suffice here.
(In reply to comment #4) > (From update of attachment 135495 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135495&action=review > > emitComplexJumpScopes is maybe not my favorite function. > > r=me > > > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2177 > > + while (m_labelScopes.size() > finallyContext.labelScopesSize) > > + m_labelScopes.removeLast(); > > m_labelScopes.shrink(finallyContext.labelScopesSize) should suffice here. SegmentedVector is shrinkless!
Landed in http://trac.webkit.org/changeset/113136
This commit gives >4% penalty on sunspider for me, tested on QNX without DFG turned on. Any idea?
(In reply to comment #7) > This commit gives >4% penalty on sunspider for me, tested on QNX without DFG turned on. Any idea? I don't remember this being a regression. I will retest.
(In reply to comment #8) > (In reply to comment #7) > > This commit gives >4% penalty on sunspider for me, tested on QNX without DFG turned on. Any idea? > > I don't remember this being a regression. I will retest. I reran all benchmarks on this change set. There is no difference for me.