Bug 146850

Summary:

jsc-tailcall: Implement the tail call opcodes in the DFG

Product:

WebKit

Reporter:

Basile Clement <basile_clement>

Component:

JavaScriptCore

Assignee:

Basile Clement <basile_clement>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

fpizlo, ggaren, keith_miller, mark.lam, mmirman, msaboff, saam

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

146484

Bug Blocks:

146477, 146851, 147198

Attachments:

Description	Flags
Patch	none
Patch	msaboff: review+

Description Basile Clement 2015-07-10 12:38:57 PDT

...

Comment 1 Basile Clement 2015-07-31 14:49:36 PDT

*** Bug 147198 has been marked as a duplicate of this bug. ***

Comment 2 Basile Clement 2015-07-31 19:08:34 PDT

Created attachment 257988 [details]
Patch

Comment 3 Basile Clement 2015-07-31 19:40:31 PDT

Just realized this is missing callee save stuff.

Comment 4 Basile Clement 2015-08-03 14:42:45 PDT

Created attachment 258116 [details]
Patch

Comment 5 Michael Saboff 2015-08-03 16:02:56 PDT

Comment on attachment 258116 [details]
Patch

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

r+me with a couple of comments.

> Source/JavaScriptCore/ChangeLog:13
> +         - EmulatedTailCall and EmulatedTailCallVarargs are here to perform a

What about changing Emulated* to Inlined*?

> Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:139
> +    // FIXME: We shouldn't leave holes on the stack when performing an OSR exit

Please add a bug for the FIXME

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:881
> +        m_jit.breakpoint();

This should only be for debug builds.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:842
> +        m_jit.breakpoint();

Ditto

Comment 6 Basile Clement 2015-08-03 16:17:58 PDT

(In reply to comment #5)
> Comment on attachment 258116 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258116&action=review
> 
> r+me with a couple of comments.
> 
> > Source/JavaScriptCore/ChangeLog:13
> > +         - EmulatedTailCall and EmulatedTailCallVarargs are here to perform a
> 
> What about changing Emulated* to Inlined*?

I find InlinedTailCall to be confusing since the tail call itself is not inlined, it is inside an inlined function. And TailCallInsideAnInlinedFunction is not great either. But I'm open to suggestions.
 
> > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:139
> > +    // FIXME: We shouldn't leave holes on the stack when performing an OSR exit
> 
> Please add a bug for the FIXME

Oops, thought I did that already. I'll add it.

Comment 7 Michael Saboff 2015-08-03 16:37:42 PDT

(In reply to comment #6)
> (In reply to comment #5)
> > Comment on attachment 258116 [details]
> > > Source/JavaScriptCore/ChangeLog:13
> > > +         - EmulatedTailCall and EmulatedTailCallVarargs are here to perform a
> > 
> > What about changing Emulated* to Inlined*?
> 
> I find InlinedTailCall to be confusing since the tail call itself is not
> inlined, it is inside an inlined function. And
> TailCallInsideAnInlinedFunction is not great either. But I'm open to
> suggestions.

What about TailCallInlinedCaller?

Comment 8 Basile Clement 2015-08-04 10:50:52 PDT

Committed r187791 <https://trac.webkit.org/changeset/187791>.