Closed
Bug 691597
Opened 13 years ago
Closed 13 years ago
IonMonkey: IonBuilder's jsop_neg() assumes unguaranteed slot availability.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: decoder, Unassigned)
Details
(Keywords: assertion, testcase)
Attachments
(1 file)
2.09 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
The following testcase asserts on ionmonkey revision acf3c1fb7c94 (run with --ion-eager), tested on 64 bit: function f() { -null; } f();
Comment 1•13 years ago
|
||
This one is actually pretty good. The problem is that jsop_neg() is implemented by pushing MConstant(-1) followed by an MMul, but there is no slot in the MBasicBlock into which the MConstant can be pushed so the MMul can consume it.
Comment 2•13 years ago
|
||
This changes jsop_neg() to not assume that there exists an extra slot which it can use for temporarily pushing MConstant(-1). An alternate solution is to just give IonBuilder's nslots "just one additional slot for extra temporary padding", but it seems tremendously irritating to detect these errors if we adopt that solution, especially if they nest.
Attachment #564416 -
Flags: review?(dvander)
Updated•13 years ago
|
Summary: IM: Assertion failure: stackPosition_ < gen()->nslots(), at ion/MIRGraph.cpp:314 → IonMonkey: IonBuilder's jsop_neg() assumes unguaranteed slot availability.
Comment on attachment 564416 [details] [diff] [review] Refactor jsop_neg() to not assume extra slot availability. What about adding JOF_TMPSLOT to JSOP_NEG in jsopcode.tbl?
Comment 4•13 years ago
|
||
(In reply to David Anderson [:dvander] from comment #3) > What about adding JOF_TMPSLOT to JSOP_NEG in jsopcode.tbl? It seems dubious to change the opcode to support a quirk of a JIT when it is simple to match that JIT to interpreter behavior.
Comment on attachment 564416 [details] [diff] [review] Refactor jsop_neg() to not assume extra slot availability. FWIW, JOF_TMPSLOT exists to handle these quirks. Pre-conservative GC, it allowed ops to decompose in the interpreter while keeping their temporary values rooted. In JM (where it is used frequently) it allows similar decomposition. >-IonBuilder::jsop_binary(JSOp op) >+IonBuilder::jsop_binary_explicit(JSOp op, MDefinition *left, MDefinition *right) I would just overload jsop_binary, "_explicit" feels weird.
Attachment #564416 -
Flags: review?(dvander) → review+
Comment 6•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/578219c4f056
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 7•11 years ago
|
||
A testcase for this bug was automatically identified at js/src/jit-test/tests/ion/bug691597.js.
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•