Bug 625141 - Avoid overwriting used register in TypedArray IC (r=dvander)

There was an edge case when creating an IC to store to typed arrays, in which
we would overwrite a register which was a component of the address. Fix by
preserving the address in that case, and restoring it after the overwrite.

This also checks that we save/restore each register only once.

js/src/assembler/assembler/X86Assembler.h

@@ -2376,7 +2376,9 @@ public:
 
     static void relinkJump(void* from, void* to)
     {
-        FIXME_INSN_PRINTING;
+        js::JaegerSpew(js::JSpew_Insns,
+                       ISPFX "##relinkJump ((from=%p)) ((to=%p))\n",
+                       from, to);
         setRel32(from, to);
     }
 
@@ -2412,7 +2414,10 @@ public:
 
     static void repatchLoadPtrToLEA(void* where)
     {
-        FIXME_INSN_PRINTING;
+        js::JaegerSpew(js::JSpew_Insns,
+                       ISPFX "##repatchLoadPtrToLEA ((where=%p))\n",
+                       where);
+
 #if WTF_CPU_X86_64
         // On x86-64 pointer memory accesses require a 64-bit operand, and as such a REX prefix.
         // Skip over the prefix byte.
@@ -2423,7 +2428,9 @@ public:
     
     static void repatchLEAToLoadPtr(void* where)
     {
-        FIXME_INSN_PRINTING;
+        js::JaegerSpew(js::JSpew_Insns,
+                       ISPFX "##repatchLEAToLoadPtr ((where=%p))\n",
+                       where);
 #if WTF_CPU_X86_64
         // On x86-64 pointer memory accesses require a 64-bit operand, and as such a REX prefix.
         // Skip over the prefix byte.

js/src/jit-test/tests/basic/bug625141-1.js

@@ -0,0 +1,13 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+function f() {
+  var arr = new Int32Array(10);
+  x = function () { return arr.length; }
+  for (var i = 0; i < arr.length; i++) {
+    arr[i] = i;
+  }
+  assertEq(arr[5], 5);
+}
+f();

js/src/jit-test/tests/basic/bug625141-2.js

@@ -0,0 +1,13 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+function f() {
+  var arr = new Int8Array(10);
+  x = function () { return arr.length; }
+  for (var i = 0; i < arr.length; i++) {
+    arr[i] = i;
+  }
+  assertEq(arr[5], 5);
+}
+f();

js/src/methodjit/FastOps.cpp

@@ -1301,7 +1301,7 @@ mjit::Compiler::jsop_setelem(bool popGuaranteed)
     // js_DoubleToECMAInt32(), which would clobber registers. To deal with
     // this, we tell the IC exactly which registers need to be saved
     // across calls.
-    ic.volatileMask = frame.regsInUse() & Registers::TempRegs;
+    ic.volatileMask = frame.regsInUse();
 
     // If the RHS will be popped, and doesn't overlap any live values, then
     // there's no need to save it across calls. Note that this is not true of

js/src/methodjit/TypedArrayIC.h

@@ -410,6 +410,7 @@ StoreToTypedArray(JSContext *cx, Assembler &masm, js::TypedArray *tarray, T addr
             return false;
 
         PreserveRegisters saveRHS(masm);
+        PreserveRegisters saveLHS(masm);
 
         // There are three tricky situations to handle:
         //   (1) The RHS needs conversion. saveMask will be stomped, and 
@@ -441,7 +442,7 @@ StoreToTypedArray(JSContext *cx, Assembler &masm, js::TypedArray *tarray, T addr
             //   - won't clobber the key, object, or RHS type regs
             //   - is temporary, but
             //   - is not in saveMask, which contains live volatile registers.
-            uint32 allowMask = Registers::TempRegs;
+            uint32 allowMask = Registers::AvailRegs;
             if (singleByte)
                 allowMask &= Registers::SingleByteRegs;
 
@@ -465,11 +466,25 @@ StoreToTypedArray(JSContext *cx, Assembler &masm, js::TypedArray *tarray, T addr
                 } else {
                     // Oh no! *All* single byte registers are pinned. This
                     // sucks. We'll swap the type and data registers in |vr|
-                    // and unswap them later. First, save both registers to
-                    // make this easier.
-                    saveRHS.preserve(Registers::mask2Regs(vr.typeReg(), vr.dataReg()));
+                    // and unswap them later.
 
-                    // Perform the swap.
+                    // If |vr|'s registers are part of the address, swapping is
+                    // going to cause problems during the store.
+                    uint32 vrRegs = Registers::mask2Regs(vr.dataReg(), vr.typeReg());
+                    uint32 lhsMask = vrRegs & Assembler::maskAddress(address);
+
+                    // We'll also need to save any of the registers which won't
+                    // be restored via |lhsMask| above.
+                    uint32 rhsMask = vrRegs & ~lhsMask;
+
+                    // Push them, but get the order right. We'll pop LHS first.
+                    saveRHS.preserve(rhsMask);
+                    saveLHS.preserve(lhsMask);
+
+                    // Don't store/restore registers if we dont have to.
+                    saveMask &= ~lhsMask;
+
+                    // Actually perform the swap.
                     masm.swap(vr.typeReg(), vr.dataReg());
                     vr = ValueRemat::FromRegisters(vr.dataReg(), vr.typeReg());
                     newReg = vr.dataReg();
@@ -491,12 +506,17 @@ StoreToTypedArray(JSContext *cx, Assembler &masm, js::TypedArray *tarray, T addr
         }
 
         GenConversionForIntArray(masm, tarray, vr, saveMask);
+
+        // Restore the registers in |address|. |GenConversionForIntArray| won't
+        // restore them because we told it not to by fiddling with |saveMask|.
+        saveLHS.restore();
+
         if (vr.isConstant())
             StoreToIntArray(masm, tarray, Imm32(vr.value().toInt32()), address);
         else
             StoreToIntArray(masm, tarray, vr.dataReg(), address);
 
-        // Note that this will also correctly restore the damage from the
+        // Note that this will finish restoring the damage from the
         // earlier register swap.
         saveRHS.restore();
         break;