[Webkit-unassigned] [Bug 130638] [Win64] ASM LLINT is not enabled.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Apr 11 16:05:44 PDT 2014
https://bugs.webkit.org/show_bug.cgi?id=130638
--- Comment #21 from peavo at outlook.com 2014-04-11 16:06:03 PST ---
(In reply to comment #18)
Thanks for looking into this :)
> (From update of attachment 227778 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227778&action=review
>
> > Source/JavaScriptCore/jit/Repatch.cpp:836
> > - stubJit.store32(MacroAssembler::TrustedImm32(reinterpret_cast<uint32_t>(structure->id())), MacroAssembler::Address(baseGPR, JSCell::structureIDOffset()));
> > +#if USE(JSVALUE64)
> > + uint32_t val = structure->id();
> > +#else
> > + uint32_t val = reinterpret_cast<uint32_t>(structure->id());
> > +#endif
> > + stubJit.store32(MacroAssembler::TrustedImm32(val), MacroAssembler::Address(baseGPR, JSCell::structureIDOffset()));
>
> Can you please clarify why is this change needed? Your ChangeLog mentioned a "compile fix". Can you please provide some details? Thanks.
For some reason, MSVC does not accept a reinterpret_cast from uint32_t to uint32_t.
For example,
uint32_t p = 0;
reinterpret_cast<uint32_t>(p);
will generate the error
error C2440: 'reinterpret_cast' : cannot convert from 'uint32_t' to 'uint32_t'
Conversion is a valid standard conversion, which can be performed implicitly or by use of static_cast, C-style cast or function-style cast
>
> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:387
> > +# The signature of a slow path call is extern "C" SlowPathReturnType slowpathfunction(ExecState* exec, Instruction* pc).
> > +# The size of the SlowPathReturnType is 16 bytes.
> > +# On Win64, when the return type is larger than 8 bytes, we need to allocate space on the stack for the return value.
> > +# On entry rcx (t2), should contain a pointer to this stack space. The other parameters are shifted to the right,
> > +# rdx (t1) should contain the first argument, and r8 (t6) should contain the second argument.
> > +# On return, rax contains a pointer to this stack value, and we then need to copy the 16 byte return value into rax (t0) and rdx (t1)
> > +# since the return value is expected to be split between the two.
> > +# See http://msdn.microsoft.com/en-us/library/7572ztz4.aspx
> > +macro callSlowPathWin64(slowPath, arg1, arg2)
> > + move arg1, t1
> > + move arg2, t6
> > + subp 80, sp
> > + move sp, t2
> > + addp 48, t2
> > + call slowPath
> > + addp 80, sp
> > + move 8[t0], t1
> > + move [t0], t0
> > +end
>
> I suggest moving this into the cCall2() macro as a X86_64_WIN target instead of having a separate function like this.
>
> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:395
> > - cCall2(slowPath, cfr, PC)
> > + if X86_64_WIN
> > + callSlowPathWin64(slowPath, cfr, PC)
> > + else
> > + cCall2(slowPath, cfr, PC)
> > + end
>
> Instead of adding this condition everywhere, is there any reason why you can't just add a case to cCall2() to handle X86_64_WIN?
>
> Same with all the uses of cCall2() below.
I think we need to have a different implementation when the function return
type is larger than 64-bit, which is the case for the slow path call.
Win64 ABI then says that the otherwise first argument register (rcx) should
hold a pointer to a stack allocated area for the return value.
See also http://msdn.microsoft.com/en-us/library/7572ztz4.aspx for further details.
I made a new macro cCall2SlowPath in the latest patch, though, so we can avoid
adding the condition everywhere.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list