Bug 1054972 - IonMonkey: GVN: More misc cleanups. r=nbp, a=sledru
authorDan Gohman <sunfish@mozilla.com>
Tue, 02 Sep 2014 13:01:32 -0700
changeset 216745 c0d46e44a6cb
parent 216744 94dc71a06159
child 216746 316374007734
push id3896
push userryanvm@gmail.com
push date2014-09-15 19:15 +0000
treeherdermozilla-beta@c5ee54bc44f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, sledru
bugs1054972
milestone33.0
Bug 1054972 - IonMonkey: GVN: More misc cleanups. r=nbp, a=sledru
js/src/jit/ValueNumbering.cpp
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -237,34 +237,34 @@ ValueNumberer::pushDeadPhiOperands(MPhi 
         MDefinition *op = phi->getOperand(o);
         if (WillBecomeDead(op) && !op->isInWorklist() &&
             !phiBlock->dominates(phiBlock->getPredecessor(o)))
         {
             op->setInWorklist();
             if (!deadDefs_.append(op))
                 return false;
         } else {
-           op->setUseRemovedUnchecked();
+            op->setUseRemovedUnchecked();
         }
     }
     return true;
 }
 
 // Assuming ins is dead, push each dead operand of ins to the delete worklist.
 bool
 ValueNumberer::pushDeadInsOperands(MInstruction *ins)
 {
     for (size_t o = 0, e = ins->numOperands(); o != e; ++o) {
         MDefinition *op = ins->getOperand(o);
         if (WillBecomeDead(op) && !op->isInWorklist()) {
             op->setInWorklist();
             if (!deadDefs_.append(op))
                 return false;
         } else {
-           op->setUseRemovedUnchecked();
+            op->setUseRemovedUnchecked();
         }
     }
     return true;
 }
 
 bool
 ValueNumberer::deleteDef(MDefinition *def)
 {
@@ -480,18 +480,20 @@ ValueNumberer::visitDefinition(MDefiniti
                 def->opName(), def->id(), sim->opName(), sim->id());
         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 deleted.
         def->setNotGuardUnchecked();
 
-        if (IsDead(def) && !deleteDefsRecursively(def))
-            return false;
+        if (DeadIfUnused(def)) {
+            if (!deleteDefsRecursively(def))
+                return false;
+        }
         def = sim;
     }
 
     // Look for a dominating def which makes this def redundant.
     MDefinition *rep = leader(def);
     if (rep != def) {
         if (rep == nullptr)
             return false;
@@ -501,18 +503,25 @@ ValueNumberer::visitDefinition(MDefiniti
                     def->opName(), def->id(), rep->opName(), rep->id());
             ReplaceAllUsesWith(def, rep);
 
             // The node's congruentTo said |def| is congruent to |rep|, and it's
             // dominated by |rep|. If |def| is a guard, it's covered by |rep|,
             // so we can clear |def|'s guard flag and let it be deleted.
             def->setNotGuardUnchecked();
 
-            if (IsDead(def) && !deleteDefsRecursively(def))
-                return false;
+            if (DeadIfUnused(def)) {
+                // deleteDef should not add anything to the deadDefs, as the
+                // redundant operation should have the same input operands.
+                mozilla::DebugOnly<bool> r = deleteDef(def);
+                MOZ_ASSERT(r, "deleteDef shouldn't have tried to add anything to the worklist, "
+                              "so it shouldn't have failed");
+                MOZ_ASSERT(deadDefs_.empty(),
+                           "deleteDef shouldn't have added anything to the worklist");
+            }
             def = rep;
         }
     }
 
     // If this instruction has a dependency() into an unreachable block, we'll
     // need to update AliasAnalysis.
     if (updateAliasAnalysis_ && !dependenciesBroken_) {
         const MDefinition *dep = def->dependency();