diff --git a/js/src/ion/EdgeCaseAnalysis.cpp b/js/src/ion/EdgeCaseAnalysis.cpp index 85418a3811b..d17c2694b3d 100644 --- a/js/src/ion/EdgeCaseAnalysis.cpp +++ b/js/src/ion/EdgeCaseAnalysis.cpp @@ -25,11 +25,16 @@ EdgeCaseAnalysis::EdgeCaseAnalysis(MIRGenerator *mir, MIRGraph &graph) bool EdgeCaseAnalysis::analyzeLate() { + // Renumber definitions for NeedNegativeZeroCheck under analyzeEdgeCasesBackward. + uint32 nextId = 1; + for (ReversePostorderIterator block(graph.rpoBegin()); block != graph.rpoEnd(); block++) { if (mir->shouldCancel("Analyze Late (first loop)")) return false; - for (MDefinitionIterator iter(*block); iter; iter++) + for (MDefinitionIterator iter(*block); iter; iter++) { + iter->setId(nextId++); iter->analyzeEdgeCasesForward(); + } } for (PostorderIterator block(graph.poBegin()); block != graph.poEnd(); block++) { diff --git a/js/src/ion/Ion.cpp b/js/src/ion/Ion.cpp index bde7c5a4ee5..572274d3a33 100644 --- a/js/src/ion/Ion.cpp +++ b/js/src/ion/Ion.cpp @@ -919,6 +919,9 @@ CompileBackEnd(MIRGenerator *mir) if (mir->shouldCancel("DCE")) return NULL; + // Passes after this point must not move instructions; these analyses + // depend on knowing the final order in which instructions will execute. + if (js_IonOptions.edgeCaseAnalysis) { EdgeCaseAnalysis edgeCaseAnalysis(mir, graph); if (!edgeCaseAnalysis.analyzeLate()) diff --git a/js/src/ion/MIR.cpp b/js/src/ion/MIR.cpp index 2f2c46face8..7ebc5927b67 100644 --- a/js/src/ion/MIR.cpp +++ b/js/src/ion/MIR.cpp @@ -591,43 +591,53 @@ NeedNegativeZeroCheck(MDefinition *def) // Test if all uses have the same symantic for -0 and 0 for (MUseIterator use = def->usesBegin(); use != def->usesEnd(); use++) { if (use->node()->isResumePoint()) - return true; + continue; MDefinition *use_def = use->node()->toDefinition(); switch (use_def->op()) { case MDefinition::Op_Add: { // x + y gives -0, when both x and y are -0 - // - When other operand can't produce -0 (i.e. all opcodes, except Mul/Div/ToInt32) - // Remove negative zero check on this operand - // - When both operands can produce -0 (both Mul/Div/ToInt32 opcode) - // We can remove the check eagerly on this operand. - MDefinition *operand = use_def->getOperand(0); - if (operand == def) { - operand = use_def->getOperand(1); - // Don't remove check when both operands are same definition - // As removing it from one operand, will remove it from both. - if (operand == def) - return true; + // Figure out the order in which the addition's operands will + // execute. EdgeCaseAnalysis::analyzeLate has renumbered the MIR + // definitions for us so that this just requires comparing ids. + MDefinition *first = use_def->getOperand(0); + MDefinition *second = use_def->getOperand(1); + if (first->id() > second->id()) { + MDefinition *temp = first; + first = second; + second = temp; } - // Check if check is possibly eagerly removed on other operand - // and don't remove check eagerly on this operand in that case. - if (operand->isMul()) { - MMul *mul = operand->toMul(); - if (!mul->canBeNegativeZero()) + if (def == first) { + // Negative zero checks can be removed on the first executed + // operand only if it is guaranteed the second executed operand + // will produce a value other than -0. While the second is + // typed as an int32, a bailout taken between execution of the + // operands may change that type and cause a -0 to flow to the + // second. + // + // There is no way to test whether there are any bailouts + // between execution of the operands, so remove negative + // zero checks from the first only if the second's type is + // independent from type changes that may occur after bailing. + switch (second->op()) { + case MDefinition::Op_Constant: + case MDefinition::Op_BitAnd: + case MDefinition::Op_BitOr: + case MDefinition::Op_BitXor: + case MDefinition::Op_BitNot: + case MDefinition::Op_Lsh: + case MDefinition::Op_Rsh: + break; + default: return true; - } else if (operand->isDiv()) { - MDiv *div = operand->toDiv(); - if (!div->canBeNegativeZero()) - return true; - } else if (operand->isToInt32()) { - MToInt32 *int32 = operand->toToInt32(); - if (!int32->canBeNegativeZero()) - return true; - } else if (operand->isPhi()) { - return true; + } } + + // The negative zero check can always be removed on the second + // executed operand; by the time this executes the first will have + // been evaluated as int32 and the addition's result cannot be -0. break; } case MDefinition::Op_StoreElement: diff --git a/js/src/jit-test/tests/ion/bug816786.js b/js/src/jit-test/tests/ion/bug816786.js new file mode 100644 index 00000000000..dde74096321 --- /dev/null +++ b/js/src/jit-test/tests/ion/bug816786.js @@ -0,0 +1,38 @@ +var g; +function test(a, b) { + + g = 0; + for(var i=0; i<100; i++) { + g += i + } + + var t = a*b; + + for(var i=0; i<100; i++) { + t = x.y + t; + return t; + } + +} + +function negzero(x) { + return x===0 && (1/x)===-Infinity; +} + + +var x = {y:0}; +var a = 0; +var b = 0; +for(var i=0; i<58; i++) { + var o = test(a, b); + + // Test returns + // * 0, if i < 50 + // * -0, if i >= 50 + assertEq(negzero(o), i>50); + + if (i == 50) { + a = -1 + x.y = -0 + } +}