Closed Bug 713571 Opened 12 years ago Closed 12 years ago

IonMonkey: Compile JSOP_PICK & JSOP_SWAP

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: nbp, Assigned: nbp)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

We already have some compilation failures caused only by JSOP_PICK & JSOP_SWAP.  JSOP_SWAP.

Both need to manipulate the StackSlot of MBasicBloc while taking care to reorganized the linked list embedded in the extra fields of the StackSlot.
Attached patch Implement JSOP_PICK & JSOP_SWAP. (obsolete) — Splinter Review
Attachment #584351 - Flags: review?(dvander)
Comment on attachment 584351 [details] [diff] [review]
Implement JSOP_PICK & JSOP_SWAP.

I'd prefer if we could solve this using existing stack transition methods, since this update code looks fairly complicated and in the past has been error-prone.

SWAP is easy: DUP2, store top to -3, pop, store top to -1, pop
PICK can be done with similar motions - see methodjit/Compiler.cpp (FrameState is the spiritual predecessor to MBasicBlock.)
Attachment #584351 - Flags: review?(dvander)
Simplify jsop_pick by adding swapAt function, which care only about 2 elements at
a specified depth.
Attachment #584351 - Attachment is obsolete: true
Attachment #585593 - Flags: review?(dvander)
Comment on attachment 585593 [details] [diff] [review]
Implement JSOP_PICK & JSOP_SWAP.

Review of attachment 585593 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ion/MIRGraph.cpp
@@ +452,5 @@
> +    JS_ASSERT(lhsDepth >= info_.firstStackSlot());
> +
> +    StackSlot &lhs = slots_[lhsDepth];
> +    StackSlot &rhs = slots_[rhsDepth];
> +    StackSlot tmp;

This |tmp| declaration should be moved down to its first use,

@@ +467,5 @@
> +    updateIndexes(lhs, lhsDepth, rhsDepth);
> +    updateIndexes(rhs, rhsDepth, lhsDepth);
> +
> +    // Swap values.
> +    tmp = lhs;

... here.
Attachment #585593 - Flags: review?(dvander) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/2ed7cba609d7
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.