Bug 125964 – CStack Branch: Stop threading callFrameRegister through LLIntSlowCalls

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

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().

[Michael Saboff]

Created attachment 219595 [details]
Patch

[Geoffrey Garen]

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?

[Michael Saboff]

(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.

[Michael Saboff]

Committed r160815: <http://trac.webkit.org/changeset/160815>

[Geoffrey Garen]

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?

[Michael Saboff]

(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>