Closed
Bug 674334
Opened 13 years ago
Closed 13 years ago
IonMonkey: Failures on optimized builds and on large-stack scripts
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: adrake, Assigned: adrake)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
7.61 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
This fixes: 1) Off by one in linear scan reifier where the "to" half of a move would be included in the freeRegs set. 2) Inactive intervals were removed from the active list (I don't think this actually breaks anything due to the way InlineList is implemented). 3) The 'handled' list was #ifdef DEBUG, and is now needed for the algorithm proper. 4) The code generator did not properly take into account fixed frame sizes when computing stack slot offsets. 5) The code generator swapped Move::GENERAL and Move::DOUBLE at move creation time. 6) MoveEmitterX86::toOperand did not handle register operands, thus register to register moves could not be emitted. 7) The code generator called emitDoubleMove for integer register moves, and vice-versa.
Attachment #548563 -
Flags: review?(dvander)
Assignee | ||
Comment 1•13 years ago
|
||
Removes spurious whitespace insertion that didn't get qrefreshed before I hit submit on the initial report.
Assignee: general → adrake
Attachment #548563 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #548563 -
Flags: review?(dvander)
Attachment #548564 -
Flags: review?(dvander)
Comment on attachment 548564 [details] [diff] [review] Patch v1 Review of attachment 548564 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for finding and fixing these! ::: js/src/ion/shared/CodeGenerator-shared.h @@ +79,5 @@ > } > > inline int32 SlotToStackOffset(int32 slot) { > JS_ASSERT(slot >= 0 && slot <= int32(graph.localSlotCount())); > + int32 offset = masm.framePushed() - frameStaticSize_ + slot * STACK_SLOT_SIZE; Let's put the slots at the top of the frame instead, clearly this was totally wrong before though.
Attachment #548564 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 3•13 years ago
|
||
- JS_ASSERT(slot >= 0 && slot <= int32(graph.localSlotCount())); - int32 offset = masm.framePushed() - frameStaticSize_ + slot * STACK_SLOT_SIZE; + JS_ASSERT(slot > 0 && slot <= int32(graph.localSlotCount())); + int32 offset = masm.framePushed() - slot * STACK_SLOT_SIZE; How about that?
(In reply to comment #3) > - JS_ASSERT(slot >= 0 && slot <= int32(graph.localSlotCount())); > - int32 offset = masm.framePushed() - frameStaticSize_ + slot * > STACK_SLOT_SIZE; > + JS_ASSERT(slot > 0 && slot <= int32(graph.localSlotCount())); > + int32 offset = masm.framePushed() - slot * STACK_SLOT_SIZE; > > How about that? Looks good.
Assignee | ||
Comment 5•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/07a3818d8d24
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•