Bug 1027359 - Fix incorrect codegen in ma_mod_mask. r=mjrosenb, a=lmandel
authorShu-yu Guo <shu@rfrn.org>
Mon, 30 Jun 2014 14:01:24 -0700
changeset 209034 65af5e10bfe45c0cbcaf412ab5ccf60dde81bc1d
parent 209033 0f59e93abd3653482686bce4d5b82adaa8b65d79
child 209035 a3dce4eedb9a35fa82199b84d75bdb01d3cc1f04
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjrosenb, lmandel
bugs1027359
milestone32.0a2
Bug 1027359 - Fix incorrect codegen in ma_mod_mask. r=mjrosenb, a=lmandel
js/src/jit/arm/CodeGenerator-arm.cpp
js/src/jit/arm/LIR-arm.h
js/src/jit/arm/Lowering-arm.cpp
js/src/jit/arm/MacroAssembler-arm.cpp
js/src/jit/arm/MacroAssembler-arm.h
--- a/js/src/jit/arm/CodeGenerator-arm.cpp
+++ b/js/src/jit/arm/CodeGenerator-arm.cpp
@@ -853,19 +853,20 @@ CodeGeneratorARM::visitModPowTwoI(LModPo
     return true;
 }
 
 bool
 CodeGeneratorARM::visitModMaskI(LModMaskI *ins)
 {
     Register src = ToRegister(ins->getOperand(0));
     Register dest = ToRegister(ins->getDef(0));
-    Register tmp = ToRegister(ins->getTemp(0));
+    Register tmp1 = ToRegister(ins->getTemp(0));
+    Register tmp2 = ToRegister(ins->getTemp(1));
     MMod *mir = ins->mir();
-    masm.ma_mod_mask(src, dest, tmp, ins->shift());
+    masm.ma_mod_mask(src, dest, tmp1, tmp2, ins->shift());
     if (mir->canBeNegativeDividend()) {
         if (!mir->isTruncated()) {
             JS_ASSERT(mir->fallible());
             if (!bailoutIf(Assembler::Zero, ins->snapshot()))
                 return false;
         } else {
             // -0|0 == 0
         }
--- a/js/src/jit/arm/LIR-arm.h
+++ b/js/src/jit/arm/LIR-arm.h
@@ -258,28 +258,30 @@ class LModPowTwoI : public LInstructionH
         setOperand(0, lhs);
     }
 
     MMod *mir() const {
         return mir_->toMod();
     }
 };
 
-class LModMaskI : public LInstructionHelper<1, 1, 1>
+class LModMaskI : public LInstructionHelper<1, 1, 2>
 {
     const int32_t shift_;
 
   public:
     LIR_HEADER(ModMaskI);
 
-    LModMaskI(const LAllocation &lhs, const LDefinition &temp1, int32_t shift)
+    LModMaskI(const LAllocation &lhs, const LDefinition &temp1, const LDefinition &temp2,
+              int32_t shift)
       : shift_(shift)
     {
         setOperand(0, lhs);
         setTemp(0, temp1);
+        setTemp(1, temp2);
     }
 
     int32_t shift() const {
         return shift_;
     }
 
     MMod *mir() const {
         return mir_->toMod();
--- a/js/src/jit/arm/Lowering-arm.cpp
+++ b/js/src/jit/arm/Lowering-arm.cpp
@@ -316,17 +316,17 @@ LIRGeneratorARM::lowerModI(MMod *mod)
         int32_t rhs = mod->rhs()->toConstant()->value().toInt32();
         int32_t shift = FloorLog2(rhs);
         if (rhs > 0 && 1 << shift == rhs) {
             LModPowTwoI *lir = new(alloc()) LModPowTwoI(useRegister(mod->lhs()), shift);
             if (mod->fallible() && !assignSnapshot(lir, Bailout_BaselineInfo))
                 return false;
             return define(lir, mod);
         } else if (shift < 31 && (1 << (shift+1)) - 1 == rhs) {
-            LModMaskI *lir = new(alloc()) LModMaskI(useRegister(mod->lhs()), temp(LDefinition::GENERAL), shift+1);
+            LModMaskI *lir = new(alloc()) LModMaskI(useRegister(mod->lhs()), temp(), temp(), shift+1);
             if (mod->fallible() && !assignSnapshot(lir, Bailout_BaselineInfo))
                 return false;
             return define(lir, mod);
         }
     }
 
     if (hasIDIV()) {
         LModI *lir = new(alloc()) LModI(useRegister(mod->lhs()), useRegister(mod->rhs()), temp());
--- a/js/src/jit/arm/MacroAssembler-arm.cpp
+++ b/js/src/jit/arm/MacroAssembler-arm.cpp
@@ -913,17 +913,18 @@ MacroAssemblerARM::ma_check_mul(Register
         as_cmp(ScratchRegister, asr(dest, 31));
         return NotEqual;
     }
 
     MOZ_ASSUME_UNREACHABLE("Condition NYI");
 }
 
 void
-MacroAssemblerARM::ma_mod_mask(Register src, Register dest, Register hold, int32_t shift)
+MacroAssemblerARM::ma_mod_mask(Register src, Register dest, Register hold, Register tmp,
+                               int32_t shift)
 {
     // MATH:
     // We wish to compute x % (1<<y) - 1 for a known constant, y.
     // first, let b = (1<<y) and C = (1<<y)-1, then think of the 32 bit dividend as
     // a number in base b, namely c_0*1 + c_1*b + c_2*b^2 ... c_n*b^n
     // now, since both addition and multiplication commute with modulus,
     // x % C == (c_0 + c_1*b + ... + c_n*b^n) % C ==
     // (c_0 % C) + (c_1%C) * (b % C) + (c_2 % C) * (b^2 % C)...
@@ -936,38 +937,42 @@ MacroAssemblerARM::ma_mod_mask(Register 
     Label head;
 
     // hold holds -1 if the value was negative, 1 otherwise.
     // ScratchRegister holds the remaining bits that have not been processed
     // lr serves as a temporary location to store extracted bits into as well
     //    as holding the trial subtraction as a temp value
     // dest is the accumulator (and holds the final result)
 
-    // move the whole value into the scratch register, setting the codition codes so
-    // we can muck with them later
-    as_mov(ScratchRegister, O2Reg(src), SetCond);
+    // move the whole value into tmp, setting the codition codes so we can
+    // muck with them later.
+    //
+    // Note that we cannot use ScratchRegister in place of tmp here, as ma_and
+    // below on certain architectures move the mask into ScratchRegister
+    // before performing the bitwise and.
+    as_mov(tmp, O2Reg(src), SetCond);
     // Zero out the dest.
     ma_mov(Imm32(0), dest);
     // Set the hold appropriately.
     ma_mov(Imm32(1), hold);
     ma_mov(Imm32(-1), hold, NoSetCond, Signed);
-    ma_rsb(Imm32(0), ScratchRegister, SetCond, Signed);
+    ma_rsb(Imm32(0), tmp, SetCond, Signed);
     // Begin the main loop.
     bind(&head);
 
     // Extract the bottom bits into lr.
-    ma_and(Imm32(mask), ScratchRegister, secondScratchReg_);
+    ma_and(Imm32(mask), tmp, secondScratchReg_);
     // Add those bits to the accumulator.
     ma_add(secondScratchReg_, dest, dest);
     // Do a trial subtraction, this is the same operation as cmp, but we store the dest
     ma_sub(dest, Imm32(mask), secondScratchReg_, SetCond);
     // If (sum - C) > 0, store sum - C back into sum, thus performing a modulus.
     ma_mov(secondScratchReg_, dest, NoSetCond, NotSigned);
     // Get rid of the bits that we extracted before, and set the condition codes
-    as_mov(ScratchRegister, lsr(ScratchRegister, shift), SetCond);
+    as_mov(tmp, lsr(tmp, shift), SetCond);
     // If the shift produced zero, finish, otherwise, continue in the loop.
     ma_b(&head, NonZero);
     // Check the hold to see if we need to negate the result.  Hold can only be 1 or -1,
     // so this will never set the 0 flag.
     ma_cmp(hold, Imm32(0));
     // If the hold was non-zero, negate the result to be in line with what JS wants
     // this will set the condition codes if we try to negate
     ma_rsb(Imm32(0), dest, SetCond, Signed);
--- a/js/src/jit/arm/MacroAssembler-arm.h
+++ b/js/src/jit/arm/MacroAssembler-arm.h
@@ -258,17 +258,18 @@ class MacroAssemblerARM : public Assembl
     // multiplies.  For now, there are only two that we care about.
     void ma_mul(Register src1, Register src2, Register dest);
     void ma_mul(Register src1, Imm32 imm, Register dest);
     Condition ma_check_mul(Register src1, Register src2, Register dest, Condition cond);
     Condition ma_check_mul(Register src1, Imm32 imm, Register dest, Condition cond);
 
     // fast mod, uses scratch registers, and thus needs to be in the assembler
     // implicitly assumes that we can overwrite dest at the beginning of the sequence
-    void ma_mod_mask(Register src, Register dest, Register hold, int32_t shift);
+    void ma_mod_mask(Register src, Register dest, Register hold, Register tmp,
+                     int32_t shift);
 
     // mod, depends on integer divide instructions being supported
     void ma_smod(Register num, Register div, Register dest);
     void ma_umod(Register num, Register div, Register dest);
 
     // division, depends on integer divide instructions being supported
     void ma_sdiv(Register num, Register div, Register dest, Condition cond = Always);
     void ma_udiv(Register num, Register div, Register dest, Condition cond = Always);