Closed
Bug 847605
Opened 11 years ago
Closed 11 years ago
Frequent jit-test TEST-UNEXPECTED-FAIL | jit_test.py --no-jm | auto-regress/bug755564.js: Assertion failure: GeneralRegisterSet(Registers::SingleByteRegs).has(lhs), at ../../../js/src/ion/shared/Assembler-x86-shared.h:726 and more
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: RyanVM, Unassigned)
References
Details
(Keywords: assertion, intermittent-failure)
Attachments
(1 file)
5.07 KB,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
I don't know exactly when this regressed because we hadn't been running these since moz.build landed last week. However, inbound is now closed for this due to the high frequency with which it's hitting. https://tbpl.mozilla.org/php/getParsedLog.php?id=20309752&tree=Mozilla-Inbound Linux mozilla-inbound leak test build on 2013-03-04 11:51:20 PST for push a3d0d869bd50 slave: bld-linux64-ec2-623 TEST-PASS | jit_test.py --no-ion --no-jm --no-ti| /builds/slave/m-in-lx-d-00000000000000000000/build/js/src/jit-test/tests/auto-regress/bug755564.js TEST-PASS | jit_test.py --no-ion --no-ti| /builds/slave/m-in-lx-d-00000000000000000000/build/js/src/jit-test/tests/auto-regress/bug755564.js FAIL - /builds/slave/m-in-lx-d-00000000000000000000/build/js/src/jit-test/tests/auto-regress/bug755564.js TEST-UNEXPECTED-FAIL | jit_test.py --no-jm | /builds/slave/m-in-lx-d-00000000000000000000/build/js/src/jit-test/tests/auto-regress/bug755564.js: Assertion failure: GeneralRegisterSet(Registers::SingleByteRegs).has(lhs), at ../../../js/src/ion/shared/Assembler-x86-shared.h:726 TEST-PASS | jit_test.py --no-ion --no-ti --always-mjit --debugjit| /builds/slave/m-in-lx-d-00000000000000000000/build/js/src/jit-test/tests/auto-regress/bug755564.js and others like it...
Reporter | ||
Updated•11 years ago
|
OS: All → Linux
Hardware: All → x86
Comment 1•11 years ago
|
||
This bug is coming from Bug 807853, by adding a second use of testb which does not ensure that operands can be encoded. --- a/js/src/ion/shared/MacroAssembler-x86-shared.h +++ b/js/src/ion/shared/MacroAssembler-x86-shared.h @@ -140,6 +143,10 @@ class MacroAssemblerX86Shared : public Assembler testl(Operand(address), imm); j(cond, label); } + void branchTestBool(Condition cond, const Register &lhs, const Register &rhs, Label *label) { + testb(lhs, rhs); + j(cond, label); + } // The following functions are exposed for use in platform-shared code. template <typename T>
Blocks: 807853
Comment 2•11 years ago
|
||
I am pushing a fix right now, The problem might be coming from: LIRGenerator::visitParCheckOverRecursed(MParCheckOverRecursed *ins) LIRGenerator::visitParCheckInterrupt(MParCheckInterrupt *ins) which are using "temp()" for the register used in branchTestBool. The current work-around makes sure that we fallback on a 32 bits compare instead of a using the right registers. Shu, Nikos, any constraints at changing these to be fixed temps?
Comment 3•11 years ago
|
||
(In reply to Nicolas B. Pierron [:nbp] from comment #2) > I am pushing a fix right now, > > The problem might be coming from: > > LIRGenerator::visitParCheckOverRecursed(MParCheckOverRecursed *ins) > LIRGenerator::visitParCheckInterrupt(MParCheckInterrupt *ins) > > which are using "temp()" for the register used in branchTestBool. > > The current work-around makes sure that we fallback on a 32 bits compare > instead of a using the right registers. > > Shu, Nikos, any constraints at changing these to be fixed temps? Its sounds that the 2 out of line paths used for these instructions should be oolVMCall instead of callWithABI.
Comment 5•11 years ago
|
||
Ok, this fix does not seems to be needed after all as all bug failures went away with the backout of Bug 829602 from the tree. https://hg.mozilla.org/integration/mozilla-inbound/rev/37f8e72f5124
Blocks: 829602
Reporter | ||
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/6af9594ac0d4
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Comment 7•11 years ago
|
||
It's worth noting that this patch changes the semantics of branchTestBool() in an incorrect way (it now tests the entire word, not just the least significant byte). I will post a compensating patch as part of bug 829602, either by using tempFixed() or rewrite the relevant code to mask away the insignificant bits. (The problem here is that the functions in question test the return value from C functions that return |bool|, which is typically just one byte in size)
Comment 8•11 years ago
|
||
Given that it is taking me some time to nail down the last transient failure from bug 829602, and that there are new users for |branchTestBool()| being added in the meantime, I want to land my revised patch independently. Hence I am reopening this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 9•11 years ago
|
||
Not sure about the name of the function, but it seems to fulfill a need.
Attachment #721810 -
Flags: review?(nicolas.b.pierron)
Updated•11 years ago
|
Attachment #721810 -
Flags: review?(nicolas.b.pierron) → review+
Comment 10•11 years ago
|
||
Try run for 1add3f03e255 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=1add3f03e255 Results (out of 19 total builds): success: 17 warnings: 1 failure: 1 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nmatsakis@mozilla.com-1add3f03e255
Reporter | ||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ba7e066d215c
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•