Bug 126141 - cStack branch doesn't run navier-stokes because closure calls aren't implemented yet
Summary: cStack branch doesn't run navier-stokes because closure calls aren't implemen...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 113621
  Show dependency tree / graph
 
Reported: 2013-12-22 16:47 PST by Filip Pizlo
Modified: 2013-12-23 11:11 PST (History)
8 users (show)

See Also:

Attachments
the patch (11.57 KB, patch)
2013-12-23 10:55 PST, Filip Pizlo 		msaboff: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-12-22 16:47:15 PST 
I'll look into it.
Comment 1 Filip Pizlo 2013-12-23 10:55:41 PST 
Created attachment 219918 [details]
the patch
Comment 2 Michael Saboff 2013-12-23 11:04:34 PST 
Comment on attachment 219918 [details]
the patch

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

r=me with comments.

> Source/JavaScriptCore/jit/JITCall.cpp:147
> +    addPtr(TrustedImm32(-static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC))), stackPointerRegister, regT1);
>      emitGetFromCallFrameHeader64(JSStack::Callee, regT0, regT1);

Why don't you change this to a helper uses SP and takes into account the CallerFrameAndPC bias?

> Source/JavaScriptCore/jit/JITCall.cpp:191
> +    } // SP holds newCallFrame with ArgumentCount initialized.

Change the comment to include that SP is offset by CallerFrameAndPC.
Comment 3 Filip Pizlo 2013-12-23 11:11:14 PST 
(In reply to comment #2)
> (From update of attachment 219918 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=219918&action=review
> 
> r=me with comments.
> 
> > Source/JavaScriptCore/jit/JITCall.cpp:147
> > +    addPtr(TrustedImm32(-static_cast<ptrdiff_t>(sizeof(CallerFrameAndPC))), stackPointerRegister, regT1);
> >      emitGetFromCallFrameHeader64(JSStack::Callee, regT0, regT1);
> 
> Why don't you change this to a helper uses SP and takes into account the CallerFrameAndPC bias?

Good point, I can do this in one line already.

> 
> > Source/JavaScriptCore/jit/JITCall.cpp:191
> > +    } // SP holds newCallFrame with ArgumentCount initialized.
> 
> Change the comment to include that SP is offset by CallerFrameAndPC.

Fixed.
Comment 4 Filip Pizlo 2013-12-23 11:11:53 PST 
Landed in http://trac.webkit.org/changeset/161006