[webkit-reviews] review granted: [Bug 176601] Turn recursive tail calls into loops : [Attachment 324120] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 18 11:46:56 PDT 2017


Saam Barati <sbarati at apple.com> has granted Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 176601: Turn recursive tail calls into loops
https://bugs.webkit.org/show_bug.cgi?id=176601

Attachment 324120: Patch

https://bugs.webkit.org/attachment.cgi?id=324120&action=review




--- Comment #85 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 324120
  --> https://bugs.webkit.org/attachment.cgi?id=324120
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324120&action=review

r=me with a few comments.

> Source/JavaScriptCore/bytecode/CodeBlock.h:928
> +    bool hasTailCalls() const {return m_unlinkedCode->hasTailCalls();}

style nit: this should be written as:
{ return m_unlinkedCode->hasTailCalls(); }
for the spacing to follow webkit style

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:1321
> +    // We must add the end of op_enter as a potential jump target, because
the bytecode parser may decide to split its basic block
> +    // to have somewhere to jump to if there is a recursive tail-call that
points to this function.
> +    m_codeBlock->addJumpTarget(instructions().size());
> +    // This disables peephole optimizations when an instruction is a jump
target
> +    m_lastOpcodeID = op_end;

Is it worth just emitting a label since that will do this work for us? Or
should we keep this as is?

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:1469
> +	   // We jump right after it and not before it, because of some
invariant saying that an OSR entry point cannot have predecessors in the IR.

This is not an accurate comment. The invariant is that a root in the CFG cannot
have a predecessor. An OSR entrypoint *can* have predecessors. For example,
loops are often OSR entry points. FWIW, we could totally jump to op_enter, we'd
just need to split before op_enter, not after. This may be awkward. I'm totally
OK with how you've implemented it.


More information about the webkit-reviews mailing list