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

Basile Clement
Reported 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.
Attachments
Patch (110.44 KB, patch)
2015-07-10 18:13 PDT, Basile Clement 		no flags
DetailsFormatted DiffDiff
Patch (115.09 KB, patch)
2015-07-14 15:17 PDT, Basile Clement 		no flags
DetailsFormatted DiffDiff
Patch (118.35 KB, patch)
2015-08-31 19:04 PDT, Basile Clement 		no flags
DetailsFormatted DiffDiff
Patch (28.40 KB, patch)
2015-09-01 14:09 PDT, Basile Clement 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Basile Clement
Comment 1 2015-07-10 18:13:01 PDT
Created attachment 256632 [details] Patch
Saam Barati
Comment 2 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.
Saam Barati
Comment 3 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).
Basile Clement
Comment 4 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.
Basile Clement
Comment 5 2015-07-14 15:17:05 PDT
Created attachment 256800 [details] Patch Add LogicalOpNode as a possible tail call, simplifies TryNode and ScopeNode.
Basile Clement
Comment 6 2015-07-14 15:36:22 PDT
Committed r186822 <http://trac.webkit.org/changeset/186822>.
Basile Clement
Comment 7 2015-08-31 19:04:08 PDT
Reopening to attach new patch.
Basile Clement
Comment 8 2015-08-31 19:04:09 PDT
Created attachment 260344 [details] Patch
Basile Clement
Comment 9 2015-08-31 19:04:40 PDT
*** This bug has been marked as a duplicate of bug 148665 ***
Basile Clement
Comment 10 2015-09-01 14:09:03 PDT
Reopening to attach new patch.
Basile Clement
Comment 11 2015-09-01 14:09:04 PDT
Created attachment 260380 [details] Patch
Basile Clement
Comment 12 2015-09-01 14:09:56 PDT
*** This bug has been marked as a duplicate of bug 148665 ***
Csaba Osztrogonác
Comment 13 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).
Note You need to log in before you can comment on or make changes to this bug.