Bug 125964 - CStack Branch: Stop threading callFrameRegister through LLIntSlowCalls
Summary: CStack Branch: Stop threading callFrameRegister through LLIntSlowCalls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-18 17:29 PST by Michael Saboff
Modified: 2013-12-18 22:34 PST (History)
1 user (show)

See Also:


Attachments
Patch (5.55 KB, patch)
2013-12-18 17:44 PST, Michael Saboff
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2013-12-18 17:29:19 PST
The LLint always passes the callFrameRegister to LLInt slow patch calls as the "exec" argument.  The slow calls always return an exec value, usually the one passed in.  There is no need for this now that we are using the CPU's call frame register.  In some cases it is even wrong.  Instead we should only use the second return value when we have a real value to return, like the execCallee for a llint_slow_path_call().
Comment 1 Michael Saboff 2013-12-18 17:44:17 PST
Created attachment 219595 [details]
Patch
Comment 2 Geoffrey Garen 2013-12-18 18:05:38 PST
Comment on attachment 219595 [details]
Patch

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

r=me

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:804
>      callSlowPath(_llint_slow_path_size_and_alloc_frame_for_varargs)
>      branchIfException(_llint_throw_from_slow_path_trampoline)
> -    loadp CodeBlock[cfr], t0
> -    loadp CodeBlock::m_vm[t0], t0
> -    loadp VM::newCallFrameReturnValue[t0], t0
> -    move t0, sp
> +    # calleeFrame in t1
> +    move t1, sp

Is it valid for _llint_slow_path_size_and_alloc_frame_for_varargs to allocate calleeFrame like this?
Comment 3 Michael Saboff 2013-12-18 18:09:26 PST
(In reply to comment #2)
> (From update of attachment 219595 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219595&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:804
> >      callSlowPath(_llint_slow_path_size_and_alloc_frame_for_varargs)
> >      branchIfException(_llint_throw_from_slow_path_trampoline)
> > -    loadp CodeBlock[cfr], t0
> > -    loadp CodeBlock::m_vm[t0], t0
> > -    loadp VM::newCallFrameReturnValue[t0], t0
> > -    move t0, sp
> > +    # calleeFrame in t1
> > +    move t1, sp
> 
> Is it valid for _llint_slow_path_size_and_alloc_frame_for_varargs to allocate calleeFrame like this?

Yes it is, because all it is doing is address arithmetic, i.e. where the frame should go.  It also does the stack check.  However it never writes anything into the new frame header.
Comment 4 Michael Saboff 2013-12-18 18:18:44 PST
Committed r160815: <http://trac.webkit.org/changeset/160815>
Comment 5 Geoffrey Garen 2013-12-18 18:23:59 PST
I see. Can we rename _llint_slow_path_size_and_alloc_frame_for_varargs to _llint_slow_path_size_frame_for_varargs, and similar names, if any?
Comment 6 Michael Saboff 2013-12-18 22:34:45 PST
(In reply to comment #5)
> I see. Can we rename _llint_slow_path_size_and_alloc_frame_for_varargs to _llint_slow_path_size_frame_for_varargs, and similar names, if any?

WIll do <https://bugs.webkit.org/show_bug.cgi?id=125980>