Bug 122472 - Win64 JIT broken
Summary: Win64 JIT broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P3 Major
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 122787
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-07 16:09 PDT by Alex Christensen
Modified: 2013-10-30 09:04 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.78 KB, patch)
2013-10-29 20:18 PDT, Alex Christensen
ggaren: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2013-10-07 16:09:35 PDT
The Win64 JIT has broken in the last few days.  It worked in r156671.  It's broken in r157054.  I think the culprit is r156896, but I'm not sure.
Comment 1 Radar WebKit Bug Importer 2013-10-08 14:15:35 PDT
<rdar://problem/15179750>
Comment 2 Alex Christensen 2013-10-08 14:27:45 PDT
I confirmed that r156896 broke the Win64 JIT, and it is because edx is now used for returnValue2Register and secondArgumentRegister, but only on Windows.  My attempts at blindly switching registers are not successful, though.  Michael, do you know what registers should be used instead?
Comment 3 Alex Christensen 2013-10-08 15:00:04 PDT
This is from r156896 because it works in r156895 but not in r156896, but it's not because of edx because returnValue2Register isn't even used in Win64.  The call stack says it's crashing inside of operationNewRegexp, which was added in the same revision.  exec is 0, regexpPtr is 0x00000000097f14cc.
Comment 4 Geoffrey Garen 2013-10-08 15:01:46 PDT
Perhaps it's secondArgumentRegister that's wrong?
Comment 5 Alex Christensen 2013-10-08 15:32:19 PDT
The first, second, and third argument registers worked with ecx, edx, and r8.  I've tried changing them with no success.

For those of you who like reading disassembled JIT code, here's what the JIT is generating.  I put a break point in the first line of operationNewRegexp to get this.  I hope this helps:

00000000088400B5  add         byte ptr [rbp-77h],cl  
00000000088400B8  pop         rbp  
00000000088400B9  mov         eax,0ABB49h  
00000000088400BE  add         byte ptr [rax],al  
00000000088400C0  add         byte ptr [rax],al  
00000000088400C2  add         byte ptr [rax],al  
00000000088400C4  mov         qword ptr [r13-50h],r11  
00000000088400C8  mov         r11,0  
00000000088400D2  mov         qword ptr [r13],r11  
00000000088400D6  cmp         qword ptr [r13],0  
00000000088400DB  jne         0000000008840112  
00000000088400E1  mov         rcx,rsp  
00000000088400E4  mov         qword ptr [rsp+0A0h],r13  
00000000088400EC  mov         dword ptr [r13+34h],4  
00000000088400F4  mov         r11,8A72C80h  
00000000088400FE  mov         qword ptr [r11],r13  
0000000008840101  mov         r11,5C0BA10h  
000000000884010B  call        r11  
000000000884010E  mov         qword ptr [r13],rax  
0000000008840112  mov         rax,87EF970h  
000000000884011C  mov         qword ptr [r13-58h],rax  
0000000008840120  mov         rax,qword ptr [0000000008B4C010h]  
000000000884012A  mov         qword ptr [r13-60h],rax  
000000000884012E  test        r15,rax  
0000000008840131  jne         00000000088407EB  
0000000008840137  mov         r11,0D1E7BEEFh  
0000000008840141  cmp         qword ptr [rax],r11  
0000000008840144  jne         00000000088407EB  
000000000884014A  mov         rax,qword ptr [rax+8]  
000000000884014E  mov         rax,qword ptr [rax]  
0000000008840152  mov         qword ptr [r13-58h],rax  
0000000008840156  test        r15,rax  
0000000008840159  jne         0000000008840835  
000000000884015F  mov         r11,0D1E7BEEFh  
0000000008840169  cmp         qword ptr [rax],r11  
000000000884016C  jne         0000000008840835  
0000000008840172  mov         rax,qword ptr [rax+8]  
0000000008840176  mov         rax,qword ptr [rax]  
000000000884017A  mov         qword ptr [r13-58h],rax  
000000000884017E  test        r15,rax  
0000000008840181  jne         000000000884087F  
0000000008840187  mov         r11,0D1E7BEEFh  
0000000008840191  cmp         qword ptr [rax],r11  
0000000008840194  jne         000000000884087F  
000000000884019A  mov         rax,qword ptr [rax+8]  
000000000884019E  mov         rax,qword ptr [rax]  
00000000088401A2  mov         qword ptr [r13-8],rax  
00000000088401A6  mov         rax,87EF970h  
00000000088401B0  mov         qword ptr [r13-58h],rax  
00000000088401B4  mov         rax,qword ptr [0000000008B4C010h]  
00000000088401BE  mov         qword ptr [r13-60h],rax  
00000000088401C2  test        r15,rax  
00000000088401C5  jne         00000000088408C9  
00000000088401CB  mov         r11,0D1E7BEEFh  
00000000088401D5  cmp         qword ptr [rax],r11  
00000000088401D8  jne         00000000088408C9  
00000000088401DE  mov         rax,qword ptr [rax+8]  
00000000088401E2  mov         rax,qword ptr [rax]  
00000000088401E6  mov         qword ptr [r13-10h],rax  
00000000088401EA  mov         rax,qword ptr [r13-10h]  
00000000088401EE  test        r15,rax  
00000000088401F1  jne         0000000008840913  
00000000088401F7  mov         r11,0D1E7BEEFh  
0000000008840201  cmp         qword ptr [rax],r11  
0000000008840204  jne         0000000008840913  
000000000884020A  mov         rax,qword ptr [rax+8]  
000000000884020E  mov         rax,qword ptr [rax]  
0000000008840212  mov         qword ptr [r13-18h],rax  
0000000008840216  mov         rax,qword ptr [r13-18h]  
000000000884021A  mov         qword ptr [r13-70h],rax  
000000000884021E  test        r15,rax  
0000000008840221  jne         000000000884095D  
0000000008840227  mov         r11,0D1E7BEEFh  
0000000008840231  cmp         qword ptr [rax],r11  
0000000008840234  jne         000000000884095D  
000000000884023A  mov         rax,qword ptr [rax+8]  
000000000884023E  mov         rax,qword ptr [rax]  
0000000008840242  mov         qword ptr [r13-58h],rax  
0000000008840246  mov         r11,7D0FB30h  
0000000008840250  mov         qword ptr [r13-68h],r11  
0000000008840254  mov         r11,0FFFF000000000000h  
000000000884025E  mov         qword ptr [r13-60h],r11  
0000000008840262  lea         rdx,[r13-0A8h]  
0000000008840269  mov         dword ptr [rdx+30h],3  
0000000008840270  mov         dword ptr [r13+34h],5Eh  
0000000008840278  mov         qword ptr [rdx+28h],r13  
000000000884027C  mov         qword ptr [rdx+20h],rax  
0000000008840280  mov         r13,rdx  
0000000008840283  mov         r11,0  
000000000884028D  cmp         rax,r11  
0000000008840290  jne         00000000088409A7  
0000000008840296  mov         rdx,qword ptr [rax+20h]  
000000000884029A  mov         qword ptr [r13+18h],rdx  
000000000884029E  call        00000000088402A3  
00000000088402A3  mov         qword ptr [r13-20h],rax  
00000000088402A7  mov         rsi,8E0FE70h  
00000000088402B1  mov         rdi,r13  
00000000088402B4  mov         dword ptr [r13+34h],67h  
00000000088402BC  mov         r11,8A72C80h  
00000000088402C6  mov         qword ptr [r11],r13  
00000000088402C9  mov         r11,5BF4100h  
00000000088402D3  call        r11  
00000000088402D6  mov         r11,8A74778h               <- VISUAL STUDIO SAYS THIS IS THE CURRENT STATEMENT AND 0 IS IN r11
00000000088402E0  mov         r11,qword ptr [r11]  
00000000088402E3  test        r11,r11  
00000000088402E6  jne         0000000008841039  
00000000088402EC  mov         qword ptr [r13-28h],rax  
00000000088402F0  mov         r11,94E6F70h  
00000000088402FA  mov         qword ptr [rsp+28h],r11  
00000000088402FF  mov         rcx,rsp  
0000000008840302  mov         qword ptr [rsp+0A0h],r13  
000000000884030A  mov         dword ptr [r13+34h],6Ah  
0000000008840312  mov         r11,8A72C80h  
000000000884031C  mov         qword ptr [r11],r13  
000000000884031F  mov         r11,5C0E410h  
0000000008840329  call        r11  
000000000884032C  mov         qword ptr [r13-30h],rax  
0000000008840330  mov         rax,qword ptr [r13-10h]  
0000000008840334  test        r15,rax  
0000000008840337  jne         00000000088409B5  
000000000884033D  mov         r11,0D1E7BEEFh  
0000000008840347  cmp         qword ptr [rax],r11  
000000000884034A  jne         00000000088409B5  
0000000008840350  mov         rax,qword ptr [rax+8]  
0000000008840354  mov         rax,qword ptr [rax]  
0000000008840358  mov         qword ptr [r13-38h],rax  
000000000884035C  mov         r11,94E6E70h  
0000000008840366  mov         qword ptr [rsp+28h],r11  
000000000884036B  mov         rcx,rsp  
000000000884036E  mov         qword ptr [rsp+0A0h],r13  
0000000008840376  mov         dword ptr [r13+34h],76h  
000000000884037E  mov         r11,8A72C80h  
0000000008840388  mov         qword ptr [r11],r13  
000000000884038B  mov         r11,5C0E410h  
0000000008840395  call        r11
Comment 6 Mark Lam 2013-10-08 15:45:46 PDT
According to http://msdn.microsoft.com/en-us/library/9z1stfyw.aspx, the argument registers are all correct.  Now, looking at the rest of the diff.
Comment 7 Alex Christensen 2013-10-09 12:53:04 PDT
I built 64-bit WinLauncher with the JIT disabled from r157159 and it runs most JavaScript fine, but it crashes doing the SunSpider 1.0.1 recursive control flow test.  There might be another problem, or r156896 may have introduced a non-JIT bug as well, but SunSpider runs successfully at r156895.
Comment 8 Mark Lam 2013-10-11 11:05:56 PDT
(In reply to comment #7)
> I built 64-bit WinLauncher with the JIT disabled from r157159 and it runs most JavaScript fine, but it crashes doing the SunSpider 1.0.1 recursive control flow test.  There might be another problem, or r156896 may have introduced a non-JIT bug as well, but SunSpider runs successfully at r156895.

Alex, did you get the LLINT building on Windows already?

Regarding the crash you're seeing, were you using a release or debug build?  I'm seeing what appears to be a different crash in a debug build.
Comment 9 Alex Christensen 2013-10-11 13:22:58 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > I built 64-bit WinLauncher with the JIT disabled from r157159 and it runs most JavaScript fine, but it crashes doing the SunSpider 1.0.1 recursive control flow test.  There might be another problem, or r156896 may have introduced a non-JIT bug as well, but SunSpider runs successfully at r156895.
> 
> Alex, did you get the LLINT building on Windows already?
Yes.  I think it's been building in Win64 for a while.  I just disabled the JIT in Platform.h and it should remove any JIT-related bugs.
> 
> Regarding the crash you're seeing, were you using a release or debug build?  I'm seeing what appears to be a different crash in a debug build.
I'm pretty sure it was in a release build of WinCairo.
Comment 10 Mark Lam 2013-10-11 13:26:26 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Alex, did you get the LLINT building on Windows already?
> Yes.  I think it's been building in Win64 for a while.  I just disabled the JIT in Platform.h and it should remove any JIT-related bugs.

FYI, disabling JIT in Platform.h will also disable the ASM LLINT.  Instead, it will force the use of the C loop LLINT.  To disable the JIT but still use the ASM LLINT, add useJIT() = false; in Options::initialize().
Comment 11 Alex Christensen 2013-10-17 14:38:38 PDT
As of r157589, all tests run successfully on Win64 with ENABLE_JIT defined to be 0.  They all fail with the JIT, which means that the only problem now is in the JIT.  Should any of the assembly in JitStubsMSVC64.asm be changed?
Comment 12 Mark Lam 2013-10-17 14:43:16 PDT
(In reply to comment #11)
> As of r157589, all tests run successfully on Win64 with ENABLE_JIT defined to be 0.  They all fail with the JIT, which means that the only problem now is in the JIT.  Should any of the assembly in JitStubsMSVC64.asm be changed?

With ENABLE_JIT defined to 0, you're testing the C Loop LLINT.  The next step is to ENABLE_JIT 1 and disable the JIT at runtime so that you can run the ASM LLINT.  To disable the JIT at runtime, search in Source/JavaScriptCore/runtime/Options.cpp for "useJIT() = false;".  That statement disables the JIT but is #ifdef'ed out.  You can simply copy and paste it below the #endif.  The useJIT() flag can be set more than once.  The last value it is set to will be the one that takes effect.  To disable the JIT, you'll want to set it to false.
Comment 13 Michael Saboff 2013-10-17 14:45:40 PDT
(In reply to comment #11)
> As of r157589, all tests run successfully on Win64 with ENABLE_JIT defined to be 0.  They all fail with the JIT, which means that the only problem now is in the JIT.  Should any of the assembly in JitStubsMSVC64.asm be changed?

I broke things in r157581.  I'm currently working on a fix (https://bugs.webkit.org/show_bug.cgi?id=122980)
Comment 14 Alex Christensen 2013-10-25 13:34:50 PDT
The Win64 JIT is still broken.  I assume nobody tested http://trac.webkit.org/changeset/157650 because it crashes immediately after the call rcx in ctiTrampoline.  I'm not sure where rcx is coming from.  Is there something I could read about how the JIT works?  I'm not even sure what a trampoline is.
Comment 15 Geoffrey Garen 2013-10-25 14:33:27 PDT
ctiTrampoline is the function that is called before entering JIT code. Its job is to set up the C stack frame that JIT code will use, when calling from C to JIT code, and then to tear it down when returning to C from JIT code.
Comment 16 Alex Christensen 2013-10-29 20:18:33 PDT
Created attachment 215465 [details]
Patch
Comment 17 Alex Christensen 2013-10-29 20:23:40 PDT
This is way outside of my expertise to fix.  The argumentGPR registers need to be changed in GPRInfo.h and something is still wrong with the Visual Studio calling conventions.  Let's disable it until somebody who knows the JIT fixes it.
Comment 18 Geoffrey Garen 2013-10-29 23:45:11 PDT
Comment on attachment 215465 [details]
Patch

r=me
Comment 19 WebKit Commit Bot 2013-10-30 08:32:02 PDT
Comment on attachment 215465 [details]
Patch

Rejecting attachment 215465 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 215465, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
t.org/repository/webkit/trunk ...
Authentication realm: <http://svn.webkit.org:80> Mac OS Forge
Password for 'commit-queue@webkit.org': 
Authentication realm: <http://svn.webkit.org:80> Mac OS Forge
Username: Use of uninitialized value $username in chomp at /usr/share/git-core/perl/Git/SVN/Prompt.pm line 114.
error: git-svn died of signal 11

Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 139 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
Current branch master is up to date.

Full output: http://webkit-queues.appspot.com/results/17098194
Comment 20 Alex Christensen 2013-10-30 09:04:33 PDT
r158272