From ef1c647d6be62081fb1137b6af690bdd4ea723db Mon Sep 17 00:00:00 2001 From: Hannes Verschore Date: Mon, 25 Aug 2014 09:52:50 +0200 Subject: [PATCH] Bug 1055762 - IonMonkey: Take the conversion into consideration when deciding when to box, r=jandem --- js/src/jit-test/tests/ion/bug1055762.js | 13 +++ js/src/jit/CodeGenerator.cpp | 8 +- js/src/jit/Lowering.cpp | 23 +++-- js/src/jit/MIR.h | 58 +++++------ js/src/jit/TypePolicy.cpp | 130 ++++++++++++++---------- 5 files changed, 134 insertions(+), 98 deletions(-) create mode 100644 js/src/jit-test/tests/ion/bug1055762.js diff --git a/js/src/jit-test/tests/ion/bug1055762.js b/js/src/jit-test/tests/ion/bug1055762.js new file mode 100644 index 000000000000..54f09bf168a8 --- /dev/null +++ b/js/src/jit-test/tests/ion/bug1055762.js @@ -0,0 +1,13 @@ +// |jit-test| error: TypeError +function g() { + f(0); +} +function f(y) { + return (undefined <= y); +} +try { + g(); +} catch (e) {} +(function() { + f()() +})(); diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index e57694fdc42b..39dbfd4d2cd5 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -235,12 +235,12 @@ CodeGenerator::visitValueToDouble(LValueToDouble *lir) masm.branchTestDouble(Assembler::Equal, tag, &isDouble); masm.branchTestInt32(Assembler::Equal, tag, &isInt32); - if (mir->conversion() != MToDouble::NumbersOnly) { + if (mir->conversion() != MToFPInstruction::NumbersOnly) { masm.branchTestBoolean(Assembler::Equal, tag, &isBool); masm.branchTestUndefined(Assembler::Equal, tag, &isUndefined); hasBoolean = true; hasUndefined = true; - if (mir->conversion() != MToDouble::NonNullNonStringPrimitives) { + if (mir->conversion() != MToFPInstruction::NonNullNonStringPrimitives) { masm.branchTestNull(Assembler::Equal, tag, &isNull); hasNull = true; } @@ -293,12 +293,12 @@ CodeGenerator::visitValueToFloat32(LValueToFloat32 *lir) masm.branchTestDouble(Assembler::Equal, tag, &isDouble); masm.branchTestInt32(Assembler::Equal, tag, &isInt32); - if (mir->conversion() != MToFloat32::NumbersOnly) { + if (mir->conversion() != MToFPInstruction::NumbersOnly) { masm.branchTestBoolean(Assembler::Equal, tag, &isBool); masm.branchTestUndefined(Assembler::Equal, tag, &isUndefined); hasBoolean = true; hasUndefined = true; - if (mir->conversion() != MToFloat32::NonNullNonStringPrimitives) { + if (mir->conversion() != MToFPInstruction::NonNullNonStringPrimitives) { masm.branchTestNull(Assembler::Equal, tag, &isNull); hasNull = true; } diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index 86ce64517d1b..f3af7ff088eb 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -1741,7 +1741,7 @@ bool LIRGenerator::visitToDouble(MToDouble *convert) { MDefinition *opd = convert->input(); - mozilla::DebugOnly conversion = convert->conversion(); + mozilla::DebugOnly conversion = convert->conversion(); switch (opd->type()) { case MIRType_Value: @@ -1753,15 +1753,16 @@ LIRGenerator::visitToDouble(MToDouble *convert) } case MIRType_Null: - JS_ASSERT(conversion != MToDouble::NumbersOnly && conversion != MToDouble::NonNullNonStringPrimitives); + JS_ASSERT(conversion != MToFPInstruction::NumbersOnly && + conversion != MToFPInstruction::NonNullNonStringPrimitives); return lowerConstantDouble(0, convert); case MIRType_Undefined: - JS_ASSERT(conversion != MToDouble::NumbersOnly); + JS_ASSERT(conversion != MToFPInstruction::NumbersOnly); return lowerConstantDouble(GenericNaN(), convert); case MIRType_Boolean: - JS_ASSERT(conversion != MToDouble::NumbersOnly); + JS_ASSERT(conversion != MToFPInstruction::NumbersOnly); /* FALLTHROUGH */ case MIRType_Int32: @@ -1802,15 +1803,16 @@ LIRGenerator::visitToFloat32(MToFloat32 *convert) } case MIRType_Null: - JS_ASSERT(conversion != MToFloat32::NonStringPrimitives); + JS_ASSERT(conversion != MToFPInstruction::NumbersOnly && + conversion != MToFPInstruction::NonNullNonStringPrimitives); return lowerConstantFloat32(0, convert); case MIRType_Undefined: - JS_ASSERT(conversion != MToFloat32::NumbersOnly); + JS_ASSERT(conversion != MToFPInstruction::NumbersOnly); return lowerConstantFloat32(GenericNaN(), convert); case MIRType_Boolean: - JS_ASSERT(conversion != MToFloat32::NumbersOnly); + JS_ASSERT(conversion != MToFPInstruction::NumbersOnly); /* FALLTHROUGH */ case MIRType_Int32: @@ -1854,10 +1856,15 @@ LIRGenerator::visitToInt32(MToInt32 *convert) } case MIRType_Null: + JS_ASSERT(convert->conversion() == MacroAssembler::IntConversion_Any); return define(new(alloc()) LInteger(0), convert); - case MIRType_Int32: case MIRType_Boolean: + JS_ASSERT(convert->conversion() == MacroAssembler::IntConversion_Any || + convert->conversion() == MacroAssembler::IntConversion_NumbersOrBoolsOnly); + return redefine(convert, opd); + + case MIRType_Int32: return redefine(convert, opd); case MIRType_Float32: diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 611df7ac5090..66f3feea58cd 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -3644,9 +3644,7 @@ class MReturnFromCtor } }; -// Converts a primitive (either typed or untyped) to a double. If the input is -// not primitive at runtime, a bailout occurs. -class MToDouble +class MToFPInstruction : public MUnaryInstruction, public ToDoublePolicy { @@ -3661,10 +3659,31 @@ class MToDouble private: ConversionKind conversion_; + protected: + explicit MToFPInstruction(MDefinition *def, ConversionKind conversion = NonStringPrimitives) + : MUnaryInstruction(def), conversion_(conversion) + { } + + public: + ConversionKind conversion() const { + return conversion_; + } + + TypePolicy *typePolicy() { + return this; + } +}; + +// Converts a primitive (either typed or untyped) to a double. If the input is +// not primitive at runtime, a bailout occurs. +class MToDouble + : public MToFPInstruction +{ + private: TruncateKind implicitTruncate_; explicit MToDouble(MDefinition *def, ConversionKind conversion = NonStringPrimitives) - : MUnaryInstruction(def), conversion_(conversion), implicitTruncate_(NoTruncate) + : MToFPInstruction(def, conversion), implicitTruncate_(NoTruncate) { setResultType(MIRType_Double); setMovable(); @@ -3686,14 +3705,6 @@ class MToDouble return new(alloc) MToDouble(def); } - ConversionKind conversion() const { - return conversion_; - } - - TypePolicy *typePolicy() { - return this; - } - MDefinition *foldsTo(TempAllocator &alloc); bool congruentTo(const MDefinition *ins) const { if (!ins->isToDouble() || ins->toToDouble()->conversion() != conversion()) @@ -3725,22 +3736,11 @@ class MToDouble // Converts a primitive (either typed or untyped) to a float32. If the input is // not primitive at runtime, a bailout occurs. class MToFloat32 - : public MUnaryInstruction, - public ToDoublePolicy + : public MToFPInstruction { - public: - // Types of values which can be converted. - enum ConversionKind { - NonStringPrimitives, - NonNullNonStringPrimitives, - NumbersOnly - }; - protected: - ConversionKind conversion_; - MToFloat32(MDefinition *def, ConversionKind conversion) - : MUnaryInstruction(def), conversion_(conversion) + : MToFPInstruction(def, conversion) { setResultType(MIRType_Float32); setMovable(); @@ -3762,14 +3762,6 @@ class MToFloat32 return new(alloc) MToFloat32(def, NonStringPrimitives); } - ConversionKind conversion() const { - return conversion_; - } - - TypePolicy *typePolicy() { - return this; - } - virtual MDefinition *foldsTo(TempAllocator &alloc); bool congruentTo(const MDefinition *ins) const { if (!ins->isToFloat32() || ins->toToFloat32()->conversion() != conversion()) diff --git a/js/src/jit/TypePolicy.cpp b/js/src/jit/TypePolicy.cpp index b9a2cf0c3abe..9e0fbb5677fd 100644 --- a/js/src/jit/TypePolicy.cpp +++ b/js/src/jit/TypePolicy.cpp @@ -132,11 +132,11 @@ ComparePolicy::adjustInputs(TempAllocator &alloc, MInstruction *def) // Unbox rhs that is definitely Boolean MDefinition *rhs = def->getOperand(1); if (rhs->type() != MIRType_Boolean) { - if (rhs->type() != MIRType_Value) - rhs = boxAt(alloc, def, rhs); MInstruction *unbox = MUnbox::New(alloc, rhs, MIRType_Boolean, MUnbox::Infallible); def->block()->insertBefore(def, unbox); def->replaceOperand(1, unbox); + if (!unbox->typePolicy()->adjustInputs(alloc, unbox)) + return false; } JS_ASSERT(def->getOperand(0)->type() != MIRType_Boolean); @@ -158,11 +158,11 @@ ComparePolicy::adjustInputs(TempAllocator &alloc, MInstruction *def) // Unbox rhs that is definitely String MDefinition *rhs = def->getOperand(1); if (rhs->type() != MIRType_String) { - if (rhs->type() != MIRType_Value) - rhs = boxAt(alloc, def, rhs); MInstruction *unbox = MUnbox::New(alloc, rhs, MIRType_String, MUnbox::Infallible); def->block()->insertBefore(def, unbox); def->replaceOperand(1, unbox); + if (!unbox->typePolicy()->adjustInputs(alloc, unbox)) + return false; } JS_ASSERT(def->getOperand(0)->type() != MIRType_String); @@ -190,30 +190,20 @@ ComparePolicy::adjustInputs(TempAllocator &alloc, MInstruction *def) switch (type) { case MIRType_Double: { - MToDouble::ConversionKind convert = MToDouble::NumbersOnly; + MToFPInstruction::ConversionKind convert = MToFPInstruction::NumbersOnly; if (compare->compareType() == MCompare::Compare_DoubleMaybeCoerceLHS && i == 0) - convert = MToDouble::NonNullNonStringPrimitives; + convert = MToFPInstruction::NonNullNonStringPrimitives; else if (compare->compareType() == MCompare::Compare_DoubleMaybeCoerceRHS && i == 1) - convert = MToDouble::NonNullNonStringPrimitives; - if (in->type() == MIRType_Null || - (in->type() == MIRType_Boolean && convert == MToDouble::NumbersOnly)) - { - in = boxAt(alloc, def, in); - } + convert = MToFPInstruction::NonNullNonStringPrimitives; replace = MToDouble::New(alloc, in, convert); break; } case MIRType_Float32: { - MToFloat32::ConversionKind convert = MToFloat32::NumbersOnly; + MToFPInstruction::ConversionKind convert = MToFPInstruction::NumbersOnly; if (compare->compareType() == MCompare::Compare_DoubleMaybeCoerceLHS && i == 0) - convert = MToFloat32::NonNullNonStringPrimitives; + convert = MToFPInstruction::NonNullNonStringPrimitives; else if (compare->compareType() == MCompare::Compare_DoubleMaybeCoerceRHS && i == 1) - convert = MToFloat32::NonNullNonStringPrimitives; - if (in->type() == MIRType_Null || - (in->type() == MIRType_Boolean && convert == MToFloat32::NumbersOnly)) - { - in = boxAt(alloc, def, in); - } + convert = MToFPInstruction::NonNullNonStringPrimitives; replace = MToFloat32::New(alloc, in, convert); break; } @@ -225,18 +215,6 @@ ComparePolicy::adjustInputs(TempAllocator &alloc, MInstruction *def) { convert = MacroAssembler::IntConversion_NumbersOrBoolsOnly; } - if (convert == MacroAssembler::IntConversion_NumbersOnly) { - if (in->type() != MIRType_Int32 && in->type() != MIRType_Value) - in = boxAt(alloc, def, in); - } else { - MOZ_ASSERT(convert == MacroAssembler::IntConversion_NumbersOrBoolsOnly); - if (in->type() != MIRType_Int32 && - in->type() != MIRType_Boolean && - in->type() != MIRType_Value) - { - in = boxAt(alloc, def, in); - } - } replace = MToInt32::New(alloc, in, convert); break; } @@ -386,13 +364,11 @@ StringPolicy::staticAdjustInputs(TempAllocator &alloc, MInstruction *ins) if (in->type() == MIRType_String) return true; - if (in->type() != MIRType_Value) - in = boxAt(alloc, ins, in); - MUnbox *replace = MUnbox::New(alloc, in, MIRType_String, MUnbox::Fallible); ins->block()->insertBefore(ins, replace); ins->replaceOperand(Op, replace); - return true; + + return replace->typePolicy()->adjustInputs(alloc, replace); } template bool StringPolicy<0>::staticAdjustInputs(TempAllocator &alloc, MInstruction *ins); @@ -428,13 +404,11 @@ IntPolicy::staticAdjustInputs(TempAllocator &alloc, MInstruction *def) if (in->type() == MIRType_Int32) return true; - if (in->type() != MIRType_Value) - in = boxAt(alloc, def, in); - MUnbox *replace = MUnbox::New(alloc, in, MIRType_Int32, MUnbox::Fallible); def->block()->insertBefore(def, replace); def->replaceOperand(Op, replace); - return true; + + return replace->typePolicy()->adjustInputs(alloc, replace); } template bool IntPolicy<0>::staticAdjustInputs(TempAllocator &alloc, MInstruction *def); @@ -544,12 +518,42 @@ template bool BoxExceptPolicy<2, MIRType_String>::staticAdjustInputs(TempAllocat bool ToDoublePolicy::staticAdjustInputs(TempAllocator &alloc, MInstruction *ins) { + JS_ASSERT(ins->isToDouble() || ins->isToFloat32()); + MDefinition *in = ins->getOperand(0); - if (in->type() != MIRType_Object && - in->type() != MIRType_String && - in->type() != MIRType_Symbol) - { + MToFPInstruction::ConversionKind conversion; + if (ins->isToDouble()) + conversion = ins->toToDouble()->conversion(); + else + conversion = ins->toToFloat32()->conversion(); + + switch (in->type()) { + case MIRType_Int32: + case MIRType_Float32: + case MIRType_Double: + case MIRType_Value: + // No need for boxing for these types. return true; + case MIRType_Null: + // No need for boxing, when we will convert. + if (conversion == MToFPInstruction::NonStringPrimitives) + return true; + break; + case MIRType_Undefined: + case MIRType_Boolean: + // No need for boxing, when we will convert. + if (conversion == MToFPInstruction::NonStringPrimitives) + return true; + if (conversion == MToFPInstruction::NonNullNonStringPrimitives) + return true; + break; + case MIRType_Object: + case MIRType_String: + case MIRType_Symbol: + // Objects might be effectful. Symbols give TypeError. + break; + default: + break; } in = boxAt(alloc, ins, in); @@ -562,26 +566,46 @@ ToInt32Policy::staticAdjustInputs(TempAllocator &alloc, MInstruction *ins) { JS_ASSERT(ins->isToInt32() || ins->isTruncateToInt32()); + MacroAssembler::IntConversionInputKind conversion = MacroAssembler::IntConversion_Any; + if (ins->isToInt32()) + conversion = ins->toToInt32()->conversion(); + MDefinition *in = ins->getOperand(0); switch (in->type()) { + case MIRType_Int32: + case MIRType_Float32: + case MIRType_Double: + case MIRType_Value: + // No need for boxing for these types. + return true; + case MIRType_Undefined: + // No need for boxing when truncating. + if (ins->isTruncateToInt32()) + return true; + break; + case MIRType_Null: + // No need for boxing, when we will convert. + if (conversion == MacroAssembler::IntConversion_Any) + return true; + break; + case MIRType_Boolean: + // No need for boxing, when we will convert. + if (conversion == MacroAssembler::IntConversion_Any) + return true; + if (conversion == MacroAssembler::IntConversion_NumbersOrBoolsOnly) + return true; + break; case MIRType_Object: case MIRType_String: case MIRType_Symbol: // Objects might be effectful. Symbols give TypeError. - in = boxAt(alloc, ins, in); - ins->replaceOperand(0, in); - break; - case MIRType_Undefined: - // Undefined coerce to NaN, not int32, when not truncated. - if (ins->isToInt32()) { - in = boxAt(alloc, ins, in); - ins->replaceOperand(0, in); - } break; default: break; } + in = boxAt(alloc, ins, in); + ins->replaceOperand(0, in); return true; }