RESOLVED FIXED 112676
REGRESSION(r146089): It broke 20 sputnik tests on ARM traditional and Thumb2 
https://bugs.webkit.org/show_bug.cgi?id=112676
Summary REGRESSION(r146089): It broke 20 sputnik tests on ARM traditional and Thumb2
Csaba Osztrogonác
Reported 2013-03-19 03:02:33 PDT
A change between r146057-r146163 broke 20 sputnik tests on ARM traditional, maybe on ARM Thumb2 too, who knows. See this page for details: http://build.webkit.sed.hu/results/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/r146163%20%288084%29/results.html I'm going to bisect the culprit revision.
Attachments
command-line test (3.08 KB, application/x-javascript)
2013-03-19 13:17 PDT, Filip Pizlo 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Csaba Osztrogonác
Comment 1 2013-03-19 03:48:36 PDT
The bug is already valid on r146100.
Csaba Osztrogonác
Comment 2 2013-03-19 05:00:49 PDT
I got it, http://trac.webkit.org/changeset/146089 is the culprit or it simple unhid an ARM traditional DFG JIT bug.
Csaba Osztrogonác
Comment 4 2013-03-19 06:29:38 PDT
Hmmm, maybe it was already fixed by http://trac.webkit.org/changeset/146179. Let me check it.
Csaba Osztrogonác
Comment 5 2013-03-19 06:53:38 PDT
(In reply to comment #4) > Hmmm, maybe it was already fixed by http://trac.webkit.org/changeset/146179. Let me check it. No, it wasn't fixed, the bug is still valid on r146200.
Geoffrey Garen
Comment 6 2013-03-19 09:50:47 PDT
<rdar://problem/13452502>
Filip Pizlo
Comment 7 2013-03-19 13:17:00 PDT
(In reply to comment #5) > (In reply to comment #4) > > Hmmm, maybe it was already fixed by http://trac.webkit.org/changeset/146179. Let me check it. > > No, it wasn't fixed, the bug is still valid on r146200. I cannot reproduce this on command-line in r146239. Ossy, can you check if the attached test case passes on jsc command line for you in ARMv7? Also, is ARMv7 on Qt using the ARMv7Assembler?
Filip Pizlo
Comment 8 2013-03-19 13:17:20 PDT
Created attachment 193906 [details] command-line test
Csaba Osztrogonác
Comment 9 2013-03-19 15:54:45 PDT
(In reply to comment #8) > Created an attachment (id=193906) [details] > command-line test I have ARM traditional build now and this test fails with DFG JIT, but passes with disabled DFG JIT: $ ./jsc -f 1.js --useDFGJIT=false <span><span class="pass">PASS</span> </span> <br /><span class="pass">TEST COMPLETE</span> $ ./jsc -f 1.js <span><span class="fail">FAIL</span> SputnikError: #002.05833591723e-3126.3659873724e-314-002.397855243786e-312F </span> <br /><span class="pass">TEST COMPLETE</span> Let me check it on Thumb2 build too.
Csaba Osztrogonác
Comment 10 2013-03-19 16:04:41 PDT
I got similar results with Thumb2 build: (on r146178) $ ./jsc -f 1.js --useDFGJIT=false <span><span class="pass">PASS</span> </span> <br /><span class="pass">TEST COMPLETE</span> buildbot@panda1:~/cute1/slaves/armReleaseTest/buildslave/arm-qt-linux-release- $ ./jsc -f 1.js <span><span class="fail">FAIL</span> SputnikError: #002.1219957905e-3142.1219957905e-314-002.1219957905e-314F </span> <br /><span class="pass">TEST COMPLETE</span>
Csaba Osztrogonác
Comment 11 2013-03-19 16:08:44 PDT
(In reply to comment #7) > Also, is ARMv7 on Qt using the ARMv7Assembler? Our GCC's default is ARM, which uses ARMAssembler, but we can override it with -mthumb cflag and then ARMv7Assembler is used. I checked manually and this bug is valid with both of them.
Filip Pizlo
Comment 12 2013-03-19 16:19:39 PDT
(In reply to comment #11) > (In reply to comment #7) > > Also, is ARMv7 on Qt using the ARMv7Assembler? > > Our GCC's default is ARM, which uses ARMAssembler, but we can > override it with -mthumb cflag and then ARMv7Assembler is used. > I checked manually and this bug is valid with both of them. OK - this is super weird! This test passes fine for me. I think I found the issue though. Look at this code in DFGSpeculativeJIT.h: JITCompiler::Call callOperation(C_DFGOperation_EJ operation, GPRReg result, GPRReg arg1Tag, GPRReg arg1Payload) { m_jit.setupArgumentsWithExecState(arg1Payload, arg1Tag); return appendCallWithExceptionCheckSetResult(operation, result); } Can you try passing EABI_32BIT_DUMMY_ARG like so: m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag); And seeing if it passes? The DUMMY_ARG thing isn't necessary on our ABI but I think it is on yours. Sorry for this breakage! ABIs are hard! ;-)
Csaba Osztrogonác
Comment 13 2013-03-19 16:46:39 PDT
(In reply to comment #12) > Can you try passing EABI_32BIT_DUMMY_ARG like so: > > m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag); > > And seeing if it passes? The DUMMY_ARG thing isn't necessary on our ABI but I think it is on yours. > > Sorry for this breakage! ABIs are hard! ;-) Yay, this command line test passes with this change. Many thanks for the fix.
Filip Pizlo
Comment 14 2013-03-19 17:49:08 PDT
(In reply to comment #13) > (In reply to comment #12) > > Can you try passing EABI_32BIT_DUMMY_ARG like so: > > > > m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag); > > > > And seeing if it passes? The DUMMY_ARG thing isn't necessary on our ABI but I think it is on yours. > > > > Sorry for this breakage! ABIs are hard! ;-) > > Yay, this command line test passes with this change. Many thanks for the fix. Can you land?
Csaba Osztrogonác
Comment 15 2013-03-20 00:45:58 PDT
Fix landed in http://trac.webkit.org/changeset/146309.
Filip Pizlo
Comment 16 2013-03-20 00:57:32 PDT
(In reply to comment #15) > Fix landed in http://trac.webkit.org/changeset/146309. Thanks! :-)
Note You need to log in before you can comment on or make changes to this bug.