Bug 147893 - jsc-tailcall: Arity fixup should make use of the possible extra empty slots at top of the frame
Summary: jsc-tailcall: Arity fixup should make use of the possible extra empty slots a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basile Clement
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-11 12:28 PDT by Basile Clement
Modified: 2015-08-11 19:00 PDT (History)
4 users (show)

See Also:

Attachments
Patch (13.37 KB, patch)
2015-08-11 17:35 PDT, Basile Clement 		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 Basile Clement 2015-08-11 12:28:00 PDT 
This would:
 - Allow to not have to move the call frame when there is a single missing argument and the top of the frame was not aligned
 - Make computing the "real" frame simpler: any frame always has aligned size alignUp(max(argCount, numParameters) + CallFrameHeaderSize). Right now, the formula is quite contrived:
  alignUp(max(argCount, argCount + alignUp(numParameters - argCount)) + CallFrameHeaderSize)
Comment 1 Basile Clement 2015-08-11 17:35:58 PDT 
Created attachment 258786 [details]
Patch
Comment 2 Michael Saboff 2015-08-11 17:58:06 PDT 
Comment on attachment 258786 [details]
Patch

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

r=me

> Source/JavaScriptCore/jit/ThunkGenerators.cpp:443
> +    jit.store32(JSInterfaceJIT::regT5, MacroAssembler::BaseIndex(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::argumentGPR2, JSInterfaceJIT::TimesEight));
> +    jit.move(JSInterfaceJIT::TrustedImm32(JSValue::UndefinedTag), JSInterfaceJIT::regT5);
> +    jit.store32(JSInterfaceJIT::regT5, MacroAssembler::BaseIndex(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::argumentGPR2, JSInterfaceJIT::TimesEight, 4));

Instead of no offset or 4 and even though the existing code does this, use OBJECT_OFFSETOF(JSValue, u.asBits.payload) and OBJECT_OFFSETOF(JSValue, u.asBits.tag).
Comment 3 Basile Clement 2015-08-11 18:03:49 PDT 
(In reply to comment #2)
> Comment on attachment 258786 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258786&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jit/ThunkGenerators.cpp:443
> > +    jit.store32(JSInterfaceJIT::regT5, MacroAssembler::BaseIndex(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::argumentGPR2, JSInterfaceJIT::TimesEight));
> > +    jit.move(JSInterfaceJIT::TrustedImm32(JSValue::UndefinedTag), JSInterfaceJIT::regT5);
> > +    jit.store32(JSInterfaceJIT::regT5, MacroAssembler::BaseIndex(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::argumentGPR2, JSInterfaceJIT::TimesEight, 4));
> 
> Instead of no offset or 4 and even though the existing code does this, use
> OBJECT_OFFSETOF(JSValue, u.asBits.payload) and OBJECT_OFFSETOF(JSValue,
> u.asBits.tag).

Fixed locally (with the TagOffset and PayloadOffset shorthands).
Comment 4 Basile Clement 2015-08-11 19:00:24 PDT 
Committed r188318 <http://trac.webkit.org/changeset/188318>.