Bug 836373 - IonMonkey changes to compareString for the baseline jit. r=jandem

js/src/ion/CodeGenerator.cpp

@@ -2866,33 +2866,7 @@ CodeGenerator::emitCompareS(LInstruction *lir, JSOp op, Register left, Register
     if (!ool)
         return false;
 
-    Label notPointerEqual;
+    masm.compareStrings(op, left, right, output, temp, ool->entry());
-    // Fast path for identical strings
-    masm.branchPtr(Assembler::NotEqual, left, right, &notPointerEqual);
-    masm.move32(Imm32(op == JSOP_EQ || op == JSOP_STRICTEQ), output);
-    masm.jump(ool->rejoin());
-
-    masm.bind(&notPointerEqual);
-    masm.loadPtr(Address(left, JSString::offsetOfLengthAndFlags()), output);
-    masm.loadPtr(Address(right, JSString::offsetOfLengthAndFlags()), temp);
-
-    Label notAtom;
-    // We can optimize the equality operation to a pointer compare for
-    // two atoms.
-    Imm32 atomBit(JSString::ATOM_BIT);
-    masm.branchTest32(Assembler::Zero, output, atomBit, &notAtom);
-    masm.branchTest32(Assembler::Zero, temp, atomBit, &notAtom);
-
-    masm.cmpPtr(left, right);
-    emitSet(JSOpToCondition(op), output);
-    masm.jump(ool->rejoin());
-
-    masm.bind(&notAtom);
-    // Strings of different length can never be equal.
-    masm.rshiftPtr(Imm32(JSString::LENGTH_SHIFT), output);
-    masm.rshiftPtr(Imm32(JSString::LENGTH_SHIFT), temp);
-    masm.branchPtr(Assembler::Equal, output, temp, ool->entry());
-    masm.move32(Imm32(op == JSOP_NE || op == JSOP_STRICTNE), output);
 
     masm.bind(ool->rejoin());
     return true;
@@ -3056,7 +3030,7 @@ CodeGenerator::visitIsNullOrLikeUndefined(LIsNullOrLikeUndefined *lir)
     else
         cond = masm.testUndefined(cond, value);
 
-    emitSet(cond, output);
+    masm.emitSet(cond, output);
     return true;
 }
 
@@ -4096,7 +4070,7 @@ CodeGenerator::visitIteratorMore(LIteratorMore *lir)
     // Set output to true if props_cursor < props_end.
     masm.loadPtr(Address(output, offsetof(NativeIterator, props_end)), temp);
     masm.cmpPtr(Address(output, offsetof(NativeIterator, props_cursor)), temp);
-    emitSet(Assembler::LessThan, output);
+    masm.emitSet(Assembler::LessThan, output);
 
     masm.bind(ool->rejoin());
     return true;

js/src/ion/IonMacroAssembler.cpp

@@ -432,6 +432,43 @@ MacroAssembler::initGCThing(const Register &obj, JSObject *templateObject)
     }
 }
 
+void
+MacroAssembler::compareStrings(JSOp op, Register left, Register right, Register result,
+                               Register temp, Label *fail)
+{
+    JS_ASSERT(IsEqualityOp(op));
+
+    Label done;
+    Label notPointerEqual;
+    // Fast path for identical strings.
+    branchPtr(Assembler::NotEqual, left, right, &notPointerEqual);
+    move32(Imm32(op == JSOP_EQ || op == JSOP_STRICTEQ), result);
+    jump(&done);
+
+    bind(&notPointerEqual);
+    loadPtr(Address(left, JSString::offsetOfLengthAndFlags()), result);
+    loadPtr(Address(right, JSString::offsetOfLengthAndFlags()), temp);
+
+    Label notAtom;
+    // Optimize the equality operation to a pointer compare for two atoms.
+    Imm32 atomBit(JSString::ATOM_BIT);
+    branchTest32(Assembler::Zero, result, atomBit, &notAtom);
+    branchTest32(Assembler::Zero, temp, atomBit, &notAtom);
+
+    cmpPtr(left, right);
+    emitSet(JSOpToCondition(op), result);
+    jump(&done);
+
+    bind(&notAtom);
+    // Strings of different length can never be equal.
+    rshiftPtr(Imm32(JSString::LENGTH_SHIFT), result);
+    rshiftPtr(Imm32(JSString::LENGTH_SHIFT), temp);
+    branchPtr(Assembler::Equal, result, temp, fail);
+    move32(Imm32(op == JSOP_NE || op == JSOP_STRICTNE), result);
+
+    bind(&done);
+}
+
 void
 MacroAssembler::parCheckInterruptFlags(const Register &tempReg,
                                        Label *fail)

js/src/ion/IonMacroAssembler.h

@@ -500,6 +500,11 @@ class MacroAssembler : public MacroAssemblerSpecific
                        Label *fail);
     void initGCThing(const Register &obj, JSObject *templateObject);
 
+    // Compares two strings for equality based on the JSOP.
+    // This checks for identical pointers, atoms and length and fails for everything else.
+    void compareStrings(JSOp op, Register left, Register right, Register result,
+                        Register temp, Label *fail);
+
     // Checks the flags that signal that parallel code may need to interrupt or
     // abort.  Branches to fail in that case.
     void parCheckInterruptFlags(const Register &tempReg,

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

@@ -103,13 +103,6 @@ CodeGeneratorARM::visitTestIAndBranch(LTestIAndBranch *test)
     return true;
 }
 
-void
-CodeGeneratorARM::emitSet(Assembler::Condition cond, const Register &dest)
-{
-    masm.ma_mov(Imm32(0), dest);
-    masm.ma_mov(Imm32(1), dest, NoSetCond, cond);
-}
-
 bool
 CodeGeneratorARM::visitCompare(LCompare *comp)
 {
@@ -1236,7 +1229,7 @@ CodeGeneratorARM::visitCompareD(LCompareD *comp)
 
     Assembler::DoubleCondition cond = JSOpToDoubleCondition(comp->mir()->jsop());
     masm.compareDouble(lhs, rhs);
-    emitSet(Assembler::ConditionFromDoubleCondition(cond), ToRegister(comp->output()));
+    masm.emitSet(Assembler::ConditionFromDoubleCondition(cond), ToRegister(comp->output()));
     return true;
 }
 
@@ -1270,7 +1263,7 @@ CodeGeneratorARM::visitCompareB(LCompareB *lir)
             masm.cmp32(lhs.payloadReg(), Imm32(rhs->toConstant()->toBoolean()));
         else
             masm.cmp32(lhs.payloadReg(), ToRegister(rhs));
-        emitSet(JSOpToCondition(mir->jsop()), output);
+        masm.emitSet(JSOpToCondition(mir->jsop()), output);
         masm.jump(&done);
     }
 
@@ -1322,7 +1315,7 @@ CodeGeneratorARM::visitCompareV(LCompareV *lir)
     masm.j(Assembler::NotEqual, &notEqual);
     {
         masm.cmp32(lhs.payloadReg(), rhs.payloadReg());
-        emitSet(cond, output);
+        masm.emitSet(cond, output);
         masm.jump(&done);
     }
     masm.bind(&notEqual);
@@ -1363,7 +1356,7 @@ CodeGeneratorARM::visitNotI(LNotI *ins)
 {
     // It is hard to optimize !x, so just do it the basic way for now.
     masm.ma_cmp(ToRegister(ins->input()), Imm32(0));
-    emitSet(Assembler::Equal, ToRegister(ins->output()));
+    masm.emitSet(Assembler::Equal, ToRegister(ins->output()));
     return true;
 }
 

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

@@ -57,9 +57,6 @@ class CodeGeneratorARM : public CodeGeneratorShared
 
     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);

js/src/ion/arm/MacroAssembler-arm.h

@@ -1065,6 +1065,13 @@ class MacroAssemblerARMCompat : public MacroAssemblerARM
         ma_lsl(imm, dest, dest);
     }
 
+    void
+    emitSet(Assembler::Condition cond, const Register &dest)
+    {
+        ma_mov(Imm32(0), dest);
+        ma_mov(Imm32(1), dest, NoSetCond, cond);
+    }
+
     // Setup a call to C/C++ code, given the number of general arguments it
     // takes. Note that this only supports cdecl.
     //

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

@@ -120,42 +120,6 @@ CodeGeneratorX86Shared::visitTestDAndBranch(LTestDAndBranch *test)
     return true;
 }
 
-void
-CodeGeneratorX86Shared::emitSet(Assembler::Condition cond, const Register &dest,
-                                Assembler::NaNCond ifNaN)
-{
-    if (GeneralRegisterSet(Registers::SingleByteRegs).has(dest)) {
-        // If the register we're defining is a single byte register,
-        // take advantage of the setCC instruction
-        masm.setCC(cond, dest);
-        masm.movzxbl(dest, dest);
-
-        if (ifNaN != Assembler::NaN_Unexpected) {
-            Label noNaN;
-            masm.j(Assembler::NoParity, &noNaN);
-            if (ifNaN == Assembler::NaN_IsTrue)
-                masm.movl(Imm32(1), dest);
-            else
-                masm.xorl(dest, dest);
-            masm.bind(&noNaN);
-        }
-    } else {
-        Label end;
-        Label ifFalse;
-
-        if (ifNaN == Assembler::NaN_IsFalse)
-            masm.j(Assembler::Parity, &ifFalse);
-        masm.movl(Imm32(1), dest);
-        masm.j(cond, &end);
-        if (ifNaN == Assembler::NaN_IsTrue)
-            masm.j(Assembler::Parity, &end);
-        masm.bind(&ifFalse);
-        masm.xorl(dest, dest);
-
-        masm.bind(&end);
-    }
-}
-
 void
 CodeGeneratorX86Shared::emitCompare(MCompare::CompareType type, const LAllocation *left, const LAllocation *right)
 {
@@ -176,7 +140,7 @@ bool
 CodeGeneratorX86Shared::visitCompare(LCompare *comp)
 {
     emitCompare(comp->mir()->compareType(), comp->left(), comp->right());
-    emitSet(JSOpToCondition(comp->jsop()), ToRegister(comp->output()));
+    masm.emitSet(JSOpToCondition(comp->jsop()), ToRegister(comp->output()));
     return true;
 }
 
@@ -197,7 +161,7 @@ CodeGeneratorX86Shared::visitCompareD(LCompareD *comp)
 
     Assembler::DoubleCondition cond = JSOpToDoubleCondition(comp->mir()->jsop());
     masm.compareDouble(cond, lhs, rhs);
-    emitSet(Assembler::ConditionFromDoubleCondition(cond), ToRegister(comp->output()),
+    masm.emitSet(Assembler::ConditionFromDoubleCondition(cond), ToRegister(comp->output()),
             Assembler::NaNCondFromDoubleCondition(cond));
     return true;
 }
@@ -206,7 +170,7 @@ bool
 CodeGeneratorX86Shared::visitNotI(LNotI *ins)
 {
     masm.cmpl(ToRegister(ins->input()), Imm32(0));
-    emitSet(Assembler::Equal, ToRegister(ins->output()));
+    masm.emitSet(Assembler::Equal, ToRegister(ins->output()));
     return true;
 }
 
@@ -217,7 +181,7 @@ CodeGeneratorX86Shared::visitNotD(LNotD *ins)
 
     masm.xorpd(ScratchFloatReg, ScratchFloatReg);
     masm.compareDouble(Assembler::DoubleEqualOrUnordered, opd, ScratchFloatReg);
-    emitSet(Assembler::Equal, ToRegister(ins->output()), Assembler::NaN_IsTrue);
+    masm.emitSet(Assembler::Equal, ToRegister(ins->output()), Assembler::NaN_IsTrue);
     return true;
 }
 

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

@@ -64,11 +64,6 @@ class CodeGeneratorX86Shared : public CodeGeneratorShared
 
     void emitCompare(MCompare::CompareType type, const LAllocation *left, const LAllocation *right);
 
-    // Emits a conditional set.
-    void emitSet(Assembler::Condition cond, const Register &dest,
-                 Assembler::NaNCond ifNaN = Assembler::NaN_Unexpected);
-    void emitSet(Assembler::DoubleCondition 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,

js/src/ion/shared/MacroAssembler-x86-shared.h

@@ -391,6 +391,40 @@ class MacroAssemblerX86Shared : public Assembler
         return true;
     }
 
+    void emitSet(Assembler::Condition cond, const Register &dest,
+                 Assembler::NaNCond ifNaN = Assembler::NaN_Unexpected) {
+        if (GeneralRegisterSet(Registers::SingleByteRegs).has(dest)) {
+            // If the register we're defining is a single byte register,
+            // take advantage of the setCC instruction
+            setCC(cond, dest);
+            movzxbl(dest, dest);
+
+            if (ifNaN != Assembler::NaN_Unexpected) {
+                Label noNaN;
+                j(Assembler::NoParity, &noNaN);
+                if (ifNaN == Assembler::NaN_IsTrue)
+                    movl(Imm32(1), dest);
+                else
+                    xorl(dest, dest);
+                bind(&noNaN);
+            }
+        } else {
+            Label end;
+            Label ifFalse;
+
+            if (ifNaN == Assembler::NaN_IsFalse)
+                j(Assembler::Parity, &ifFalse);
+            movl(Imm32(1), dest);
+            j(cond, &end);
+            if (ifNaN == Assembler::NaN_IsTrue)
+                j(Assembler::Parity, &end);
+            bind(&ifFalse);
+            xorl(dest, dest);
+
+            bind(&end);
+        }
+    }
+
     // Emit a JMP that can be toggled to a CMP. See ToggleToJmp(), ToggleToCmp().
     CodeOffsetLabel toggledJump(Label *label) {
         CodeOffsetLabel offset(size());

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

@@ -347,7 +347,7 @@ CodeGeneratorX64::visitCompareB(LCompareB *lir)
 
     // Perform the comparison.
     masm.cmpq(lhs.valueReg(), ScratchReg);
-    emitSet(JSOpToCondition(mir->jsop()), output);
+    masm.emitSet(JSOpToCondition(mir->jsop()), output);
     return true;
 }
 
@@ -380,11 +380,10 @@ CodeGeneratorX64::visitCompareV(LCompareV *lir)
     const ValueOperand rhs = ToValue(lir, LCompareV::RhsInput);
     const Register output = ToRegister(lir->output());
 
-    JS_ASSERT(mir->jsop() == JSOP_EQ || mir->jsop() == JSOP_STRICTEQ ||
+    JS_ASSERT(IsEqualityOp(mir->jsop()));
-              mir->jsop() == JSOP_NE || mir->jsop() == JSOP_STRICTNE);
 
     masm.cmpq(lhs.valueReg(), rhs.valueReg());
-    emitSet(JSOpToCondition(mir->jsop()), output);
+    masm.emitSet(JSOpToCondition(mir->jsop()), output);
     return true;
 }
 

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

@@ -351,7 +351,7 @@ CodeGeneratorX86::visitCompareB(LCompareB *lir)
             masm.cmp32(lhs.payloadReg(), Imm32(rhs->toConstant()->toBoolean()));
         else
             masm.cmp32(lhs.payloadReg(), ToRegister(rhs));
-        emitSet(JSOpToCondition(mir->jsop()), output);
+        masm.emitSet(JSOpToCondition(mir->jsop()), output);
         masm.jump(&done);
     }
     masm.bind(&notBoolean);
@@ -394,15 +394,14 @@ CodeGeneratorX86::visitCompareV(LCompareV *lir)
     const ValueOperand rhs = ToValue(lir, LCompareV::RhsInput);
     const Register output = ToRegister(lir->output());
 
-    JS_ASSERT(mir->jsop() == JSOP_EQ || mir->jsop() == JSOP_STRICTEQ ||
+    JS_ASSERT(IsEqualityOp(mir->jsop()));
-              mir->jsop() == JSOP_NE || mir->jsop() == JSOP_STRICTNE);
 
     Label notEqual, done;
     masm.cmp32(lhs.typeReg(), rhs.typeReg());
     masm.j(Assembler::NotEqual, &notEqual);
     {
         masm.cmp32(lhs.payloadReg(), rhs.payloadReg());
-        emitSet(cond, output);
+        masm.emitSet(cond, output);
         masm.jump(&done);
     }
     masm.bind(&notEqual);

js/src/jsopcode.h

@@ -459,6 +459,12 @@ IsGlobalOp(JSOp op)
     return js_CodeSpec[op].format & JOF_GNAME;
 }
 
+inline bool
+IsEqualityOp(JSOp op)
+{
+    return op == JSOP_EQ || op == JSOP_NE || op == JSOP_STRICTEQ || op == JSOP_STRICTNE;
+}
+
 inline bool
 IsGetterPC(jsbytecode *pc)
 {