From 4b00cf33c0e91cb7038e613403f2991421f0e10e Mon Sep 17 00:00:00 2001 From: Hannes Verschore Date: Fri, 1 Feb 2013 11:39:02 +0100 Subject: [PATCH] Bug 828119: IonMonkey: Add fastpath for strict string comparison, r=jandem --- js/src/ion/CodeGenerator.cpp | 48 +++++++++++++++++++++++---- js/src/ion/CodeGenerator.h | 3 ++ js/src/ion/IonMacroAssembler.h | 7 ++++ js/src/ion/LIR-Common.h | 28 ++++++++++++++++ js/src/ion/LOpcodes.h | 1 + js/src/ion/Lowering.cpp | 13 ++++++++ js/src/ion/MIR.cpp | 38 ++++++++++++++++++++++ js/src/ion/MIR.h | 9 ++++++ js/src/ion/TypePolicy.cpp | 52 +++++++++++++++++++++++------- js/src/jit-test/tests/bug828119.js | 32 ++++++++++++++++++ 10 files changed, 213 insertions(+), 18 deletions(-) create mode 100644 js/src/jit-test/tests/bug828119.js diff --git a/js/src/ion/CodeGenerator.cpp b/js/src/ion/CodeGenerator.cpp index c9be3e9d11f..abc60e5c4f8 100644 --- a/js/src/ion/CodeGenerator.cpp +++ b/js/src/ion/CodeGenerator.cpp @@ -2372,13 +2372,10 @@ static const VMFunction stringsNotEqualInfo = FunctionInfo(ion::StringsEqual); bool -CodeGenerator::visitCompareS(LCompareS *lir) +CodeGenerator::emitCompareS(LInstruction *lir, JSOp op, Register left, Register right, + Register output, Register temp) { - JSOp op = lir->mir()->jsop(); - Register left = ToRegister(lir->left()); - Register right = ToRegister(lir->right()); - Register output = ToRegister(lir->output()); - Register temp = ToRegister(lir->temp()); + JS_ASSERT(lir->isCompareS() || lir->isCompareStrictS()); OutOfLineCode *ool = NULL; if (op == JSOP_EQ || op == JSOP_STRICTEQ) { @@ -2423,6 +2420,45 @@ CodeGenerator::visitCompareS(LCompareS *lir) return true; } +bool +CodeGenerator::visitCompareStrictS(LCompareStrictS *lir) +{ + JSOp op = lir->mir()->jsop(); + JS_ASSERT(op == JSOP_STRICTEQ || op == JSOP_STRICTNE); + + const ValueOperand leftV = ToValue(lir, LCompareStrictS::Lhs); + Register right = ToRegister(lir->right()); + Register output = ToRegister(lir->output()); + Register temp = ToRegister(lir->temp0()); + + Label string, done; + + masm.branchTestString(Assembler::Equal, leftV, &string); + masm.move32(Imm32(op == JSOP_STRICTNE), output); + masm.jump(&done); + + masm.bind(&string); + Register left = masm.extractString(leftV, ToRegister(lir->temp1())); + if (!emitCompareS(lir, op, left, right, output, temp)) + return false; + + masm.bind(&done); + + return true; +} + +bool +CodeGenerator::visitCompareS(LCompareS *lir) +{ + JSOp op = lir->mir()->jsop(); + Register left = ToRegister(lir->left()); + Register right = ToRegister(lir->right()); + Register output = ToRegister(lir->output()); + Register temp = ToRegister(lir->temp()); + + return emitCompareS(lir, op, left, right, output, temp); +} + typedef bool (*CompareFn)(JSContext *, MutableHandleValue, MutableHandleValue, JSBool *); static const VMFunction EqInfo = FunctionInfo(ion::LooselyEqual); static const VMFunction NeInfo = FunctionInfo(ion::LooselyEqual); diff --git a/js/src/ion/CodeGenerator.h b/js/src/ion/CodeGenerator.h index 42bc16e86a6..7ba8e492bf7 100644 --- a/js/src/ion/CodeGenerator.h +++ b/js/src/ion/CodeGenerator.h @@ -128,7 +128,10 @@ class CodeGenerator : public CodeGeneratorSpecific bool visitModD(LModD *ins); bool visitMinMaxI(LMinMaxI *lir); bool visitBinaryV(LBinaryV *lir); + bool emitCompareS(LInstruction *lir, JSOp op, Register left, Register right, + Register output, Register temp); bool visitCompareS(LCompareS *lir); + bool visitCompareStrictS(LCompareStrictS *lir); bool visitCompareVM(LCompareVM *lir); bool visitIsNullOrLikeUndefined(LIsNullOrLikeUndefined *lir); bool visitIsNullOrLikeUndefinedAndBranch(LIsNullOrLikeUndefinedAndBranch *lir); diff --git a/js/src/ion/IonMacroAssembler.h b/js/src/ion/IonMacroAssembler.h index fa321c85376..2802f486ac7 100644 --- a/js/src/ion/IonMacroAssembler.h +++ b/js/src/ion/IonMacroAssembler.h @@ -476,6 +476,13 @@ class MacroAssembler : public MacroAssemblerSpecific } } + Register extractString(const Address &address, Register scratch) { + return extractObject(address, scratch); + } + Register extractString(const ValueOperand &value, Register scratch) { + return extractObject(value, scratch); + } + // Inline version of js_TypedArray_uint8_clamp_double. // This function clobbers the input register. void clampDoubleToUint8(FloatRegister input, Register output); diff --git a/js/src/ion/LIR-Common.h b/js/src/ion/LIR-Common.h index cfeb0aadc5b..faade7d0153 100644 --- a/js/src/ion/LIR-Common.h +++ b/js/src/ion/LIR-Common.h @@ -1118,6 +1118,34 @@ class LCompareS : public LInstructionHelper<1, 2, 1> } }; +// strict-equality between value and string. +class LCompareStrictS : public LInstructionHelper<1, BOX_PIECES + 1, 2> +{ + public: + LIR_HEADER(CompareStrictS) + LCompareStrictS(const LAllocation &rhs, const LDefinition &temp0, + const LDefinition &temp1) { + setOperand(BOX_PIECES, rhs); + setTemp(0, temp0); + setTemp(1, temp1); + } + + static const size_t Lhs = 0; + + const LAllocation *right() { + return getOperand(BOX_PIECES); + } + const LDefinition *temp0() { + return getTemp(0); + } + const LDefinition *temp1() { + return getTemp(1); + } + MCompare *mir() { + return mir_->toCompare(); + } +}; + // Used for strict-equality comparisons where one side is a boolean // and the other is a value. Note that CompareI is used to compare // two booleans. diff --git a/js/src/ion/LOpcodes.h b/js/src/ion/LOpcodes.h index f9b7ef20fa8..9fd766d415d 100644 --- a/js/src/ion/LOpcodes.h +++ b/js/src/ion/LOpcodes.h @@ -62,6 +62,7 @@ _(CompareD) \ _(CompareDAndBranch) \ _(CompareS) \ + _(CompareStrictS) \ _(CompareB) \ _(CompareBAndBranch) \ _(CompareV) \ diff --git a/js/src/ion/Lowering.cpp b/js/src/ion/Lowering.cpp index e074848bcb1..8787ccaeecd 100644 --- a/js/src/ion/Lowering.cpp +++ b/js/src/ion/Lowering.cpp @@ -586,6 +586,19 @@ LIRGenerator::visitCompare(MCompare *comp) return assignSafepoint(lir, comp); } + // Strict compare between value and string + if (comp->compareType() == MCompare::Compare_StrictString) { + JS_ASSERT(left->type() == MIRType_Value); + JS_ASSERT(right->type() == MIRType_String); + + LCompareStrictS *lir = new LCompareStrictS(useRegister(right), temp(), temp()); + if (!useBox(lir, LCompareStrictS::Lhs, left)) + return false; + if (!define(lir, comp)) + return false; + return assignSafepoint(lir, comp); + } + // Unknown/unspecialized compare use a VM call. if (comp->compareType() == MCompare::Compare_Unknown) { LCompareVM *lir = new LCompareVM(); diff --git a/js/src/ion/MIR.cpp b/js/src/ion/MIR.cpp index e2d3f439b27..a5eb759479d 100644 --- a/js/src/ion/MIR.cpp +++ b/js/src/ion/MIR.cpp @@ -1294,6 +1294,7 @@ MCompare::inputType() case Compare_Double: return MIRType_Double; case Compare_String: + case Compare_StrictString: return MIRType_String; case Compare_Object: return MIRType_Object; @@ -1374,6 +1375,18 @@ MCompare::infer(const TypeOracle::BinaryTypes &b, JSContext *cx) return; } + if (strictEq && lhs == MIRType_String) { + // Lowering expects the rhs to be definitly string. + compareType_ = Compare_StrictString; + swapOperands(); + return; + } + + if (strictEq && rhs == MIRType_String) { + compareType_ = Compare_StrictString; + return; + } + // Handle compare with lhs being Undefined or Null. if (!relationalEq && IsNullOrUndefined(lhs)) { // Lowering expects the rhs to be null/undefined, so we have to @@ -1677,6 +1690,31 @@ MCompare::tryFold(bool *result) } } + if (compareType_ == Compare_StrictString) { + JS_ASSERT(op == JSOP_STRICTEQ || op == JSOP_STRICTNE); + JS_ASSERT(rhs()->type() == MIRType_String); + + switch (lhs()->type()) { + case MIRType_Value: + return false; + case MIRType_Boolean: + case MIRType_Int32: + case MIRType_Double: + case MIRType_Object: + case MIRType_Null: + case MIRType_Undefined: + *result = (op == JSOP_STRICTNE); + return true; + case MIRType_String: + // Compare_String specialization should handle this. + JS_NOT_REACHED("Wrong specialization"); + return false; + default: + JS_NOT_REACHED("Unexpected type"); + return false; + } + } + return false; } diff --git a/js/src/ion/MIR.h b/js/src/ion/MIR.h index a3a9574a58c..6887e4536ec 100644 --- a/js/src/ion/MIR.h +++ b/js/src/ion/MIR.h @@ -1509,6 +1509,15 @@ class MCompare // String compared to String Compare_String, + // Undefined compared to String + // Null compared to String + // Boolean compared to String + // Int32 compared to String + // Double compared to String + // Object compared to String + // Value compared to String + Compare_StrictString, + // Object compared to Object Compare_Object, diff --git a/js/src/ion/TypePolicy.cpp b/js/src/ion/TypePolicy.cpp index 6dadabcbdbc..ce910b09b0a 100644 --- a/js/src/ion/TypePolicy.cpp +++ b/js/src/ion/TypePolicy.cpp @@ -98,31 +98,29 @@ ComparePolicy::adjustInputs(MInstruction *def) { JS_ASSERT(def->isCompare()); MCompare *compare = def->toCompare(); - MIRType type = compare->inputType(); // Box inputs to get value - if (type == MIRType_Value) + if (compare->compareType() == MCompare::Compare_Unknown || + compare->compareType() == MCompare::Compare_Value) + { return BoxInputsPolicy::adjustInputs(def); + } - // Nothing to do for undefined and null, lowering handles all types. - if (type == MIRType_Undefined || type == MIRType_Null) - return true; - - // MIRType_Boolean specialization is done for "Anything === Bool" + // Compare_Boolean specialization is done for "Anything === Bool" // If the LHS is boolean, we set the specialization to Compare_Int32. // This matches other comparisons of the form bool === bool and // generated code of Compare_Int32 is more efficient. - if (type == MIRType_Boolean && def->getOperand(0)->type() == MIRType_Boolean) { + if (compare->compareType() == MCompare::Compare_Boolean && + def->getOperand(0)->type() == MIRType_Boolean) + { compare->setCompareType(MCompare::Compare_Int32); - type = compare->inputType(); } - // MIRType_Boolean specialization is done for "Anything === Bool" + // Compare_Boolean specialization is done for "Anything === Bool" // As of previous line Anything can't be Boolean - if (type == MIRType_Boolean) { + if (compare->compareType() == MCompare::Compare_Boolean) { // Unbox rhs that is definitely Boolean MDefinition *rhs = def->getOperand(1); - if (rhs->type() == MIRType_Value) { MInstruction *unbox = MUnbox::New(rhs, MIRType_Boolean, MUnbox::Infallible); def->block()->insertBefore(def, unbox); @@ -134,7 +132,37 @@ ComparePolicy::adjustInputs(MInstruction *def) return true; } + // Compare_StrictString specialization is done for "Anything === String" + // If the LHS is string, we set the specialization to Compare_String. + if (compare->compareType() == MCompare::Compare_StrictString && + def->getOperand(0)->type() == MIRType_String) + { + compare->setCompareType(MCompare::Compare_String); + } + + // Compare_StrictString specialization is done for "Anything === String" + // As of previous line Anything can't be String + if (compare->compareType() == MCompare::Compare_StrictString) { + // Unbox rhs that is definitely String + MDefinition *rhs = def->getOperand(1); + if (rhs->type() == MIRType_Value) { + MInstruction *unbox = MUnbox::New(rhs, MIRType_String, MUnbox::Infallible); + def->block()->insertBefore(def, unbox); + def->replaceOperand(1, unbox); + } + + JS_ASSERT(def->getOperand(0)->type() != MIRType_String); + JS_ASSERT(def->getOperand(1)->type() == MIRType_String); + return true; + } + // Convert all inputs to the right input type + MIRType type = compare->inputType(); + + // Nothing to do for undefined and null, lowering handles all types. + if (type == MIRType_Undefined || type == MIRType_Null) + return true; + for (size_t i = 0; i < 2; i++) { MDefinition *in = def->getOperand(i); if (in->type() == type) diff --git a/js/src/jit-test/tests/bug828119.js b/js/src/jit-test/tests/bug828119.js new file mode 100644 index 00000000000..e92ef357636 --- /dev/null +++ b/js/src/jit-test/tests/bug828119.js @@ -0,0 +1,32 @@ + +function cmp_string_string(a,b) { + return a === b; +} + +assertEq(cmp_string_string("a", "a"), true); +assertEq(cmp_string_string("a", "b"), false); +assertEq(cmp_string_string("a", 1), false); + +function cmp_string_string2(a,b) { + return a === b; +} + +assertEq(cmp_string_string2("a", 1.1), false); +assertEq(cmp_string_string2("a", 2), false); +assertEq(cmp_string_string2("a", {}), false); + +function cmp_string_string3(a,b) { + return a !== b; +} + +assertEq(cmp_string_string3("a", "a"), false); +assertEq(cmp_string_string3("a", "b"), true); +assertEq(cmp_string_string3("a", 1), true); + +function cmp_string_string4(a,b) { + return a !== b; +} + +assertEq(cmp_string_string4("a", 1.1), true); +assertEq(cmp_string_string4("a", 2), true); +assertEq(cmp_string_string4("a", {}), true);