Summary: | fourthTier: Arity fixup should be done while on same stack | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||||||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | fpizlo, ggaren, mark.lam, mhahnenberg, oliver | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | 117719 | ||||||||||||||
Bug Blocks: | 116888 | ||||||||||||||
Attachments: |
|
Description
Michael Saboff
2013-05-31 18:39:10 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.
Created attachment 204706 [details]
More work in progress
Working for all JITs for X86_64 and X86. Need to change over the llint.
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". (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 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. (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. (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. (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. Created attachment 204875 [details]
Work in progress using a thunk - lint work needed to finish patch
Created attachment 204950 [details]
Patch
Created attachment 204955 [details]
Patch with minor fix - replaced literals with const
Committed r151744: <http://trac.webkit.org/changeset/151744> |