[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 #28 from peavo at outlook.com  2014-04-19 03:27:23 PST ---
(In reply to comment #26)

Thanks for reviewing :)

And sorry for the late reply.

> (From update of attachment 229176 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=229176&action=review
> 
> A few minor corrections. Also, could you confirm that you really wanted to change the 'nativeCallTrampoline' method to use the t0 rather than t1? I don't see why that changed.

Yes, this is intentional.
t1 (rdx) is used to hold the 2. parameter on Windows, so it's important that is's not overwritten and used as a temp register.
Better use t0 as a temp register, which is available for use on all platforms, it seems.

> 
> > Source/JavaScriptCore/llint/LLIntData.cpp:129
> > +#if CPU(X86_64) && !OS(WINDOWS) || CPU(ARM64) || ENABLE(LLINT_C_LOOP)
> 
> Are we short-circuiting here? Maybe we need parentheses around the combined X86_64 and !OS(WINDOWS) test? I think the meaning would be clearer.

Fixed in latest patch.

> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:300
> > +        # 4 parameter registers, and the call instruction won't put the return address at the correct stack location then.
> 
> This should read:
> # 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.
> 

Updated in latest patch.

> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2084
> > +        loadp JSFunction::m_executable[arg2], t0
> 
> Why was this changed from t1 to t0? Do we need to add an 'arg3' case to the condition windows/non-windows code above?

See above comment, avoid using t1 as temp register on Windows, as it is used to hold the 2. argument.

> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:2089
> > +        call executableOffsetToFunction[t0]
> 
> Again, why is t1 being changed to t0?

See above comment.

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