Summary: | REGRESSION(r146089): It broke 20 sputnik tests on ARM traditional and Thumb2 | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fpizlo, ggaren, ossy, rgabor, zherczeg | ||||
Priority: | P1 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 108645, 112376 | ||||||
Attachments: |
|
Description
Csaba Osztrogonác
2013-03-19 03:02:33 PDT
I got it, http://trac.webkit.org/changeset/146089 is the culprit or it simple unhid an ARM traditional DFG JIT bug. I checked, this bug is valid on Thumb2 too (with an additional jsc test fail): - r146089: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/8089 - r146088: http://build.webkit.sed.hu/builders/ARMv7%20Linux%20Qt5%20Release%20%28Test%29/builds/8090 Hmmm, maybe it was already fixed by http://trac.webkit.org/changeset/146179. Let me check it. (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. (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? Created attachment 193906 [details]
command-line test
(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. 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> (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. (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! ;-) (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. (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? Fix landed in http://trac.webkit.org/changeset/146309. (In reply to comment #15) > Fix landed in http://trac.webkit.org/changeset/146309. Thanks! :-) |