Closed
Bug 1341292
Opened 7 years ago
Closed 7 years ago
MIPS32/MIPS64: Fix assert related to incorrect use of scratch registers
Categories
(Core :: JavaScript Engine: JIT, defect, P5)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: ivica.bogosavljevic, Assigned: ivica.bogosavljevic)
Details
Attachments
(1 file, 1 obsolete file)
2.35 KB,
patch
|
lth
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 Steps to reproduce: There are two unused asserts in MoveEmitterMIPS[32|64]::emitDoubleMove that cause failures in some tests. The asserts check if the source or destination register is scratch register, however scratch register is actually used only for memory to memory move. Test that are fixed by this patch (jit-tests): asm.js/testFloat32.js SIMD/uconvert.js
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(lhansen)
Comment 1•7 years ago
|
||
Comment on attachment 8839469 [details] [diff] [review] incorrect_use_scratch.patch Review of attachment 8839469 [details] [diff] [review]: ----------------------------------------------------------------- Sure, looks good. We can probably just land this, right? If you think you want some feedback on a patch but don't want to ask for review yet, then you can use the "feedback?" flag on the patch in bugzilla.
Attachment #8839469 -
Flags: review+
Updated•7 years ago
|
Flags: needinfo?(lhansen)
Updated•7 years ago
|
Assignee: nobody → ivica.bogosavljevic
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to Lars T Hansen [:lth] from comment #1) > Comment on attachment 8839469 [details] [diff] [review] > incorrect_use_scratch.patch > > Review of attachment 8839469 [details] [diff] [review]: > ----------------------------------------------------------------- > > Sure, looks good. We can probably just land this, right? > > If you think you want some feedback on a patch but don't want to ask for > review yet, then you can use the "feedback?" flag on the patch in bugzilla. It is safe to land this.
Updated•7 years ago
|
Priority: -- → P5
Comment 3•7 years ago
|
||
This bug is still causing some tests to fail. I adjusted the patch, please take a look. For mips64 this was fixed in Bug 1389401 by using second scratch register but actually the asserts should be removed because if from and to operands are memory addresses then neither can be a scratch register.
Attachment #8904527 -
Flags: review?(lhansen)
Attachment #8904527 -
Flags: checkin+
Comment 4•7 years ago
|
||
Also the other patch is now obsolete, but I cannot change it's status.
Updated•7 years ago
|
Attachment #8839469 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8904527 -
Flags: review?(lhansen)
Attachment #8904527 -
Flags: review+
Attachment #8904527 -
Flags: checkin+
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5412efabadb MIPS: Remove redundant asserts. r=lth
Keywords: checkin-needed
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c5412efabadb
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•