Bug 203705 – Disable Wasm interpreter on WinCairo

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Tadeu Zagallo]

...

[Tadeu Zagallo]

Created attachment 382524 [details]
Patch

[Yusuke Suzuki]

Do you know why ENABLE(WEBASSEMBLY) is ON in Windows?

[Tadeu Zagallo]

(In reply to Yusuke Suzuki from comment #2)
> Do you know why ENABLE(WEBASSEMBLY) is ON in Windows?

No idea, I asked in the original bug, and that's the only reason I didn't disable it in the original commit, since I didn't know whether it'd be of interest to actually make it work or whether I should just disable it. I figured I should just have the patch ready so we can get it back building just in case.

[Yusuke Suzuki]

Comment on attachment 382524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382524&action=review

r=me with FIXME request.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:2018
> +if WEBASSEMBLY and not X86_64_WIN

Can you add a FIXME and URL to fix this issue? I think this is essentially unnecessary, and it should be fixed.
I don’t consider that WEBASSEMBLY is true on Windows, so it would eventually can cause the problem again: like putting a code that is guarded by WEBASSEMBLY, but we don’t consider Windows at all.
So I think this should be fixed soon, but I don’t want to Wasm interpreter patch roll-out. So, I’m OK for this for very short-term workaround.

[Tadeu Zagallo]

Comment on attachment 382524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=382524&action=review

Thanks for the review!

>> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:2018
>> +if WEBASSEMBLY and not X86_64_WIN
> 
> Can you add a FIXME and URL to fix this issue? I think this is essentially unnecessary, and it should be fixed.
> I don’t consider that WEBASSEMBLY is true on Windows, so it would eventually can cause the problem again: like putting a code that is guarded by WEBASSEMBLY, but we don’t consider Windows at all.
> So I think this should be fixed soon, but I don’t want to Wasm interpreter patch roll-out. So, I’m OK for this for very short-term workaround.

Thanks, that makes sense. I'll add it.

[Tadeu Zagallo]

Created attachment 382540 [details]
Patch for landing

[Brent Fulgham]

Note: This breaks 64-bit Windows in general, not just WinCairo.

[Brent Fulgham]

Comment on attachment 382540 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=382540&action=review

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:2019
> +// https://bugs.webkit.org/show_bug.cgi?id=203716

Shouldn't these be '#' comments?

[Tadeu Zagallo]

Comment on attachment 382540 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=382540&action=review

>> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:2019
>> +// https://bugs.webkit.org/show_bug.cgi?id=203716
> 
> Shouldn't these be '#' comments?

We usually use `#`, so that was unintentional, but both work.

[WebKit Commit Bot]

Comment on attachment 382540 [details]
Patch for landing

Clearing flags on attachment: 382540

Committed r251904: <https://trac.webkit.org/changeset/251904>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56805484>

[Brent Fulgham]

Sadly, I don't believe this fixes the 64-bit Window build. I still see errors:

:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(26614): error A2008: syntax error : xmm1 [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\
ource\JavaScriptCore\JavaScriptCore.vcxproj]
:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(26721): error A2008: syntax error : xmm1 [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\
ource\JavaScriptCore\JavaScriptCore.vcxproj]

.... and many more.

26614 in LowLevelIntepreterWin.asm is the 'ucomi' command below:

  _offlineasm_compareJumpOp__llintOpWithJump__llintOpWithM_____makeReturn__fn__fn__fn__429_impl__op2NotInt:
    cvtsi2sd xmm0, eax                                       ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1861
    test rdx, r14                                            ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1862
    jz _offlineasm_compareJumpOp__llintOpWithJump__llintOpWithM___n__fn__makeReturn__fn__fn__fn__429_impl__slow
    add rdx, r14                                             ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1863
    movd xmm1, rdx                                           ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1864
    ucomi xmm1, xmm0                                         ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1712
    jbe _offlineasm_compareJumpOp__llintOpWithJump__llintOpWithM____makeReturn__fn__fn__fn__429_impl__jumpTarget
    add r10, 4                                               ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter.asm:308
    movzx eax, byte ptr [0 + r13 + r10 * 1]                  ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:28
    lea rdx, g_opcodeMap                                     ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:29
    jmp qword ptr [0 + rdx + rax * 8]                        ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:30

[Brent Fulgham]

Reopening: This does not fix the build issue (as shown above).

[Brent Fulgham]

More errors:

C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(5973): error A2008: syntax error : , [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(5990): error A2008: syntax error : , [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(6098): error A2008: syntax error : , [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(6115): error A2008: syntax error : , [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(6225): error A2008: syntax error : , [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(6242): error A2008: syntax error : , [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(6341): error A2152: coprocessor register cannot be first operand [C:\Projects\WebKit\OpenSource\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:/Projects/WebKit/OpenSource/WebKitBuild/Release/JavaScriptCore/DerivedSources/LowLevelInterpreterWin.asm(6358): error A2152: coprocessor register cannot be first operand [C:\Projects\WebKit\OpenSou

Line 5973 is the 'mul' command:

  _offlineasm_binaryOpCustomStore__llintOpWithMetadata__ll___n__fn__makeReturn__fn__fn__fn__op1NotIntReady:
    movsx r8, byte ptr [1 + r13 + r10 * 1]                   ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:50
    add rax, r14                                             ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1025
    movd xmm0, rax                                           ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1026
    mul xmm0, xmm1                                           ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1090
    movd rax, xmm0                                           ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1028
    sub rax, r14                                             ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1029
    mov qword ptr [0 + rbp + r8 * 8], rax                    ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1030
    add r10, 6                                               ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter.asm:308
    movzx eax, byte ptr [0 + r13 + r10 * 1]                  ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:28
    lea rdx, g_opcodeMap                                     ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:29
    jmp qword ptr [0 + rdx + rax * 8]                        ; C:/Projects/WebKit/OpenSource/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:30

[Brent Fulgham]

I really don't think things are in a good state right now. Can't we roll this out somehow?

[Ian Grunert]

I think this can be closed, wasm interpreter has been enabled on Windows by https://bugs.webkit.org/show_bug.cgi?id=222315