Bug 146481

Summary: jsc-tailcall: Recognize calls in tail position
Product: WebKit Reporter: Basile Clement <basile_clement>
Component: JavaScriptCoreAssignee: Basile Clement <basile_clement>
Status: RESOLVED DUPLICATE    
Severity: Enhancement CC: fpizlo, ggaren, mark.lam, msaboff
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 146477, 146484    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Basile Clement 2015-06-30 17:19:36 PDT 
Implement the IsInTailPosition static property (https://people.mozilla.org/~jorendorff/es6-draft.html#sec-isintailposition) to recognize calls in tail position.
Comment 1 Basile Clement 2015-07-10 18:13:01 PDT 
Created attachment 256632 [details]
Patch
Comment 2 Saam Barati 2015-07-14 14:54:57 PDT 
Comment on attachment 256632 [details]
Patch

Looks good to me. Here are a few missing cases:

LogicalANDExpression : LogicalANDExpression && BitwiseORExpression
Return HasProductionInTailPosition of BitwiseORExpression with argument nonterminal.
LogicalORExpression : LogicalORExpression || LogicalANDExpression
Return HasProductionInTailPosition of LogicalANDExpression with argument nonterminal.
Comment 3 Saam Barati 2015-07-14 14:58:43 PDT 
Comment on attachment 256632 [details]
Patch

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

> Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:2949
> +            generator.emitNode(dst, m_catchBlock, tailCallMode);

This can also be written to be more explicitly not relying on default parameter:
generator.emitNode(dot, m_catchBlock, m_finallyBlock ? NoTailCalls : tailCallMode).
Comment 4 Basile Clement 2015-07-14 15:06:15 PDT 
(In reply to comment #2)
> Comment on attachment 256632 [details]
> Patch
> 
> Looks good to me. Here are a few missing cases:
> 
> LogicalANDExpression : LogicalANDExpression && BitwiseORExpression
> Return HasProductionInTailPosition of BitwiseORExpression with argument
> nonterminal.
> LogicalORExpression : LogicalORExpression || LogicalANDExpression
> Return HasProductionInTailPosition of LogicalANDExpression with argument
> nonterminal.

Good catch, adding those.

(In reply to comment #3)
> Comment on attachment 256632 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=256632&action=review
> 
> > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:2949
> > +            generator.emitNode(dst, m_catchBlock, tailCallMode);
> 
> This can also be written to be more explicitly not relying on default
> parameter:
> generator.emitNode(dot, m_catchBlock, m_finallyBlock ? NoTailCalls :
> tailCallMode).

That is indeed better, changing this.
Comment 5 Basile Clement 2015-07-14 15:17:05 PDT 
Created attachment 256800 [details]
Patch

Add LogicalOpNode as a possible tail call, simplifies TryNode and ScopeNode.
Comment 6 Basile Clement 2015-07-14 15:36:22 PDT 
Committed r186822 <http://trac.webkit.org/changeset/186822>.
Comment 7 Basile Clement 2015-08-31 19:04:08 PDT 
Reopening to attach new patch.
Comment 8 Basile Clement 2015-08-31 19:04:09 PDT 
Created attachment 260344 [details]
Patch
Comment 9 Basile Clement 2015-08-31 19:04:40 PDT 

*** This bug has been marked as a duplicate of bug 148665 ***
Comment 10 Basile Clement 2015-09-01 14:09:03 PDT 
Reopening to attach new patch.
Comment 11 Basile Clement 2015-09-01 14:09:04 PDT 
Created attachment 260380 [details]
Patch
Comment 12 Basile Clement 2015-09-01 14:09:56 PDT 

*** This bug has been marked as a duplicate of bug 148665 ***
Comment 13 Csaba Osztrogonác 2015-09-14 11:02:07 PDT 
Comment on attachment 260380 [details]
Patch

Cleared review? from attachment 260380 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).