[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 #58 from Mark Lam <mark.lam at apple.com>  2014-06-24 17:24:41 PST ---
(From update of attachment 233709)
View in context: https://bugs.webkit.org/attachment.cgi?id=233709&action=review

I would r+ this patch but first, I'd like you to try undoing the .opPutByIdSlow changes I suggested.  If they are not needed anymore, let's undo them.  Let me know if they are still or not.  Thanks.

> Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:166
> +        load64(Address(X86Registers::esp, 32), scratchRegister);
> +        store64(scratchRegister, Address(X86Registers::esp, -32));
> +
> +        load64(Address(X86Registers::esp, 40), scratchRegister);
> +        store64(scratchRegister, Address(X86Registers::esp, -24));

To answer your question about using a POKE_ARGUMENT_OFFSET of -4 to achieve this, I think you made a good point there.  I think it will work (and one might want to go with that solution if performance is an issue here).  However, it's not obvious from reading the code as to how things work since it differs from how all other ports expect it to work.  So, if you were to go with that solution, I would like to see a comment here to describe why the 5th and 6th arguments are already in place and no further work is needed.  I would also want a comment in the definition POKE_ARGUMENT_OFFSET to explain why it has a negative offset.  However, if performance is not an issue, then let's stick with this more obvious solution for now.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1366
> +
> +    .opPutByIdSlow:
> +        callSlowPath(_llint_slow_path_put_by_id)
> +        dispatch(9)
> +

The LLINT code has been re-written recently such that all the bytecode handlers are now local labels inside the same function.  As such, I think this change may no longer be needed (though I had suggested it back when the LLINT was implementing each bytecode handler with a separate global label).  Sorry for the hassle, but can you please try undoing this change and seeing if LLINT on Win64 builds and works without this change?

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:-1307
> -.opPutByIdSlow:
> -    callSlowPath(_llint_slow_path_put_by_id)
> -    dispatch(9)
> -

Ditto.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1385
> -    additionalChecks(t1, t3, t2)
> +    additionalChecks(t1, t3, t2, .opPutByIdSlow)

Ditto.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1404
> +
> +    .opPutByIdSlow:
> +        callSlowPath(_llint_slow_path_put_by_id)
> +        dispatch(9)
> +

Ditto.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1407
> -macro noAdditionalChecks(oldStructure, scratch, scratch2)
> +macro noAdditionalChecks(oldStructure, scratch, scratch2, slowPath)

Ditto.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1410
> -macro structureChainChecks(oldStructure, scratch, scratch2)
> +macro structureChainChecks(oldStructure, scratch, scratch2, slowPath)

Ditto.

> Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1421
> -    bpneq oldStructure, [scratch], .opPutByIdSlow
> +    bpneq oldStructure, [scratch], slowPath

Ditto.

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