Bug 739854: Remove negative zero check on MToInt32. r=dvander,mjrosenb
authorHannes Verschore <hv1989@gmail.com>
Thu, 29 Mar 2012 23:18:07 +0200
changeset 91783 2c7e9bd434808dd7c4bf4785a764f4dd8a3eea32
parent 91782 24294fe42c1942fc18fa81b8852056a4afc46382
child 91784 5824c381df954d786c302650a7b73d8822ec5e18
push id781
push userhv1989@gmail.com
push dateThu, 29 Mar 2012 21:18:55 +0000
reviewersdvander, mjrosenb
bugs739854
milestone14.0a1
Bug 739854: Remove negative zero check on MToInt32. r=dvander,mjrosenb
js/src/ion/CodeGenerator.cpp
js/src/ion/LIR-Common.h
js/src/ion/MIR.cpp
js/src/ion/MIR.h
js/src/ion/arm/CodeGenerator-arm.cpp
js/src/ion/arm/CodeGenerator-arm.h
js/src/ion/shared/CodeGenerator-x86-shared.cpp
js/src/ion/shared/CodeGenerator-x86-shared.h
js/src/jit-test/tests/ion/bug739854.js
--- a/js/src/ion/CodeGenerator.cpp
+++ b/js/src/ion/CodeGenerator.cpp
@@ -79,17 +79,17 @@ CodeGenerator::visitValueToInt32(LValueT
     Label fails;
     switch (lir->mode()) {
       case LValueToInt32::TRUNCATE:
         if (!emitTruncateDouble(temp, output))
             return false;
         break;
       default:
         JS_ASSERT(lir->mode() == LValueToInt32::NORMAL);
-        emitDoubleToInt32(temp, output, &fails);
+        emitDoubleToInt32(temp, output, &fails, lir->mir()->canBeNegativeZero());
         break;
     }
     masm.jump(&done);
 
     masm.bind(&notDouble);
 
     if (lir->mode() == LValueToInt32::NORMAL) {
         // If the value is not null, it's a string, object, or undefined,
@@ -172,17 +172,17 @@ CodeGenerator::visitInt32ToDouble(LInt32
 }
 
 bool
 CodeGenerator::visitDoubleToInt32(LDoubleToInt32 *lir)
 {
     Label fail;
     FloatRegister input = ToFloatRegister(lir->input());
     Register output = ToRegister(lir->output());
-    emitDoubleToInt32(input, output, &fail);
+    emitDoubleToInt32(input, output, &fail, lir->mir()->canBeNegativeZero());
     if (!bailoutFrom(&fail, lir->snapshot()))
         return false;
     return true;
 }
 
 bool
 CodeGenerator::visitTestVAndBranch(LTestVAndBranch *lir)
 {
--- a/js/src/ion/LIR-Common.h
+++ b/js/src/ion/LIR-Common.h
@@ -1158,16 +1158,19 @@ class LValueToInt32 : public LInstructio
         return mode_;
     }
     const LDefinition *tempFloat() {
         return getTemp(0);
     }
     const LDefinition *output() {
         return getDef(0);
     }
+    MToInt32 *mir() const {
+        return mir_->toToInt32();
+    }
 };
 
 // Convert a double to an int32.
 //   Input: floating-point register
 //   Output: 32-bit integer
 //   Bailout: if the double cannot be converted to an integer.
 class LDoubleToInt32 : public LInstructionHelper<1, 1, 0>
 {
@@ -1176,19 +1179,24 @@ class LDoubleToInt32 : public LInstructi
 
     LDoubleToInt32(const LAllocation &in) {
         setOperand(0, in);
     }
 
     const LAllocation *input() {
         return getOperand(0);
     }
+
     const LDefinition *output() {
         return getDef(0);
     }
+
+    MToInt32 *mir() const {
+        return mir_->toToInt32();
+    }
 };
 
 
 // Convert a double to a truncated int32.
 //   Input: floating-point register
 //   Output: 32-bit integer
 class LTruncateDToInt32 : public LInstructionHelper<1, 1, 1>
 {
--- a/js/src/ion/MIR.cpp
+++ b/js/src/ion/MIR.cpp
@@ -534,19 +534,19 @@ NeedNegativeZeroCheck(MDefinition *def)
     for (MUseIterator use = def->usesBegin(); use != def->usesEnd(); use++) {
         if (use->node()->isResumePoint())
             return true;
 
         MDefinition *use_def = use->node()->toDefinition();
         switch (use_def->op()) {
           case MDefinition::Op_Add: {
             // x + y gives -0, when both x and y are -0
-            // - When other operand can't produce -0 (i.e. all opcodes, except MUL/DIV)
+            // - When other operand can't produce -0 (i.e. all opcodes, except Mul/Div/ToInt32)
             //   Remove negative zero check on this operand 
-            // - When both operands can produce -0 (both MUL/DIV opcode)
+            // - When both operands can produce -0 (both Mul/Div/ToInt32 opcode)
             //   We can remove the check eagerly on this operand.
             MDefinition *operand = use_def->getOperand(0);
             if (operand == def) {
                 operand = use_def->getOperand(1);
 
                 // Don't remove check when both operands are same definition
                 // As removing it from one operand, will remove it from both.
                 if (operand == def)
@@ -558,32 +558,58 @@ NeedNegativeZeroCheck(MDefinition *def)
             if (operand->isMul()) {
                 MMul *mul = operand->toMul();
                 if (!mul->canBeNegativeZero())
                     return true;
             } else if (operand->isDiv()) {
                 MDiv *div = operand->toDiv();
                 if (!div->canBeNegativeZero())
                     return true;
+            } else if (operand->isToInt32()) {
+                MToInt32 *int32 = operand->toToInt32();
+                if (!int32->canBeNegativeZero())
+                    return true;
             } else if (operand->isPhi()) {
                 return true;
             }
             break;
           }
+          case MDefinition::Op_StoreElement:
+          case MDefinition::Op_StoreElementHole:
+          case MDefinition::Op_LoadElement:
+          case MDefinition::Op_LoadElementHole:
+          case MDefinition::Op_LoadTypedArrayElement:
+          case MDefinition::Op_LoadTypedArrayElementHole:
+          case MDefinition::Op_CharCodeAt:
           case MDefinition::Op_Mod:
           case MDefinition::Op_Sub:
-            // If the Mul/Div is the first operand. We still need the check
+            // Only allowed to remove check when definition is the second operand
             if (use_def->getOperand(0) == def)
                 return true;
+            if (use_def->numOperands() > 2) {
+                for (size_t i = 2; i < use_def->numOperands(); i++) {
+                    if (use_def->getOperand(i) == def)
+                        return true;
+                }
+            }
             break;
+          case MDefinition::Op_BoundsCheck:
+            // Only allowed to remove check when definition is the first operand
+            if (use_def->getOperand(1) == def)
+                return true;
+            break;
+          case MDefinition::Op_ToString:
+          case MDefinition::Op_FromCharCode:
+          case MDefinition::Op_TableSwitch:
           case MDefinition::Op_Compare:
           case MDefinition::Op_BitAnd:
           case MDefinition::Op_BitOr:
           case MDefinition::Op_BitXor:
           case MDefinition::Op_Abs:
+            // Always allowed to remove check. No matter which operand.
             break;
           default:
             return true;
         }
     }
     return false;
 }
 
@@ -986,16 +1012,22 @@ MDefinition *
 MToInt32::foldsTo(bool useValueNumbers)
 {
     MDefinition *input = getOperand(0);
     if (input->type() == MIRType_Int32)
         return input;
     return this;
 }
 
+void
+MToInt32::analyzeRange()
+{
+    canBeNegativeZero_ = NeedNegativeZeroCheck(this);
+}
+
 MDefinition *
 MTruncateToInt32::foldsTo(bool useValueNumbers)
 {
     MDefinition *input = getOperand(0);
     if (input->type() == MIRType_Int32)
         return input;
 
     if (input->type() == MIRType_Double && input->isConstant()) {
--- a/js/src/ion/MIR.h
+++ b/js/src/ion/MIR.h
@@ -1535,18 +1535,21 @@ class MToDouble
     }
 };
 
 // Converts a primitive (either typed or untyped) to an int32. If the input is
 // not primitive at runtime, a bailout occurs. If the input cannot be converted
 // to an int32 without loss (i.e. "5.5" or undefined) then a bailout occurs.
 class MToInt32 : public MUnaryInstruction
 {
+    bool canBeNegativeZero_;
+
     MToInt32(MDefinition *def)
-      : MUnaryInstruction(def)
+      : MUnaryInstruction(def),
+        canBeNegativeZero_(true)
     {
         setResultType(MIRType_Int32);
         setMovable();
     }
 
   public:
     INSTRUCTION_HEADER(ToInt32);
     static MToInt32 *New(MDefinition *def)
@@ -1554,20 +1557,26 @@ class MToInt32 : public MUnaryInstructio
         return new MToInt32(def);
     }
 
     MDefinition *input() const {
         return getOperand(0);
     }
 
     MDefinition *foldsTo(bool useValueNumbers);
+    void analyzeRange();
+
+    bool canBeNegativeZero() {
+        return canBeNegativeZero_;
+    }
 
     bool congruentTo(MDefinition *const &ins) const {
         return congruentIfOperandsEqual(ins);
     }
+
     AliasSet getAliasSet() const {
         return AliasSet::None();
     }
 };
 
 // Converts a value or typed input to a truncated int32, for use with bitwise
 // operations. This is an infallible ValueToECMAInt32.
 class MTruncateToInt32 : public MUnaryInstruction
--- a/js/src/ion/arm/CodeGenerator-arm.cpp
+++ b/js/src/ion/arm/CodeGenerator-arm.cpp
@@ -758,17 +758,17 @@ CodeGeneratorARM::visitTableSwitch(LTabl
         Label *defaultcase = mir->getDefault()->lir()->label();
     const LAllocation *temp;
 
     if (ins->index()->isDouble()) {
         temp = ins->tempInt();
 
         // The input is a double, so try and convert it to an integer.
         // If it does not fit in an integer, take the default case.
-        emitDoubleToInt32(ToFloatRegister(ins->index()), ToRegister(temp), defaultcase);
+        emitDoubleToInt32(ToFloatRegister(ins->index()), ToRegister(temp), defaultcase, false);
     } else {
         temp = ins->index();
     }
 
     int32 cases = mir->numCases();
     Register tempReg = ToRegister(temp);
     // Lower value with low value
     if (mir->low() != 0) {
@@ -897,34 +897,35 @@ CodeGeneratorARM::visitRound(LRound *lir
         return false;
     masm.bind(&end);
     return true;
 }
 
 // Checks whether a double is representable as a 32-bit integer. If so, the
 // integer is written to the output register. Otherwise, a bailout is taken to
 // the given snapshot. This function overwrites the scratch float register.
-bool
-CodeGeneratorARM::emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail)
+void
+CodeGeneratorARM::emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail, bool negativeZeroCheck)
 {
     // convert the floating point value to an integer, if it did not fit,
     //     then when we convert it *back* to  a float, it will have a
     //     different value, which we can test.
     masm.ma_vcvt_F64_I32(src, ScratchFloatReg);
     // move the value into the dest register.
     masm.ma_vxfer(ScratchFloatReg, dest);
     masm.ma_vcvt_I32_F64(ScratchFloatReg, ScratchFloatReg);
     masm.ma_vcmp(src, ScratchFloatReg);
     masm.as_vmrs(pc);
     masm.ma_b(fail, Assembler::VFP_NotEqualOrUnordered);
     // If they're equal, test for 0.  It would be nicer to test for -0.0 explicitly, but that seems hard.
-    masm.ma_cmp(dest, Imm32(0));
-    masm.ma_b(fail, Assembler::Equal);
-    // guard for /= 0.
-    return true;
+    if (negativeZeroCheck) {
+        masm.ma_cmp(dest, Imm32(0));
+        masm.ma_b(fail, Assembler::Equal);
+        // guard for != 0.
+    }
 }
 
 void
 CodeGeneratorARM::emitRoundDouble(const FloatRegister &src, const Register &dest, Label *fail)
 {
     masm.ma_vcvt_F64_I32(src, ScratchFloatReg);
     masm.ma_vxfer(ScratchFloatReg, dest);
     masm.ma_cmp(dest, Imm32(0x7fffffff));
--- a/js/src/ion/arm/CodeGenerator-arm.h
+++ b/js/src/ion/arm/CodeGenerator-arm.h
@@ -82,17 +82,17 @@ class CodeGeneratorARM : public CodeGene
     bool bailoutIf(Assembler::Condition condition, LSnapshot *snapshot);
     bool bailoutFrom(Label *label, LSnapshot *snapshot);
 
   protected:
     bool generatePrologue();
     bool generateEpilogue();
     bool generateOutOfLineCode();
 
-    bool emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail);
+    void emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail, bool negativeZeroCheck = true);
     void emitRoundDouble(const FloatRegister &src, const Register &dest, Label *fail);
 
     // Emits a conditional set.
     void emitSet(Assembler::Condition cond, const Register &dest);
 
     // Emits a branch that directs control flow to the true block if |cond| is
     // true, and the false block if |cond| is false.
     void emitBranch(Assembler::Condition cond, MBasicBlock *ifTrue, MBasicBlock *ifFalse);
--- a/js/src/ion/shared/CodeGenerator-x86-shared.cpp
+++ b/js/src/ion/shared/CodeGenerator-x86-shared.cpp
@@ -798,17 +798,17 @@ CodeGeneratorX86Shared::visitTableSwitch
     Label *defaultcase = mir->getDefault()->lir()->label();
     const LAllocation *temp;
 
     if (ins->index()->isDouble()) {
         temp = ins->tempInt();
 
         // The input is a double, so try and convert it to an integer.
         // If it does not fit in an integer, take the default case.
-        emitDoubleToInt32(ToFloatRegister(ins->index()), ToRegister(temp), defaultcase);
+        emitDoubleToInt32(ToFloatRegister(ins->index()), ToRegister(temp), defaultcase, false);
     } else {
         temp = ins->index();
     }
 
     // Lower value with low value
     if (mir->low() != 0)
         masm.subl(Imm32(mir->low()), ToRegister(temp));
 
@@ -936,44 +936,46 @@ CodeGeneratorX86Shared::visitGuardClass(
         return false;
     return true;
 }
 
 // Checks whether a double is representable as a 32-bit integer. If so, the
 // integer is written to the output register. Otherwise, a bailout is taken to
 // the given snapshot. This function overwrites the scratch float register.
 void
-CodeGeneratorX86Shared::emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail)
+CodeGeneratorX86Shared::emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail, bool negativeZeroCheck)
 {
     // Note that we don't specify the destination width for the truncated
     // conversion to integer. x64 will use the native width (quadword) which
     // sign-extends the top bits, preserving a little sanity.
     masm.cvttsd2s(src, dest);
     masm.cvtsi2sd(dest, ScratchFloatReg);
     masm.ucomisd(src, ScratchFloatReg);
     masm.j(Assembler::Parity, fail);
     masm.j(Assembler::NotEqual, fail);
 
     // Check for -0
-    Label notZero;
-    masm.testl(dest, dest);
-    masm.j(Assembler::NonZero, &notZero);
+    if (negativeZeroCheck) {
+        Label notZero;
+        masm.testl(dest, dest);
+        masm.j(Assembler::NonZero, &notZero);
 
-    if (Assembler::HasSSE41()) {
-        masm.ptest(src, src);
-        masm.j(Assembler::NonZero, fail);
-    } else {
-        // bit 0 = sign of low double
-        // bit 1 = sign of high double
-        masm.movmskpd(src, dest);
-        masm.andl(Imm32(1), dest);
-        masm.j(Assembler::NonZero, fail);
+        if (Assembler::HasSSE41()) {
+            masm.ptest(src, src);
+            masm.j(Assembler::NonZero, fail);
+        } else {
+            // bit 0 = sign of low double
+            // bit 1 = sign of high double
+            masm.movmskpd(src, dest);
+            masm.andl(Imm32(1), dest);
+            masm.j(Assembler::NonZero, fail);
+        }
+
+        masm.bind(&notZero);
     }
-    
-    masm.bind(&notZero);
 }
 
 class OutOfLineTruncate : public OutOfLineCodeBase<CodeGeneratorX86Shared>
 {
     LTruncateDToInt32 *ins_;
 
   public:
     OutOfLineTruncate(LTruncateDToInt32 *ins)
--- a/js/src/ion/shared/CodeGenerator-x86-shared.h
+++ b/js/src/ion/shared/CodeGenerator-x86-shared.h
@@ -86,17 +86,17 @@ class CodeGeneratorX86Shared : public Co
     bool bailoutIf(Assembler::Condition condition, LSnapshot *snapshot);
     bool bailoutFrom(Label *label, LSnapshot *snapshot);
 
   protected:
     bool generatePrologue();
     bool generateEpilogue();
     bool generateOutOfLineCode();
 
-    void emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail);
+    void emitDoubleToInt32(const FloatRegister &src, const Register &dest, Label *fail, bool negativeZeroCheck = true);
 
     Operand createArrayElementOperand(Register elements, const LAllocation *index);
 
     enum NaNCond {
         NaN_Unexpected,
         NaN_IsTrue,
         NaN_IsFalse
     };
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug739854.js
@@ -0,0 +1,12 @@
+function test(x) {
+    switch(x) {
+        case 0:
+          return 0;
+        default:
+          return -1;
+    }
+}
+
+for(var i=0; i<100; i++) {
+    assertEq(test(-0), 0);
+}