Bug 864400 - Optimize ModI for non-constant power-of-2 divisors. r=h4writer

js/src/ion/MIR.cpp

@@ -339,12 +339,6 @@ MDefinition::emptyResultTypeSet() const
     return resultTypeSet() && resultTypeSet()->empty();
 }
 
-static inline bool
-IsPowerOfTwo(uint32_t n)
-{
-    return (n > 0) && ((n & (n - 1)) == 0);
-}
-
 MConstant *
 MConstant::New(const Value &v)
 {
@@ -1175,6 +1169,35 @@ MDiv::fallible()
     return !isTruncated();
 }
 
+bool
+MMod::canBeDivideByZero() const
+{
+    JS_ASSERT(specialization_ == MIRType_Int32);
+    return !rhs()->isConstant() || rhs()->toConstant()->value().toInt32() == 0;
+}
+
+bool
+MMod::canBeNegativeDividend() const
+{
+    JS_ASSERT(specialization_ == MIRType_Int32);
+    return !lhs()->range() || lhs()->range()->lower() < 0;
+}
+
+bool
+MMod::canBePowerOfTwoDivisor() const
+{
+    JS_ASSERT(specialization_ == MIRType_Int32);
+
+    if (!rhs()->isConstant())
+        return true;
+
+    int32_t i = rhs()->toConstant()->value().toInt32();
+    if (i <= 0 || !IsPowerOfTwo(i))
+        return false;
+
+    return true;
+}
+
 static inline MDefinition *
 TryFold(MDefinition *original, MDefinition *replacement)
 {

js/src/ion/MIR.h

@@ -3499,6 +3499,10 @@ class MMod : public MBinaryArithInstruction
         MOZ_ASSUME_UNREACHABLE("not used");
     }
 
+    bool canBeNegativeDividend() const;
+    bool canBeDivideByZero() const;
+    bool canBePowerOfTwoDivisor() const;
+
     bool fallible();
 
     void computeRange();

js/src/ion/arm/CodeGenerator-arm.cpp

@@ -651,22 +651,26 @@ CodeGeneratorARM::visitModI(LModI *ins)
     // 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 (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;
+    if (mir->canBeNegativeDividend()) {
+        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 (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

js/src/ion/shared/CodeGenerator-x86-shared.cpp

@@ -835,33 +835,59 @@ CodeGeneratorX86Shared::visitModI(LModI *ins)
 
     Label done;
 
-    // Prevent divide by zero
-    masm.testl(rhs, rhs);
-    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;
+    // Prevent divide by zero.
+    if (ins->mir()->canBeDivideByZero()) {
+        masm.testl(rhs, rhs);
+        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;
 
     // Switch based on sign of the lhs.
-    masm.branchTest32(Assembler::Signed, lhs, lhs, &negative);
+    if (ins->mir()->canBeNegativeDividend())
+        masm.branchTest32(Assembler::Signed, lhs, lhs, &negative);
+
     // If lhs >= 0 then remainder = lhs % rhs. The remainder must be positive.
     {
+        // Check if rhs is a power-of-two.
+        if (ins->mir()->canBePowerOfTwoDivisor()) {
+            JS_ASSERT(rhs != remainder);
+
+            // Rhs y is a power-of-two if (y & (y-1)) == 0. Note that if
+            // y is any negative number other than INT32_MIN, both y and
+            // y-1 will have the sign bit set so these are never optimized
+            // as powers-of-two. If y is INT32_MIN, y-1 will be INT32_MAX
+            // and because lhs >= 0 at this point, lhs & INT32_MAX returns
+            // the correct value.
+            Label notPowerOfTwo;
+            masm.mov(rhs, remainder);
+            masm.subl(Imm32(1), remainder);
+            masm.branchTest32(Assembler::NonZero, remainder, rhs, &notPowerOfTwo);
+            {
+                masm.andl(lhs, remainder);
+                masm.jmp(&done);
+            }
+            masm.bind(&notPowerOfTwo);
+        }
+
         // Since lhs >= 0, the sign-extension will be 0
         masm.xorl(edx, edx);
         masm.idiv(rhs);
-        masm.jump(&done);
     }
 
     // Otherwise, we have to beware of two special cases:
-    {
+    if (ins->mir()->canBeNegativeDividend()) {
+        masm.jump(&done);
+
         masm.bind(&negative);
 
         // Prevent an integer overflow exception from -2147483648 % -1