Closed
Bug 1148494
Opened 9 years ago
Closed 9 years ago
MSimdUnbox should have the Guard flag set when it's fallible
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: sunfish, Assigned: bbouvier)
Details
Attachments
(1 file)
3.29 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
Currently, IonBuilder::inlineSimdCheck manually sets the Guard flag when creating an MSimdUnbox. However, EagerSimdUnbox.cpp and TypePolicy.cpp don't currently set the Guard flag for their SIMD unbox operations, even though they're still supposed to throw if their operand is not actually SIMD. It seems that MSimdUnbox should set he Guard flag in its constructor as most other classes that use it do, since looking at the CodeGenerator code for LSimdUnbox, it appears that all LSimdUnbox instances are fallible.
Assignee | ||
Comment 1•9 years ago
|
||
If anybody wants to take it, feel free, otherwise i'll do it when i'm back from pto.
Flags: needinfo?(benj)
Assignee | ||
Comment 2•9 years ago
|
||
So the patch is easy, but finding a test case that triggers the issue without the patch isn't...
Assignee: nobody → benj
Status: NEW → ASSIGNED
Flags: needinfo?(benj)
Assignee | ||
Comment 3•9 years ago
|
||
With a test case that triggers on inbound but not with this patch! (it was hard to trigger, as SIMD values are objects and thus bailouts could be produced by typebarriers or arguments checks)
Attachment #8592278 -
Flags: review?(sunfish)
Reporter | ||
Comment 4•9 years ago
|
||
Comment on attachment 8592278 [details] [diff] [review] unbox-guard.patch Review of attachment 8592278 [details] [diff] [review]: ----------------------------------------------------------------- Nice job finding a suitably obstreperous testcase :-).
Attachment #8592278 -
Flags: review?(sunfish) → review+
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1fd18912545
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d1fd18912545
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•