713571 - IonMonkey: Compile JSOP_PICK & JSOP_SWAP

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

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.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Implement JSOP_PICK & JSOP_SWAP.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.)

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: Implement JSOP_PICK & JSOP_SWAP.

Simplify jsop_pick by adding swapAt function, which care only about 2 elements at
a specified depth.

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

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.

Comment 5

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/projects/ionmonkey/rev/2ed7cba609d7