Bug 832085 - Never bail in truncated LModI/LModPowTwoI/LModMaskI (r=hv1989)
authorLuke Wagner <luke@mozilla.com>
Fri, 07 Dec 2012 18:54:05 -0800
changeset 119333 5bd1808505507f64cdcf29cdd564f701c91e876d
parent 119332 5e41d6eb4d74d60fa321c2af9ed5ecfca30d6fce
child 119334 2674257f6117e3d03c67aa02a3ca84a7720ded40
push id21615
push userlwagner@mozilla.com
push dateSat, 19 Jan 2013 01:31:32 +0000
treeherdermozilla-inbound@2674257f6117 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershv1989
bugs832085
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 832085 - Never bail in truncated LModI/LModPowTwoI/LModMaskI (r=hv1989)
js/src/ion/MIR.cpp
js/src/ion/MIR.h
js/src/ion/arm/CodeGenerator-arm.cpp
js/src/ion/arm/LIR-arm.h
js/src/ion/arm/Lowering-arm.cpp
js/src/ion/shared/CodeGenerator-x86-shared.cpp
js/src/ion/shared/LIR-x86-shared.h
js/src/ion/shared/Lowering-x86-shared.cpp
js/src/jit-test/tests/basic/testTruncatedMod.js
--- a/js/src/ion/MIR.cpp
+++ b/js/src/ion/MIR.cpp
@@ -859,16 +859,41 @@ MMod::foldsTo(bool useValueNumbers)
 
     if (MDefinition *folded = EvaluateConstantOperands(this))
         return folded;
 
     return this;
 }
 
 void
+MMod::analyzeTruncateBackward()
+{
+    if (!isTruncated())
+        setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
+}
+
+bool
+MMod::updateForReplacement(MDefinition *ins_)
+{
+    JS_ASSERT(ins_->isMod());
+    MMod *ins = ins_->toMod();
+    if (isTruncated() && ins->isTruncated())
+        setTruncated(Max(isTruncated(), ins->isTruncated()));
+    else
+        setTruncated(0);
+    return true;
+}
+
+bool
+MMod::fallible()
+{
+    return !isTruncated();
+}
+
+void
 MAdd::analyzeTruncateBackward()
 {
     if (!isTruncated()) {
         setTruncated(js::ion::EdgeCaseAnalysis::AllUsesTruncate(this));
     }
     if (isTruncated() && isTruncated() < 20) {
         // Super obvious optimization... If this operation is a double
         // BUT it happens to look like a large precision int that eventually
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -2809,35 +2809,49 @@ class MDiv : public MBinaryArithInstruct
     }
 
     bool updateForReplacement(MDefinition *ins);
     bool fallible();
 };
 
 class MMod : public MBinaryArithInstruction
 {
+    int implicitTruncate_;
+
     MMod(MDefinition *left, MDefinition *right)
-      : MBinaryArithInstruction(left, right)
+      : MBinaryArithInstruction(left, right),
+        implicitTruncate_(0)
     {
         setResultType(MIRType_Value);
     }
 
   public:
     INSTRUCTION_HEADER(Mod)
     static MMod *New(MDefinition *left, MDefinition *right) {
         return new MMod(left, right);
     }
 
     MDefinition *foldsTo(bool useValueNumbers);
+    void analyzeTruncateBackward();
+
     double getIdentity() {
         JS_NOT_REACHED("not used");
         return 1;
     }
 
+    int isTruncated() const {
+        return implicitTruncate_;
+    }
+    void setTruncated(int truncate) {
+        implicitTruncate_ = truncate;
+    }
+
+    bool updateForReplacement(MDefinition *ins);
     void computeRange();
+    bool fallible();
 };
 
 class MConcat
   : public MBinaryInstruction,
     public BinaryStringPolicy
 {
     MConcat(MDefinition *left, MDefinition *right)
       : MBinaryInstruction(left, right)
--- a/js/src/ion/arm/CodeGenerator-arm.cpp
+++ b/js/src/ion/arm/CodeGenerator-arm.cpp
@@ -571,85 +571,123 @@ CodeGeneratorARM::visitDivI(LDivI *ins)
 
 bool
 CodeGeneratorARM::visitModI(LModI *ins)
 {
     // Extract the registers from this instruction
     Register lhs = ToRegister(ins->lhs());
     Register rhs = ToRegister(ins->rhs());
     Register callTemp = ToRegister(ins->getTemp(2));
+    MMod *mir = ins->mir();
+    Label done;
     // save the lhs in case we end up with a 0 that should be a -0.0 because lhs < 0.
     JS_ASSERT(callTemp.code() > r3.code() && callTemp.code() < r12.code());
     masm.ma_mov(lhs, callTemp);
     // Prevent INT_MIN % -1;
     // The integer division will give INT_MIN, but we want -(double)INT_MIN.
     masm.ma_cmp(lhs, Imm32(INT_MIN)); // sets EQ if lhs == INT_MIN
     masm.ma_cmp(rhs, Imm32(-1), Assembler::Equal); // if EQ (LHS == INT_MIN), sets EQ if rhs == -1
-    if (!bailoutIf(Assembler::Equal, ins->snapshot()))
-        return false;
+    if (mir->isTruncated()) {
+        // (INT_MIN % -1)|0 == 0
+        Label skip;
+        masm.ma_b(&skip, Assembler::NotEqual);
+        masm.ma_mov(Imm32(0), r1);
+        masm.ma_b(&done);
+        masm.bind(&skip);
+    } else {
+        JS_ASSERT(mir->fallible());
+        if (!bailoutIf(Assembler::Equal, ins->snapshot()))
+            return false;
+    }
     // 0/X (with X < 0) is bad because both of these values *should* be doubles, and
     // the result should be -0.0, which cannot be represented in integers.
     // X/0 is bad because it will give garbage (or abort), when it should give
     // either \infty, -\infty or NAN.
 
     // Prevent 0 / X (with X < 0) and X / 0
     // testing X / Y.  Compare Y with 0.
     // There are three cases: (Y < 0), (Y == 0) and (Y > 0)
     // If (Y < 0), then we compare X with 0, and bail if X == 0
     // If (Y == 0), then we simply want to bail.  Since this does not set
     // the flags necessary for LT to trigger, we don't test X, and take the
     // bailout because the EQ flag is set.
     // if (Y > 0), we don't set EQ, and we don't trigger LT, so we don't take the bailout.
     masm.ma_cmp(rhs, Imm32(0));
     masm.ma_cmp(lhs, Imm32(0), Assembler::LessThan);
-    if (!bailoutIf(Assembler::Equal, ins->snapshot()))
-        return false;
+    if (mir->isTruncated()) {
+        // NaN|0 == 0 and (0 % -X)|0 == 0
+        Label skip;
+        masm.ma_b(&skip, Assembler::NotEqual);
+        masm.ma_mov(Imm32(0), r1);
+        masm.ma_b(&done);
+        masm.bind(&skip);
+    } else {
+        JS_ASSERT(mir->fallible());
+        if (!bailoutIf(Assembler::Equal, ins->snapshot()))
+            return false;
+    }
     masm.setupAlignedABICall(2);
     masm.passABIArg(lhs);
     masm.passABIArg(rhs);
     masm.callWithABI(JS_FUNC_TO_DATA_PTR(void *, __aeabi_idivmod));
     // If X%Y == 0 and X < 0, then we *actually* wanted to return -0.0
-    Label join;
     // See if X < 0
     masm.ma_cmp(r1, Imm32(0));
-    masm.ma_b(&join, Assembler::NotEqual);
-    masm.ma_cmp(callTemp, Imm32(0));
-    if (!bailoutIf(Assembler::Signed, ins->snapshot()))
-        return false;
-    masm.bind(&join);
+    if (mir->isTruncated()) {
+        // -0.0|0 == 0
+    } else {
+        JS_ASSERT(mir->fallible());
+        masm.ma_b(&done, Assembler::NotEqual);
+        masm.ma_cmp(callTemp, Imm32(0));
+        if (!bailoutIf(Assembler::Signed, ins->snapshot()))
+            return false;
+    }
+    masm.bind(&done);
     return true;
 }
 
 bool
 CodeGeneratorARM::visitModPowTwoI(LModPowTwoI *ins)
 {
     Register in = ToRegister(ins->getOperand(0));
     Register out = ToRegister(ins->getDef(0));
+    MMod *mir = ins->mir();
     Label fin;
     // bug 739870, jbramley has a different sequence that may help with speed here
     masm.ma_mov(in, out, SetCond);
     masm.ma_b(&fin, Assembler::Zero);
     masm.ma_rsb(Imm32(0), out, NoSetCond, Assembler::Signed);
     masm.ma_and(Imm32((1<<ins->shift())-1), out);
     masm.ma_rsb(Imm32(0), out, SetCond, Assembler::Signed);
-    if (!bailoutIf(Assembler::Zero, ins->snapshot()))
-        return false;
+    if (!mir->isTruncated()) {
+        JS_ASSERT(mir->fallible());
+        if (!bailoutIf(Assembler::Zero, ins->snapshot()))
+            return false;
+    } else {
+        // -0|0 == 0
+    }
     masm.bind(&fin);
     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));
+    MMod *mir = ins->mir();
     masm.ma_mod_mask(src, dest, tmp, ins->shift());
-    if (!bailoutIf(Assembler::Zero, ins->snapshot()))
-        return false;
+    if (!mir->isTruncated()) {
+        JS_ASSERT(mir->fallible());
+        if (!bailoutIf(Assembler::Zero, ins->snapshot()))
+            return false;
+    } else {
+        // -0|0 == 0
+    }
     return true;
 }
 bool
 CodeGeneratorARM::visitBitNotI(LBitNotI *ins)
 {
     const LAllocation *input = ins->getOperand(0);
     const LDefinition *dest = ins->getDef(0);
     // this will not actually be true on arm.
--- a/js/src/ion/arm/LIR-arm.h
+++ b/js/src/ion/arm/LIR-arm.h
@@ -118,16 +118,20 @@ class LModI : public LBinaryMath<3>
           const LDefinition &callTemp)
     {
         setOperand(0, lhs);
         setOperand(1, rhs);
         setTemp(0, temp1);
         setTemp(1, temp2);
         setTemp(2, callTemp);
     }
+
+    MMod *mir() const {
+        return mir_->toMod();
+    }
 };
 
 class LModPowTwoI : public LInstructionHelper<1, 1, 0>
 {
     const int32_t shift_;
 
   public:
     LIR_HEADER(ModPowTwoI);
@@ -136,16 +140,20 @@ class LModPowTwoI : public LInstructionH
         return shift_;
     }
 
     LModPowTwoI(const LAllocation &lhs, int32_t shift)
       : shift_(shift)
     {
         setOperand(0, lhs);
     }
+
+    MMod *mir() const {
+        return mir_->toMod();
+    }
 };
 
 class LModMaskI : public LInstructionHelper<1, 1, 1>
 {
     const int32_t shift_;
 
   public:
     LIR_HEADER(ModMaskI);
@@ -155,16 +163,20 @@ class LModMaskI : public LInstructionHel
     {
         setOperand(0, lhs);
         setTemp(0, temp1);
     }
 
     int32_t shift() const {
         return shift_;
     }
+
+    MMod *mir() const {
+        return mir_->toMod();
+    }
 };
 
 class LPowHalfD : public LInstructionHelper<1, 1, 0>
 {
   public:
     LIR_HEADER(PowHalfD);
     LPowHalfD(const LAllocation &input) {
         setOperand(0, input);
--- a/js/src/ion/arm/Lowering-arm.cpp
+++ b/js/src/ion/arm/Lowering-arm.cpp
@@ -255,28 +255,34 @@ LIRGeneratorARM::lowerMulI(MMul *mul, MD
 
 bool
 LIRGeneratorARM::lowerModI(MMod *mod)
 {
     if (mod->rhs()->isConstant()) {
         int32_t rhs = mod->rhs()->toConstant()->value().toInt32();
         int32_t shift;
         JS_FLOOR_LOG2(shift, rhs);
-        if (1 << shift == rhs) {
+        if (rhs > 0 && 1 << shift == rhs) {
             LModPowTwoI *lir = new LModPowTwoI(useRegister(mod->lhs()), shift);
-            return (assignSnapshot(lir) && define(lir, mod));
+            if (mod->fallible() && !assignSnapshot(lir))
+                return false;
+            return define(lir, mod);
         } else if (shift < 31 && (1 << (shift+1)) - 1 == rhs) {
             LModMaskI *lir = new LModMaskI(useRegister(mod->lhs()), temp(LDefinition::GENERAL), shift+1);
-            return (assignSnapshot(lir) && define(lir, mod));
+            if (mod->fallible() && !assignSnapshot(lir))
+                return false;
+            return define(lir, mod);
         }
     }
     LModI *lir = new LModI(useFixed(mod->lhs(), r0), use(mod->rhs(), r1),
                            tempFixed(r2), tempFixed(r3), temp(LDefinition::GENERAL));
 
-    return assignSnapshot(lir) && defineFixed(lir, mod, LAllocation(AnyRegister(r1)));
+    if (mod->fallible() && !assignSnapshot(lir))
+        return false;
+    return defineFixed(lir, mod, LAllocation(AnyRegister(r1)));
 }
 
 bool
 LIRGeneratorARM::visitPowHalf(MPowHalf *ins)
 {
     MDefinition *input = ins->input();
     JS_ASSERT(input->type() == MIRType_Double);
     LPowHalfD *lir = new LPowHalfD(useRegisterAtStart(input));
--- a/js/src/ion/shared/CodeGenerator-x86-shared.cpp
+++ b/js/src/ion/shared/CodeGenerator-x86-shared.cpp
@@ -124,17 +124,17 @@ CodeGeneratorX86Shared::visitTestIAndBra
 
 bool
 CodeGeneratorX86Shared::visitTestDAndBranch(LTestDAndBranch *test)
 {
     const LAllocation *opd = test->input();
 
     // ucomisd flags:
     //             Z  P  C
-    //            --------- 
+    //            ---------
     //      NaN    1  1  1
     //        >    0  0  0
     //        <    0  0  1
     //        =    1  0  0
     //
     // NaN is falsey, so comparing against 0 and then using the Z flag is
     // enough to determine which branch to take.
     masm.xorpd(ScratchFloatReg, ScratchFloatReg);
@@ -261,17 +261,17 @@ bool
 CodeGeneratorX86Shared::generateOutOfLineCode()
 {
     if (!CodeGeneratorShared::generateOutOfLineCode())
         return false;
 
     if (deoptLabel_) {
         // All non-table-based bailouts will go here.
         masm.bind(deoptLabel_);
-        
+
         // Push the frame size, so the handler can recover the IonScript.
         masm.push(Imm32(frameSize()));
 
         IonCompartment *ion = GetIonContext()->compartment->ionCompartment();
         IonCode *handler = ion->getGenericBailoutHandler();
 
         masm.jmp(handler->raw(), Relocation::IONCODE);
     }
@@ -749,36 +749,37 @@ CodeGeneratorX86Shared::visitDivI(LDivI 
     return true;
 }
 
 bool
 CodeGeneratorX86Shared::visitModPowTwoI(LModPowTwoI *ins)
 {
     Register lhs = ToRegister(ins->getOperand(0));
     int32_t shift = ins->shift();
-    Label negative, join;
+
+    Label negative, done;
     // Switch based on sign of the lhs.
     // Positive numbers are just a bitmask
     masm.branchTest32(Assembler::Signed, lhs, lhs, &negative);
     {
         masm.andl(Imm32((1 << shift) - 1), lhs);
-        masm.jump(&join);
+        masm.jump(&done);
     }
     // Negative numbers need a negate, bitmask, negate
     {
         masm.bind(&negative);
         // visitModI has an overflow check here to catch INT_MIN % -1, but
         // here the rhs is a power of 2, and cannot be -1, so the check is not generated.
         masm.negl(lhs);
         masm.andl(Imm32((1 << shift) - 1), lhs);
         masm.negl(lhs);
-        if (!bailoutIf(Assembler::Zero, ins->snapshot()))
+        if (!ins->mir()->isTruncated() && !bailoutIf(Assembler::Zero, ins->snapshot()))
             return false;
     }
-    masm.bind(&join);
+    masm.bind(&done);
     return true;
 
 }
 
 bool
 CodeGeneratorX86Shared::visitModI(LModI *ins)
 {
     Register remainder = ToRegister(ins->remainder());
@@ -790,22 +791,32 @@ CodeGeneratorX86Shared::visitModI(LModI 
     JS_ASSERT(remainder == edx);
     JS_ASSERT(temp == eax);
 
     if (lhs != temp) {
         masm.mov(lhs, temp);
         lhs = temp;
     }
 
+    Label done;
+
     // Prevent divide by zero
     masm.testl(rhs, rhs);
-    if (!bailoutIf(Assembler::Zero, ins->snapshot()))
-        return false;
+    if (ins->mir()->isTruncated()) {
+        Label notzero;
+        masm.j(Assembler::NonZero, &notzero);
+        masm.xorl(edx, edx);
+        masm.jmp(&done);
+        masm.bind(&notzero);
+    } else {
+        if (!bailoutIf(Assembler::Zero, ins->snapshot()))
+            return false;
+    }
 
-    Label negative, done;
+    Label negative;
 
     // Switch based on sign of the lhs.
     masm.branchTest32(Assembler::Signed, lhs, lhs, &negative);
     // If lhs >= 0 then remainder = lhs % rhs. The remainder must be positive.
     {
         // Since lhs >= 0, the sign-extension will be 0
         masm.xorl(edx, edx);
         masm.idiv(rhs);
@@ -816,27 +827,35 @@ CodeGeneratorX86Shared::visitModI(LModI 
     {
         masm.bind(&negative);
 
         // Prevent an integer overflow exception from -2147483648 % -1
         Label notmin;
         masm.cmpl(lhs, Imm32(INT32_MIN));
         masm.j(Assembler::NotEqual, &notmin);
         masm.cmpl(rhs, Imm32(-1));
-        if (!bailoutIf(Assembler::Equal, ins->snapshot()))
-            return false;
+        if (ins->mir()->isTruncated()) {
+            masm.j(Assembler::NotEqual, &notmin);
+            masm.xorl(edx, edx);
+            masm.jmp(&done);
+        } else {
+            if (!bailoutIf(Assembler::Equal, ins->snapshot()))
+                return false;
+        }
         masm.bind(&notmin);
 
         masm.cdq();
         masm.idiv(rhs);
 
-        // A remainder of 0 means that the rval must be -0, which is a double.
-        masm.testl(remainder, remainder);
-        if (!bailoutIf(Assembler::Zero, ins->snapshot()))
-            return false;
+        if (!ins->mir()->isTruncated()) {
+            // A remainder of 0 means that the rval must be -0, which is a double.
+            masm.testl(remainder, remainder);
+            if (!bailoutIf(Assembler::Zero, ins->snapshot()))
+                return false;
+        }
     }
 
     masm.bind(&done);
     return true;
 }
 
 bool
 CodeGeneratorX86Shared::visitBitNotI(LBitNotI *ins)
@@ -1111,17 +1130,17 @@ CodeGeneratorX86Shared::visitFloor(LFloo
         masm.bind(&negative);
         {
             // Truncate and round toward zero.
             // This is off-by-one for everything but integer-valued inputs.
             masm.cvttsd2si(input, output);
             masm.cmp32(output, Imm32(INT_MIN));
             if (!bailoutIf(Assembler::Equal, lir->snapshot()))
                 return false;
-        
+
             // Test whether the input double was integer-valued.
             masm.cvtsi2sd(output, scratch);
             masm.branchDouble(Assembler::DoubleEqualOrUnordered, input, scratch, &end);
 
             // Input is not integer-valued, so we rounded off-by-one in the
             // wrong direction. Correct by subtraction.
             masm.subl(Imm32(1), output);
             // Cannot overflow: output was already checked against INT_MIN.
--- a/js/src/ion/shared/LIR-x86-shared.h
+++ b/js/src/ion/shared/LIR-x86-shared.h
@@ -39,16 +39,19 @@ class LModI : public LBinaryMath<1>
         setOperand(0, lhs);
         setOperand(1, rhs);
         setTemp(0, temp);
     }
 
     const LDefinition *remainder() {
         return getDef(0);
     }
+    MMod *mir() const {
+        return mir_->toMod();
+    }
 };
 
 class LModPowTwoI : public LInstructionHelper<1,1,0>
 {
     const int32_t shift_;
 
   public:
     LIR_HEADER(ModPowTwoI)
@@ -60,16 +63,19 @@ class LModPowTwoI : public LInstructionH
     }
 
     int32_t shift() const {
         return shift_;
     }
     const LDefinition *remainder() {
         return getDef(0);
     }
+    MMod *mir() const {
+        return mir_->toMod();
+    }
 };
 
 // Double raised to a half power.
 class LPowHalfD : public LInstructionHelper<1, 1, 1>
 {
   public:
     LIR_HEADER(PowHalfD)
     LPowHalfD(const LAllocation &input, const LDefinition &temp) {
--- a/js/src/ion/shared/Lowering-x86-shared.cpp
+++ b/js/src/ion/shared/Lowering-x86-shared.cpp
@@ -87,23 +87,27 @@ LIRGeneratorX86Shared::lowerDivI(MDiv *d
 
 bool
 LIRGeneratorX86Shared::lowerModI(MMod *mod)
 {
     if (mod->rhs()->isConstant()) {
         int32_t rhs = mod->rhs()->toConstant()->value().toInt32();
         int32_t shift;
         JS_FLOOR_LOG2(shift, rhs);
-        if (1 << shift == rhs) {
+        if (rhs > 0 && 1 << shift == rhs) {
             LModPowTwoI *lir = new LModPowTwoI(useRegisterAtStart(mod->lhs()), shift);
-            return assignSnapshot(lir) && defineReuseInput(lir, mod, 0);
+            if (mod->fallible() && !assignSnapshot(lir))
+                return false;
+            return defineReuseInput(lir, mod, 0);
         }
     }
     LModI *lir = new LModI(useRegister(mod->lhs()), useRegister(mod->rhs()), tempFixed(eax));
-    return assignSnapshot(lir) && defineFixed(lir, mod, LAllocation(AnyRegister(edx)));
+    if (mod->fallible() && !assignSnapshot(lir))
+        return false;
+    return defineFixed(lir, mod, LAllocation(AnyRegister(edx)));
 }
 
 bool
 LIRGeneratorX86Shared::lowerUrshD(MUrsh *mir)
 {
     MDefinition *lhs = mir->lhs();
     MDefinition *rhs = mir->rhs();
 
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/testTruncatedMod.js
@@ -0,0 +1,18 @@
+var f = function(i,j) { return i%j|0 };
+assertEq(f(10, 0), 0);
+var f = function(i,j) { return i%j|0 };
+assertEq(f(0, 0), 0);
+var f = function(i,j) { return i%j|0 };
+assertEq(f(-Math.pow(2,31), -1), 0);
+var f = function(i,j) { return i%j|0 };
+assertEq(f(-4, 4), 0);
+
+var f = function(i) { return i%4|0 };
+assertEq(f(-4), 0);
+var f = function(i) { return i%4|0 };
+assertEq(f(0), 0);
+
+var f = function(i) { return i%3|0 };
+assertEq(f(-3), 0);
+var f = function(i) { return i%3|0 };
+assertEq(f(0), 0);