From d55a736aabea4411393bd88e58b3dbbe1757e0a2 Mon Sep 17 00:00:00 2001 From: Hannes Verschore Date: Wed, 22 Oct 2014 22:12:45 +0200 Subject: [PATCH] Bug 1042823 - Don't blindly make MMinMax optimize for doubles, r=jandem,sunfish --- js/src/builtin/Utilities.js | 2 +- js/src/jit/MCallOptimize.cpp | 51 ++++++++++++++--- js/src/jit/MIR.cpp | 106 ++++++++++++++++++++++++++++++++++- js/src/jit/MIR.h | 3 +- 4 files changed, 149 insertions(+), 13 deletions(-) diff --git a/js/src/builtin/Utilities.js b/js/src/builtin/Utilities.js index 1dd5a3dfd50..a7af0d67d2d 100644 --- a/js/src/builtin/Utilities.js +++ b/js/src/builtin/Utilities.js @@ -100,7 +100,7 @@ function ToLength(v) { return 0; // Math.pow(2, 53) - 1 = 0x1fffffffffffff - return v < 0x1fffffffffffff ? v : 0x1fffffffffffff; + return std_Math_min(v, 0x1fffffffffffff); } /********** Testing code **********/ diff --git a/js/src/jit/MCallOptimize.cpp b/js/src/jit/MCallOptimize.cpp index deb7829456f..4636b646670 100644 --- a/js/src/jit/MCallOptimize.cpp +++ b/js/src/jit/MCallOptimize.cpp @@ -1147,31 +1147,64 @@ IonBuilder::inlineMathFRound(CallInfo &callInfo) IonBuilder::InliningStatus IonBuilder::inlineMathMinMax(CallInfo &callInfo, bool max) { - if (callInfo.argc() < 2 || callInfo.constructing()) + if (callInfo.argc() < 1 || callInfo.constructing()) return InliningStatus_NotInlined; MIRType returnType = getInlineReturnType(); if (!IsNumberType(returnType)) return InliningStatus_NotInlined; + MDefinitionVector int32_cases(alloc()); for (unsigned i = 0; i < callInfo.argc(); i++) { - MIRType argType = callInfo.getArg(i)->type(); - if (!IsNumberType(argType)) - return InliningStatus_NotInlined; + MDefinition *arg = callInfo.getArg(i); - // When one of the arguments is double, do a double MMinMax. - if (returnType == MIRType_Int32 && IsFloatingPointType(argType)) + switch (arg->type()) { + case MIRType_Int32: + if (!int32_cases.append(arg)) + return InliningStatus_Error; + break; + case MIRType_Double: + case MIRType_Float32: + // Don't force a double MMinMax for arguments that would be a NOP + // when doing an integer MMinMax. + if (arg->isConstant()) { + double cte = arg->toConstant()->value().toDouble(); + // min(int32, cte >= INT32_MAX) = int32 + if (cte >= INT32_MAX && !max) + break; + // max(int32, cte <= INT32_MIN) = int32 + if (cte <= INT32_MIN && max) + break; + } + + // Force double MMinMax if argument is a "effectfull" double. returnType = MIRType_Double; + break; + default: + return InliningStatus_NotInlined; + } } + if (int32_cases.length() == 0) + returnType = MIRType_Double; + callInfo.setImplicitlyUsedUnchecked(); + MDefinitionVector &cases = (returnType == MIRType_Int32) ? int32_cases : callInfo.argv(); + + if (cases.length() == 1) { + MLimitedTruncate *limit = MLimitedTruncate::New(alloc(), cases[0], MDefinition::NoTruncate); + current->add(limit); + current->push(limit); + return InliningStatus_Inlined; + } + // Chain N-1 MMinMax instructions to compute the MinMax. - MMinMax *last = MMinMax::New(alloc(), callInfo.getArg(0), callInfo.getArg(1), returnType, max); + MMinMax *last = MMinMax::New(alloc(), cases[0], cases[1], returnType, max); current->add(last); - for (unsigned i = 2; i < callInfo.argc(); i++) { - MMinMax *ins = MMinMax::New(alloc(), last, callInfo.getArg(i), returnType, max); + for (unsigned i = 2; i < cases.length(); i++) { + MMinMax *ins = MMinMax::New(alloc(), last, cases[2], returnType, max); current->add(ins); last = ins; } diff --git a/js/src/jit/MIR.cpp b/js/src/jit/MIR.cpp index eaa5485f2e7..736222ca9cf 100644 --- a/js/src/jit/MIR.cpp +++ b/js/src/jit/MIR.cpp @@ -1830,6 +1830,41 @@ MMinMax::trySpecializeFloat32(TempAllocator &alloc) setResultType(MIRType_Float32); } +MDefinition * +MMinMax::foldsTo(TempAllocator &alloc) +{ + if (!lhs()->isConstant() && !rhs()->isConstant()) + return this; + + MDefinition *operand = lhs()->isConstant() ? rhs() : lhs(); + MConstant *constant = lhs()->isConstant() ? lhs()->toConstant() : rhs()->toConstant(); + + if (operand->isToDouble() && operand->getOperand(0)->type() == MIRType_Int32) { + const js::Value &val = constant->value(); + + // min(int32, cte >= INT32_MAX) = int32 + if (val.isDouble() && val.toDouble() >= INT32_MAX && !isMax()) { + MLimitedTruncate *limit = + MLimitedTruncate::New(alloc, operand->getOperand(0), MDefinition::NoTruncate); + block()->insertBefore(this, limit); + MToDouble *toDouble = MToDouble::New(alloc, limit); + block()->insertBefore(this, toDouble); + return toDouble; + } + + // max(int32, cte <= INT32_MIN) = int32 + if (val.isDouble() && val.toDouble() < INT32_MIN && isMax()) { + MLimitedTruncate *limit = + MLimitedTruncate::New(alloc, operand->getOperand(0), MDefinition::NoTruncate); + block()->insertBefore(this, limit); + MToDouble *toDouble = MToDouble::New(alloc, limit); + block()->insertBefore(this, toDouble); + return toDouble; + } + } + return this; +} + bool MAbs::fallible() const { @@ -2976,7 +3011,7 @@ MCompare::tryFold(bool *result) } bool -MCompare::evaluateConstantOperands(bool *result) +MCompare::evaluateConstantOperands(TempAllocator &alloc, bool *result) { if (type() != MIRType_Boolean && type() != MIRType_Int32) return false; @@ -2984,6 +3019,73 @@ MCompare::evaluateConstantOperands(bool *result) MDefinition *left = getOperand(0); MDefinition *right = getOperand(1); + if (compareType() == Compare_Double) { + // Optimize "MCompare MConstant (MToDouble SomethingInInt32Range). + // In most cases the MToDouble was added, because the constant is + // a double. + // e.g. v < 9007199254740991, where v is an int32 is always true. + if (!lhs()->isConstant() && !rhs()->isConstant()) + return false; + + MDefinition *operand = left->isConstant() ? right : left; + MConstant *constant = left->isConstant() ? left->toConstant() : right->toConstant(); + MOZ_ASSERT(constant->value().isDouble()); + double cte = constant->value().toDouble(); + + if (operand->isToDouble() && operand->getOperand(0)->type() == MIRType_Int32) { + bool replaced = false; + switch (jsop_) { + case JSOP_LT: + if (cte > INT32_MAX || cte < INT32_MIN) { + *result = !((constant == lhs()) ^ (cte < INT32_MIN)); + replaced = true; + } + break; + case JSOP_LE: + if (cte >= INT32_MAX || cte <= INT32_MIN) { + *result = !((constant == lhs()) ^ (cte <= INT32_MIN)); + replaced = true; + } + break; + case JSOP_GT: + if (cte > INT32_MAX || cte < INT32_MIN) { + *result = !((constant == rhs()) ^ (cte < INT32_MIN)); + replaced = true; + } + break; + case JSOP_GE: + if (cte >= INT32_MAX || cte <= INT32_MIN) { + *result = !((constant == rhs()) ^ (cte <= INT32_MIN)); + replaced = true; + } + break; + case JSOP_STRICTEQ: // Fall through. + case JSOP_EQ: + if (cte > INT32_MAX || cte < INT32_MIN) { + *result = false; + replaced = true; + } + break; + case JSOP_STRICTNE: // Fall through. + case JSOP_NE: + if (cte > INT32_MAX || cte < INT32_MIN) { + *result = true; + replaced = true; + } + break; + default: + MOZ_CRASH("Unexpected op."); + } + if (replaced) { + MLimitedTruncate *limit = + MLimitedTruncate::New(alloc, operand->getOperand(0), MDefinition::NoTruncate); + limit->setGuardUnchecked(); + block()->insertBefore(this, limit); + return true; + } + } + } + if (!left->isConstant() || !right->isConstant()) return false; @@ -3092,7 +3194,7 @@ MCompare::foldsTo(TempAllocator &alloc) { bool result; - if (tryFold(&result) || evaluateConstantOperands(&result)) { + if (tryFold(&result) || evaluateConstantOperands(alloc, &result)) { if (type() == MIRType_Int32) return MConstant::New(alloc, Int32Value(result)); diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index cfad935eb7b..537ba058c75 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -3643,7 +3643,7 @@ class MCompare CompareType compareType); bool tryFold(bool *result); - bool evaluateConstantOperands(bool *result); + bool evaluateConstantOperands(TempAllocator &alloc, bool *result); MDefinition *foldsTo(TempAllocator &alloc); void filtersUndefinedOrNull(bool trueBranch, MDefinition **subject, bool *filtersUndefined, bool *filtersNull); @@ -5008,6 +5008,7 @@ class MMinMax AliasSet getAliasSet() const { return AliasSet::None(); } + MDefinition *foldsTo(TempAllocator &alloc); void computeRange(TempAllocator &alloc); bool writeRecoverData(CompactBufferWriter &writer) const; bool canRecoverOnBailout() const {