Bug 1314438: IonMonkey - Guard we don't remove instructions where we optimized based on its type, r=nbp
authorHannes Verschore <hv1989@gmail.com>
Mon, 07 Nov 2016 09:38:05 +0100
changeset 352015 6c0d7c338607bf58a09d9a9c9bbece1e8bd5321a
parent 352014 d23f304395196c652078347c7677901d15871a23
child 352016 d38d06f85ef59c5dbb5d4a1a8d895957a78714de
child 352079 07bbb0e3dde46ed12af7dc881695097cc16af045
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1314438
milestone52.0a1
Bug 1314438: IonMonkey - Guard we don't remove instructions where we optimized based on its type, r=nbp
js/src/jit-test/tests/ion/bug1314438.js
js/src/jit/MIR.cpp
js/src/jit/RangeAnalysis.cpp
js/src/jit/ValueNumbering.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1314438.js
@@ -0,0 +1,6 @@
+
+function g(x) {
+    return (-1 % x && Math.cos(8) >>> 0);
+}
+g(2);
+assertEq(uneval(g(-1)), "-0");
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -2385,16 +2385,18 @@ MPhi::foldsTernary(TempAllocator& alloc)
     {
         return nullptr;
     }
 
     // If testArg is an int32 type we can:
     // - fold testArg ? testArg : 0 to testArg
     // - fold testArg ? 0 : testArg to 0
     if (testArg->type() == MIRType::Int32 && c->numberToDouble() == 0) {
+        testArg->setGuardRangeBailoutsUnchecked();
+
         // When folding to the constant we need to hoist it.
         if (trueDef == c && !c->block()->dominates(block()))
             c->block()->moveBefore(pred->lastIns(), c);
         return trueDef;
     }
 
     // If testArg is an double type we can:
     // - fold testArg ? testArg : 0.0 to MNaNToZero(testArg)
@@ -2916,16 +2918,19 @@ CanProduceNegativeZero(MDefinition* def)
         default:
             return true;
     }
 }
 
 static inline bool
 NeedNegativeZeroCheck(MDefinition* def)
 {
+    if (def->isGuardRangeBailouts())
+        return true;
+
     // Test if all uses have the same semantics for -0 and 0
     for (MUseIterator use = def->usesBegin(); use != def->usesEnd(); use++) {
         if (use->consumer()->isResumePoint())
             continue;
 
         MDefinition* use_def = use->consumer()->toDefinition();
         switch (use_def->op()) {
           case MDefinition::Op_Add: {
@@ -4382,18 +4387,17 @@ MCompare::tryFoldEqualOperands(bool* res
                compareType_ == Compare_String || compareType_ == Compare_StrictString ||
                compareType_ == Compare_Object || compareType_ == Compare_Bitwise);
 
     if (isDoubleComparison() || isFloat32Comparison()) {
         if (!operandsAreNeverNaN())
             return false;
     }
 
-    if (DeadIfUnused(lhs()))
-        lhs()->setGuardRangeBailouts();
+    lhs()->setGuardRangeBailoutsUnchecked();
 
     *result = (jsop() == JSOP_STRICTEQ);
     return true;
 }
 
 bool
 MCompare::tryFoldTypeOf(bool* result)
 {
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -3535,18 +3535,17 @@ RangeAnalysis::prepareForUCE(bool* shoul
         // If the false-branch is unreachable, then the test condition must be true.
         // If the true-branch is unreachable, then the test condition must be false.
         MOZ_ASSERT(block == test->ifTrue() || block == test->ifFalse());
         bool value = block == test->ifFalse();
         MConstant* constant = MConstant::New(alloc().fallible(), BooleanValue(value));
         if (!constant)
             return false;
 
-        if (DeadIfUnused(condition))
-            condition->setGuardRangeBailoutsUnchecked();
+        condition->setGuardRangeBailoutsUnchecked();
 
         test->block()->insertBefore(test, constant);
 
         test->replaceOperand(0, constant);
         JitSpew(JitSpew_Range, "Update condition of %d to reflect unreachable branches.",
                 test->id());
 
         *shouldRemoveDeadCode = true;
@@ -3572,23 +3571,24 @@ bool RangeAnalysis::tryRemovingGuards()
 
     // Flag all fallible instructions which were indirectly used in the
     // computation of the condition, such that we do not ignore
     // bailout-paths which are used to shrink the input range of the
     // operands of the condition.
     for (size_t i = 0; i < guards.length(); i++) {
         MDefinition* guard = guards[i];
 
-#ifdef DEBUG
-        // There is no need to mark an instructions if there is
-        // already a more restrictive flag on it.
+        // If this ins is a guard even without guardRangeBailouts,
+        // there is no reason in trying to hoist the guardRangeBailouts check.
         guard->setNotGuardRangeBailouts();
-        MOZ_ASSERT(DeadIfUnused(guard));
+        if (!DeadIfUnused(guard)) {
+            guard->setGuardRangeBailouts();
+            continue;
+        }
         guard->setGuardRangeBailouts();
-#endif
 
         if (!guard->isPhi()) {
             if (!guard->range())
                 continue;
 
             // Filter the range of the instruction based on its MIRType.
             Range typeFilteredRange(guard);
 
@@ -3609,21 +3609,16 @@ bool RangeAnalysis::tryRemovingGuards()
             MDefinition* operand = guard->getOperand(op);
 
             // Already marked.
             if (operand->isInWorklist())
                 continue;
 
             MOZ_ASSERT(!operand->isGuardRangeBailouts());
 
-            // No need to mark as a guard, since it is has already an even more
-            // restrictive flag set.
-            if (!DeadIfUnused(operand))
-                continue;
-
             operand->setInWorklist();
             operand->setGuardRangeBailouts();
             if (!guards.append(operand))
                 return false;
         }
     }
 
     for (size_t i = 0; i < guards.length(); i++) {
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -792,16 +792,19 @@ ValueNumberer::visitDefinition(MDefiniti
         MOZ_ASSERT(!sim->isDiscarded());
         ReplaceAllUsesWith(def, sim);
 
         // The node's foldsTo said |def| can be replaced by |rep|. If |def| is a
         // guard, then either |rep| is also a guard, or a guard isn't actually
         // needed, so we can clear |def|'s guard flag and let it be discarded.
         def->setNotGuardUnchecked();
 
+        if (def->isGuardRangeBailouts())
+            sim->setGuardRangeBailoutsUnchecked();
+
         if (DeadIfUnused(def)) {
             if (!discardDefsRecursively(def))
                 return false;
 
             // If that ended up discarding |sim|, then we're done here.
             if (sim->isDiscarded())
                 return true;
         }