WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
117102
fourthTier: Arity fixup should be done while on same stack
https://bugs.webkit.org/show_bug.cgi?id=117102
Summary
fourthTier: Arity fixup should be done while on same stack
Michael Saboff
Reported
2013-05-31 18:39:10 PDT
We currently have a common slow path that checks for arity mismatch and then increases the stack frame for the needed missing parameters. This is done while in a subroutine call on the C stack. We need to change this mechanism so that we can grow the stack while we are using it.
Attachments
Work in progress
(10.88 KB, patch)
2013-05-31 18:45 PDT
,
Michael Saboff
no flags
Details
Formatted Diff
Diff
More work in progress
(15.09 KB, patch)
2013-06-14 05:53 PDT
,
Michael Saboff
no flags
Details
Formatted Diff
Diff
Work in progress using a thunk - lint work needed to finish patch
(17.84 KB, patch)
2013-06-17 18:42 PDT
,
Michael Saboff
no flags
Details
Formatted Diff
Diff
Patch
(23.01 KB, patch)
2013-06-18 16:25 PDT
,
Michael Saboff
no flags
Details
Formatted Diff
Diff
Patch with minor fix - replaced literals with const
(23.03 KB, patch)
2013-06-18 16:52 PDT
,
Michael Saboff
oliver
: review+
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Michael Saboff
Comment 1
2013-05-31 18:45:02 PDT
Created
attachment 203478
[details]
Work in progress Coded up new check and fixup stubs for X86 and X86_64 with the Baseline JIT using the new stubs. Both platforms pass JSC tests.
Michael Saboff
Comment 2
2013-06-14 05:53:43 PDT
Created
attachment 204706
[details]
More work in progress Working for all JITs for X86_64 and X86. Need to change over the llint.
Geoffrey Garen
Comment 3
2013-06-14 07:19:33 PDT
Comment on
attachment 204706
[details]
More work in progress View in context:
https://bugs.webkit.org/attachment.cgi?id=204706&action=review
> Source/JavaScriptCore/jit/JITStubsX86.h:114 > +COMPILE_ASSERT(ASM_UNDEFINED_TAG == JSValue::UndefinedTag, JSValue_UndefinedTag_matches_ctiFixupArity); > +COMPILE_ASSERT(ASM_CALLFRAME_HEADER_SIZE == JSStack::CallFrameHeaderSize, JSStack_CallFrameHeaderSize_matches_ctiFixupArity); > +COMPILE_ASSERT(ASM_CALLFRAME_ARGUMENT_COUNT == JSStack::ArgumentCount, JSStack_ArgumentCount_matches_ctiFixupArity);
Why can't we just use these variables -- why do we need a #define?
> Source/JavaScriptCore/runtime/CommonSlowPaths.h:93 > + // Too few arguments, return the number missing arguments so the caller can
Should be "number of missing".
Michael Saboff
Comment 4
2013-06-14 17:11:15 PDT
(In reply to
comment #3
)
> (From update of
attachment 204706
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=204706&action=review
> > > Source/JavaScriptCore/jit/JITStubsX86.h:114 > > +COMPILE_ASSERT(ASM_UNDEFINED_TAG == JSValue::UndefinedTag, JSValue_UndefinedTag_matches_ctiFixupArity); > > +COMPILE_ASSERT(ASM_CALLFRAME_HEADER_SIZE == JSStack::CallFrameHeaderSize, JSStack_CallFrameHeaderSize_matches_ctiFixupArity); > > +COMPILE_ASSERT(ASM_CALLFRAME_ARGUMENT_COUNT == JSStack::ArgumentCount, JSStack_ArgumentCount_matches_ctiFixupArity); > > Why can't we just use these variables -- why do we need a #define?
I tried various constructs. It appears that only the preprocessor runs before invoking the inline assembler and therefore the enums haven't been parsed. Mark tried making these work with the inline assembler with his recent probe patch and couldn't get it to work either.
> > Source/JavaScriptCore/runtime/CommonSlowPaths.h:93 > > + // Too few arguments, return the number missing arguments so the caller can > > Should be "number of missing".
Fixed
Geoffrey Garen
Comment 5
2013-06-15 13:09:45 PDT
Can we use the MacroAssembler to generate this code instead? That will avoid the MACRO dependency / readability problem, and it will avoid having to write a copy of this code for each platform.
Filip Pizlo
Comment 6
2013-06-15 13:11:42 PDT
(In reply to
comment #5
)
> Can we use the MacroAssembler to generate this code instead? That will avoid the MACRO dependency / readability problem, and it will avoid having to write a copy of this code for each platform.
Indeed. I agree with Geoff. This is how we usually do these things. It also carries the benefit that the stubs could be inside our JIT pool, making all jumps to them be near jumps.
Filip Pizlo
Comment 7
2013-06-15 13:12:11 PDT
(In reply to
comment #6
)
> (In reply to
comment #5
) > > Can we use the MacroAssembler to generate this code instead? That will avoid the MACRO dependency / readability problem, and it will avoid having to write a copy of this code for each platform. > > Indeed. I agree with Geoff. This is how we usually do these things. It also carries the benefit that the stubs could be inside our JIT pool, making all jumps to them be near jumps.
By the way, the best way to do this is to put it in jit/ThunkGenerators.
Michael Saboff
Comment 8
2013-06-15 14:23:20 PDT
(In reply to
comment #7
)
> (In reply to
comment #6
) > > (In reply to
comment #5
) > > > Can we use the MacroAssembler to generate this code instead? That will avoid the MACRO dependency / readability problem, and it will avoid having to write a copy of this code for each platform. > > > > Indeed. I agree with Geoff. This is how we usually do these things. It also carries the benefit that the stubs could be inside our JIT pool, making all jumps to them be near jumps. > > By the way, the best way to do this is to put it in jit/ThunkGenerators.
I'll code that up. For the llint, I'm going to replicate the code in the functionArityCheck macro and not make a call.
Michael Saboff
Comment 9
2013-06-17 18:42:35 PDT
Created
attachment 204875
[details]
Work in progress using a thunk - lint work needed to finish patch
Michael Saboff
Comment 10
2013-06-18 16:25:49 PDT
Created
attachment 204950
[details]
Patch
Michael Saboff
Comment 11
2013-06-18 16:52:59 PDT
Created
attachment 204955
[details]
Patch with minor fix - replaced literals with const
Michael Saboff
Comment 12
2013-06-19 11:38:34 PDT
Committed
r151744
: <
http://trac.webkit.org/changeset/151744
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug