[Webkit-unassigned] [Bug 130638] [Win64] ASM LLINT is not enabled.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130638





--- Comment #42 from peavo at outlook.com  2014-05-26 07:33:45 PST ---
(In reply to comment #40)
> (From update of attachment 231527 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231527&action=review
> 

Thanks for reviewing!

> > Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:162
> > +        store64(X86Registers::ebp, Address(X86Registers::esp, -16));
> 
> Why do we need to preserve the ebp here?  According to http://msdn.microsoft.com/en-us/library/6t169e9c.aspx, ebp is non-volatile.
> 

This is not done to preserve ebp.
As I've understood it, for a host function call, JIT relies on that the frame pointer is put on the stack after the return address.
The first parameter of a host function call (ExecState *), will then point to the value of the frame pointer.
Since MSVC does not maintain the frame pointer on 64-bit, we have to do this manually on Windows.
Without this, I get many crashes/failures running the stress tests.

> 
> > Source/JavaScriptCore/jit/JITStubsMSVC64.asm:35
> > +    ; Allocate space for all 4 parameter registers, and align stack pointer to 16 bytes.
> > +    sub rsp, 40
> 
> I couldn't find any MSVC documentation that describes how the stack must be aligned.  I understand the first 32 bytes is for the arg registers shadow space, but Is the extra 8 byte padding here really appropriate and necessary? Did you encounter an issue without this padding?
> 

Yes, I was consistently getting a crash in the crt library without the extra stack alignment.
It was crashing on a floating point instruction, if I remember correctly.

>  
> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:71
> > +        subp 80, sp
> 
> 32 (args shadow space) + 16 (return value) ==> 48.  Why 80?  What am I missing?
> 

Fixed in latest patch.

> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:327
> > +    elsif X86_64_WIN
> > +        # On Win64 we need to manually copy the frame pointer to the stack, since MSVC may not maintain a frame pointer on 64-bit.
> > +        # See http://msdn.microsoft.com/en-us/library/9z1stfyw.aspx where it's stated that rbp MAY be used as a frame pointer.
> > +        # Also, we need to manually copy the return address to the stack, since before the call instruction we allocated space
> > +        # for the 4 parameter registers, and the call instruction won't put the return address at the correct stack location.
> > +        move sp, t2
> > +        storep cfr, [sp]
> > +        call .copyReturnAddressToStack
> > +.copyReturnAddressToStack:
> > +        pop 8[sp]
> > +        # Adjust the return address with the offset to the instruction after the call instruction.
> > +        addp 16, 8[sp]
> 
> Apart from the "move sp, t2", why do we need to do this?   The rbp is a non-volatile register even if it's not used as a frame pointer.

I have removed the calculation of the return address, since it's not really needed, as you mentioned earlier.
Storing the frame pointer on the stack is needed, I believe, since JIT relies on that the frame pointer is put on the stack, and MSVC does not maintain the frame pointer.
The first parameter of the host function call (ExecState *), will then point to the value of the frame pointer.
The same is also done in the CLOOP case.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.