From f8b227ecf0a0ffb0b003817dfc2cacfa3ceaab92 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Tue, 9 Jul 2013 10:23:58 +0200 Subject: [PATCH] Bug 864400 - Optimize ModI for non-constant power-of-2 divisors. r=h4writer --- js/src/ion/MIR.cpp | 35 +++++++++--- js/src/ion/MIR.h | 4 ++ js/src/ion/arm/CodeGenerator-arm.cpp | 30 ++++++----- .../ion/shared/CodeGenerator-x86-shared.cpp | 54 ++++++++++++++----- 4 files changed, 90 insertions(+), 33 deletions(-) diff --git a/js/src/ion/MIR.cpp b/js/src/ion/MIR.cpp index 9af0653a2463..eb1b825a3541 100644 --- a/js/src/ion/MIR.cpp +++ b/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) { diff --git a/js/src/ion/MIR.h b/js/src/ion/MIR.h index 99d88f2d10e1..d666b7e9c39d 100644 --- a/js/src/ion/MIR.h +++ b/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(); diff --git a/js/src/ion/arm/CodeGenerator-arm.cpp b/js/src/ion/arm/CodeGenerator-arm.cpp index 31a2cdffc76d..b07f89d4cea9 100644 --- a/js/src/ion/arm/CodeGenerator-arm.cpp +++ b/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 diff --git a/js/src/ion/shared/CodeGenerator-x86-shared.cpp b/js/src/ion/shared/CodeGenerator-x86-shared.cpp index 33838c67d7fa..e74d6db8e1a6 100644 --- a/js/src/ion/shared/CodeGenerator-x86-shared.cpp +++ b/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, ¬zero); - masm.xorl(edx, edx); - masm.jmp(&done); - masm.bind(¬zero); - } 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, ¬zero); + masm.xorl(edx, edx); + masm.jmp(&done); + masm.bind(¬zero); + } 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, ¬PowerOfTwo); + { + masm.andl(lhs, remainder); + masm.jmp(&done); + } + masm.bind(¬PowerOfTwo); + } + // 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