Bug 721438 -Implement LCompareS to optimize string equality tests. r=jandem

js/src/ion/CodeGenerator.cpp

@@ -1298,6 +1298,56 @@ CodeGenerator::visitBinaryV(LBinaryV *lir)
     }
 }
 
+bool
+CodeGenerator::visitCompareS(LCompareS *lir)
+{
+    JSOp op = lir->jsop();
+    Register left = ToRegister(lir->left());
+    Register right = ToRegister(lir->right());
+    Register output = ToRegister(lir->output());
+
+    typedef bool (*pf)(JSContext *, HandleString, HandleString, JSBool *);
+    static const VMFunction stringsEqualInfo = FunctionInfo<pf>(ion::StringsEqual<true>);
+    static const VMFunction stringsNotEqualInfo = FunctionInfo<pf>(ion::StringsEqual<false>);
+
+    OutOfLineCode *ool = NULL;
+    if (op == JSOP_EQ || op == JSOP_STRICTEQ) {
+        ool = oolCallVM(stringsEqualInfo, lir, (ArgList(), left, right),  StoreRegisterTo(output));
+    } else {
+        JS_ASSERT(op == JSOP_NE || op == JSOP_STRICTNE);
+        ool = oolCallVM(stringsNotEqualInfo, lir, (ArgList(), left, right), StoreRegisterTo(output));
+    }
+
+    if (!ool)
+        return false;
+
+    Label notPointerEqual;
+    // 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);
+
+    // JSString::isAtom === (lengthAndFlags & ATOM_MASK == 0)
+    JS_STATIC_ASSERT(JSString::ATOM_FLAGS == 0);
+    Imm32 atomMask(JSString::ATOM_MASK);
+
+    // This optimization is only correct for atomized strings,
+    // so we need to jump to the ool path.
+    masm.branchTest32(Assembler::NonZero, Address(left, JSString::offsetOfLengthAndFlags()), 
+                      atomMask, ool->entry());
+
+    masm.branchTest32(Assembler::NonZero, Address(right, JSString::offsetOfLengthAndFlags()), 
+                      atomMask, ool->entry());
+
+    masm.cmpPtr(left, right);
+    emitSet(JSOpToCondition(op), output);
+
+    masm.bind(ool->rejoin());
+    return true;
+}
+
 bool
 CodeGenerator::visitCompareV(LCompareV *lir)
 {

js/src/ion/CodeGenerator.h

@@ -135,6 +135,7 @@ class CodeGenerator : public CodeGeneratorSpecific
     bool visitAbsI(LAbsI *lir);
     bool visitMathFunctionD(LMathFunctionD *ins);
     bool visitBinaryV(LBinaryV *lir);
+    bool visitCompareS(LCompareS *lir);
     bool visitCompareV(LCompareV *lir);
     bool visitIsNullOrUndefined(LIsNullOrUndefined *lir);
     bool visitIsNullOrUndefinedAndBranch(LIsNullOrUndefinedAndBranch *lir);

js/src/ion/LIR-Common.h

@@ -705,6 +705,33 @@ class LCompareD : public LInstructionHelper<1, 2, 0>
     }
 };
 
+class LCompareS : public LInstructionHelper<1, 2, 0>
+{
+    JSOp jsop_;
+
+  public:
+    LIR_HEADER(CompareS);
+    LCompareS(JSOp jsop, const LAllocation &left, const LAllocation &right)
+      : jsop_(jsop)
+    {
+        setOperand(0, left);
+        setOperand(1, right);
+    }
+
+    JSOp jsop() const {
+        return jsop_;
+    }
+    const LAllocation *left() {
+        return getOperand(0);
+    }
+    const LAllocation *right() {
+        return getOperand(1);
+    }
+    const LDefinition *output() {
+        return getDef(0);
+    }
+};
+
 class LCompareV : public LCallInstructionHelper<1, 2 * BOX_PIECES, 0>
 {
     JSOp jsop_;

js/src/ion/LOpcodes.h

@@ -80,6 +80,7 @@
     _(TestVAndBranch)               \
     _(Compare)                      \
     _(CompareD)                     \
+    _(CompareS)                     \
     _(CompareV)                     \
     _(CompareAndBranch)             \
     _(CompareDAndBranch)            \

js/src/ion/Lowering.cpp

@@ -350,6 +350,16 @@ LIRGenerator::visitCompare(MCompare *comp)
     MDefinition *right = comp->getOperand(1);
 
     if (comp->specialization() != MIRType_None) {
+        // Move below the emitAtUses call if we ever implement
+        // LCompareSAndBranch. Doing this now wouldn't be wrong, but doesn't
+        // make sense and avoids confusion.
+        if (comp->specialization() == MIRType_String) {
+            LCompareS *lir = new LCompareS(comp->jsop(), useRegister(left), useRegister(right));
+            if (!define(lir, comp))
+                return false;
+            return assignSafepoint(lir, comp);
+        }
+
         // Sniff out if the output of this compare is used only for a branching.
         // If it is, then we willl emit an LCompare*AndBranch instruction in place
         // of this compare and any test that uses this compare. Thus, we can

js/src/ion/MIR.cpp

@@ -992,6 +992,12 @@ MCompare::infer(JSContext *cx, const TypeOracle::BinaryTypes &b)
             return;
         }
 
+        if (lhs == MIRType_String && rhs == MIRType_String) {
+            // We don't yet want to optimize relational string compares.
+            specialization_ = MIRType_String;
+            return;
+        }
+
         if (IsNullOrUndefined(lhs)) {
             // Lowering expects the rhs to be null/undefined, so we have to
             // swap the operands. This is necessary since we may not know which

js/src/ion/TypePolicy.cpp

@@ -173,6 +173,9 @@ ComparePolicy::adjustInputs(MInstruction *def)
           case MIRType_Object:
             replace = MUnbox::New(in, MIRType_Object, MUnbox::Infallible);
             break;
+          case MIRType_String:
+            replace = MUnbox::New(in, MIRType_String, MUnbox::Infallible);
+            break;
           default:
             JS_NOT_REACHED("Unknown compare specialization");
             return false;

js/src/ion/VMFunctions.cpp

@@ -189,6 +189,20 @@ GreaterThanOrEqual(JSContext *cx, const Value &lhs, const Value &rhs, JSBool *re
     return true;
 }
 
+template<bool Equal>
+bool
+StringsEqual(JSContext *cx, HandleString lhs, HandleString rhs, JSBool *res)
+{
+    bool equal;
+    if (!js::EqualStrings(cx, lhs, rhs, &equal))
+        return false;
+    *res = (equal == Equal);
+    return true;
+}
+
+template bool StringsEqual<true>(JSContext *cx, HandleString lhs, HandleString rhs, JSBool *res);
+template bool StringsEqual<false>(JSContext *cx, HandleString lhs, HandleString rhs, JSBool *res);
+
 bool
 ValueToBooleanComplement(JSContext *cx, const Value &input, JSBool *output)
 {

js/src/ion/VMFunctions.h

@@ -415,6 +415,9 @@ bool LessThanOrEqual(JSContext *cx, const Value &lhs, const Value &rhs, JSBool *
 bool GreaterThan(JSContext *cx, const Value &lhs, const Value &rhs, JSBool *res);
 bool GreaterThanOrEqual(JSContext *cx, const Value &lhs, const Value &rhs, JSBool *res);
 
+template<bool Equal>
+bool StringsEqual(JSContext *cx, HandleString left, HandleString right, JSBool *res);
+
 bool ValueToBooleanComplement(JSContext *cx, const Value &input, JSBool *output);
 
 bool IteratorMore(JSContext *cx, HandleObject obj, JSBool *res);

js/src/ion/x86/Assembler-x86.h

@@ -337,6 +337,9 @@ class Assembler : public AssemblerX86Shared
         masm.cmpl_ir(ptr.value, src.code());
         writeDataRelocation(ptr);
     }
+    void cmpl(const Register &lhs, const Register &rhs) {
+        masm.cmpl_rr(rhs.code(), lhs.code());
+    }
     void cmpl(const Operand &op, ImmGCPtr imm) {
         switch (op.kind()) {
           case Operand::REG:

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

@@ -376,8 +376,11 @@ class MacroAssemblerX86 : public MacroAssemblerX86Shared
     void cmpPtr(const Address &lhs, const ImmWord rhs) {
         cmpl(Operand(lhs), rhs);
     }
+    void cmpPtr(const Register &lhs, const Register &rhs) {
+        cmpl(lhs, rhs);
+    }
     void testPtr(const Register &lhs, const Register &rhs) {
-        return testl(lhs, rhs);
+        testl(lhs, rhs);
     }
 
     Condition testNegativeZero(const FloatRegister &reg, const Register &scratch);

js/src/jit-test/tests/ion/compare-string.js

@@ -0,0 +1,40 @@
+function compareToAtom(a) {
+    return a == 'test';
+}
+
+function compareToAtomNe(a) {
+    return a != 'test';
+}
+
+var st = 'st';
+
+function compareToRope(a) {
+    return a == ('te' + st);
+}
+
+function compareToRopeNe(a) {
+    var st = 'st';
+    return a != ('te' + st);
+}
+
+function main() {
+    var test = 'test';
+    var foobar = 'foobar';
+
+    assertEq(compareToAtom(test), true);
+    assertEq(compareToAtom(foobar), false);
+
+    assertEq(compareToAtomNe(test), false);
+    assertEq(compareToAtomNe(foobar), true);
+
+
+    assertEq(compareToRope(test), true);
+    assertEq(compareToRope(foobar), false);
+
+    assertEq(compareToRopeNe(test), false);
+    assertEq(compareToRopeNe(foobar), true);
+}
+
+for (var i = 0; i < 100000; i++) {
+    main();
+}