This should make benchmarking and debugging this optimization a bit less painful. Name of the option: optimizeRecursiveTailCalls
Created attachment 327060 [details] Patch
Comment on attachment 327060 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327060&action=review r=me > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1318 > + if (Options::optimizeRecursiveTailCalls()) { I suggest making this: if (LIKELY(Options::optimizeRecursiveTailCalls())) { > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425 > + if (!Options::optimizeRecursiveTailCalls()) I suggest making this: if (UNLIKELY(!Options::optimizeRecursiveTailCalls()))
Created attachment 327062 [details] Patch for landing
Comment on attachment 327062 [details] Patch for landing Clearing flags on attachment: 327062 Committed r224918: <https://trac.webkit.org/changeset/224918>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35588622>
Comment on attachment 327062 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=327062&action=review > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425 > + if (UNLIKELY(!Options::optimizeRecursiveTailCalls())) Why reference this option anywhere else? Isn’t it only here that we care?
(In reply to Saam Barati from comment #7) > Comment on attachment 327062 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=327062&action=review > > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425 > > + if (UNLIKELY(!Options::optimizeRecursiveTailCalls())) > > Why reference this option anywhere else? Isn’t it only here that we care? The other two places where we reference this option take care of splitting the entry block for functions that have at least one tail call, so the optimisation has somewhere to jump to. I decided to also make them conditional on the option, to make it easier to benchmark cleanly the cost incurred by this.
(In reply to Robin Morisset from comment #8) > (In reply to Saam Barati from comment #7) > > Comment on attachment 327062 [details] > > Patch for landing > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=327062&action=review > > > > > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1425 > > > + if (UNLIKELY(!Options::optimizeRecursiveTailCalls())) > > > > Why reference this option anywhere else? Isn’t it only here that we care? > > The other two places where we reference this option take care of splitting > the entry block for functions that have at least one tail call, so the > optimisation has somewhere to jump to. > I decided to also make them conditional on the option, to make it easier to > benchmark cleanly the cost incurred by this. 👍🏼