Bug 1054972 - IonMonkey: GVN: More misc cleanups r=nbp
authorDan Gohman <sunfish@mozilla.com>
Tue, 02 Sep 2014 13:01:32 -0700
changeset 203265 afac7b1435bc55c1dacb792a4e28d4448b8ae88c
parent 203264 61f05ae95aa46a44f84ad462fa9913aaa2e1b713
child 203266 0cf223c85b3b06e63a7ff66b634454a5176986df
push id27425
push userryanvm@gmail.com
push dateWed, 03 Sep 2014 20:38:59 +0000
treeherdermozilla-central@acbdce59da2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp
bugs1054972
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1054972 - IonMonkey: GVN: More misc cleanups r=nbp
js/src/jit/ValueNumbering.cpp
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -227,17 +227,17 @@ ValueNumberer::discardPhiOperands(MPhi *
     // MPhi saves operands in a vector so we iterate in reverse.
     for (int o = phi->numOperands() - 1; o >= 0; --o) {
         MDefinition *op = phi->getOperand(o);
         phi->removeOperand(o);
         if (IsDead(op) && !phiBlock->dominates(op->block())) {
             if (!deadDefs_.append(op))
                 return false;
         } else {
-           op->setUseRemovedUnchecked();
+            op->setUseRemovedUnchecked();
         }
     }
     return true;
 }
 
 // Assuming ins is dead, discard its operands. If an operand becomes dead, push
 // it to the delete worklist.
 bool
@@ -245,17 +245,17 @@ ValueNumberer::discardInsOperands(MInstr
 {
     for (size_t o = 0, e = ins->numOperands(); o != e; ++o) {
         MDefinition *op = ins->getOperand(o);
         ins->discardOperand(o);
         if (IsDead(op)) {
             if (!deadDefs_.append(op))
                 return false;
         } else {
-           op->setUseRemovedUnchecked();
+            op->setUseRemovedUnchecked();
         }
     }
     return true;
 }
 
 bool
 ValueNumberer::deleteDef(MDefinition *def)
 {
@@ -469,18 +469,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;
@@ -490,18 +492,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();