Bug 1054972 - IonMonkey: GVN: Avoid setting UseRemoved flags unnecessarily. r=nbp, a=sledru
authorDan Gohman <sunfish@mozilla.com>
Tue, 02 Sep 2014 13:01:32 -0700
changeset 216746 316374007734
parent 216745 c0d46e44a6cb
child 216747 c5ee54bc44f8
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: Avoid setting UseRemoved flags unnecessarily. r=nbp, a=sledru
js/src/jit/IonAnalysis.cpp
js/src/jit/MIR.cpp
js/src/jit/MIR.h
js/src/jit/RangeAnalysis.cpp
js/src/jit/ValueNumbering.cpp
js/src/jit/ValueNumbering.h
--- a/js/src/jit/IonAnalysis.cpp
+++ b/js/src/jit/IonAnalysis.cpp
@@ -293,17 +293,17 @@ jit::EliminatePhis(MIRGenerator *mir, MI
         MPhiIterator iter = block->phisBegin();
         while (iter != block->phisEnd()) {
             // Flag all as unused, only observable phis would be marked as used
             // when processed by the work list.
             iter->setUnused();
 
             // If the phi is redundant, remove it here.
             if (MDefinition *redundant = IsPhiRedundant(*iter)) {
-                iter->replaceAllUsesWith(redundant);
+                iter->justReplaceAllUsesWith(redundant);
                 iter = block->discardPhiAt(iter);
                 continue;
             }
 
             // Enqueue observable Phis.
             if (IsPhiObservable(*iter, observe)) {
                 iter->setInWorklist();
                 if (!worklist.append(*iter))
@@ -331,17 +331,17 @@ jit::EliminatePhis(MIRGenerator *mir, MI
                     if (!use->isUnused()) {
                         use->setUnusedUnchecked();
                         use->setInWorklist();
                         if (!worklist.append(use))
                             return false;
                     }
                 }
             }
-            phi->replaceAllUsesWith(redundant);
+            phi->justReplaceAllUsesWith(redundant);
         } else {
             // Otherwise flag them as used.
             phi->setNotUnused();
         }
 
         // The current phi is/was used, so all its operands are used.
         for (size_t i = 0, e = phi->numOperands(); i < e; i++) {
             MDefinition *in = phi->getOperand(i);
@@ -729,17 +729,17 @@ TypeAnalyzer::replaceRedundantPhi(MPhi *
         v = MagicValue(JS_OPTIMIZED_OUT);
         break;
       default:
         MOZ_ASSUME_UNREACHABLE("unexpected type");
     }
     MConstant *c = MConstant::New(alloc(), v);
     // The instruction pass will insert the box
     block->insertBefore(*(block->begin()), c);
-    phi->replaceAllUsesWith(c);
+    phi->justReplaceAllUsesWith(c);
 }
 
 bool
 TypeAnalyzer::insertConversions()
 {
     // Instructions are processed in reverse postorder: all uses are defs are
     // seen before uses. This ensures that output adjustment (which may rewrite
     // inputs of uses) does not conflict with input adjustment.
--- a/js/src/jit/MIR.cpp
+++ b/js/src/jit/MIR.cpp
@@ -416,22 +416,28 @@ MDefinition::hasLiveDefUses() const
     }
 
     return false;
 }
 
 void
 MDefinition::replaceAllUsesWith(MDefinition *dom)
 {
+    for (size_t i = 0, e = numOperands(); i < e; ++i)
+        getOperand(i)->setUseRemovedUnchecked();
+
+    justReplaceAllUsesWith(dom);
+}
+
+void
+MDefinition::justReplaceAllUsesWith(MDefinition *dom)
+{
     JS_ASSERT(dom != nullptr);
     JS_ASSERT(dom != this);
 
-    for (size_t i = 0, e = numOperands(); i < e; i++)
-        getOperand(i)->setUseRemovedUnchecked();
-
     for (MUseIterator i(usesBegin()); i != usesEnd(); i++)
         i->setProducerUnchecked(dom);
     dom->uses_.takeElements(uses_);
 }
 
 bool
 MDefinition::emptyResultTypeSet() const
 {
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -627,16 +627,19 @@ class MDefinition : public MNode
     void addUse(MUse *use) {
         uses_.pushFront(use);
     }
     void addUseUnchecked(MUse *use) {
         uses_.pushFrontUnchecked(use);
     }
     void replaceAllUsesWith(MDefinition *dom);
 
+    // Like replaceAllUsesWith, but doesn't set UseRemoved on |this|'s operands.
+    void justReplaceAllUsesWith(MDefinition *dom);
+
     // Mark this instruction as having replaced all uses of ins, as during GVN,
     // returning false if the replacement should not be performed. For use when
     // GVN eliminates instructions which are not equivalent to one another.
     virtual bool updateForReplacement(MDefinition *ins) {
         return true;
     }
 
     void setVirtualRegister(uint32_t vreg) {
--- a/js/src/jit/RangeAnalysis.cpp
+++ b/js/src/jit/RangeAnalysis.cpp
@@ -267,17 +267,17 @@ RangeAnalysis::removeBetaNodes()
     for (PostorderIterator i(graph_.poBegin()); i != graph_.poEnd(); i++) {
         MBasicBlock *block = *i;
         for (MDefinitionIterator iter(*i); iter; ) {
             MDefinition *def = *iter;
             if (def->isBeta()) {
                 MDefinition *op = def->getOperand(0);
                 IonSpew(IonSpew_Range, "Removing beta node %d for %d",
                         def->id(), op->id());
-                def->replaceAllUsesWith(op);
+                def->justReplaceAllUsesWith(op);
                 iter = block->discardDefAt(iter);
             } else {
                 // We only place Beta nodes at the beginning of basic
                 // blocks, so if we see something else, we can move on
                 // to the next block.
                 break;
             }
         }
--- a/js/src/jit/ValueNumbering.cpp
+++ b/js/src/jit/ValueNumbering.cpp
@@ -148,24 +148,26 @@ IsDead(const MDefinition *def)
 // is deleted. TODO: This misses cases where the definition is used multiple
 // times by the same user (bug 1031396).
 static bool
 WillBecomeDead(const MDefinition *def)
 {
     return def->hasOneUse() && DeadIfUnused(def);
 }
 
-// Call MDefinition::replaceAllUsesWith, and add some GVN-specific asserts.
+// Call MDefinition::justReplaceAllUsesWith, and add some GVN-specific asserts.
 static void
 ReplaceAllUsesWith(MDefinition *from, MDefinition *to)
 {
     MOZ_ASSERT(from != to, "GVN shouldn't try to replace a value with itself");
     MOZ_ASSERT(from->type() == to->type(), "Def replacement has different type");
 
-    from->replaceAllUsesWith(to);
+    // We don't need the extra setting of UseRemoved flags that the regular
+    // replaceAllUsesWith does because we do it ourselves.
+    from->justReplaceAllUsesWith(to);
 }
 
 // Test whether succ is a successor of newControl.
 static bool
 HasSuccessor(const MControlInstruction *newControl, const MBasicBlock *succ)
 {
     for (size_t i = 0, e = newControl->numSuccessors(); i != e; ++i) {
         if (newControl->getSuccessor(i) == succ)
@@ -226,67 +228,72 @@ bool
 ValueNumberer::deleteDefsRecursively(MDefinition *def)
 {
     return deleteDef(def) && processDeadDefs();
 }
 
 // Assuming phi is dead, push each dead operand of phi not dominated by the phi
 // to the delete worklist.
 bool
-ValueNumberer::pushDeadPhiOperands(MPhi *phi, const MBasicBlock *phiBlock)
+ValueNumberer::pushDeadPhiOperands(MPhi *phi, const MBasicBlock *phiBlock,
+                                   UseRemovedOption useRemovedOption)
 {
     for (size_t o = 0, e = phi->numOperands(); o != e; ++o) {
         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();
+            if (useRemovedOption == SetUseRemoved)
+                op->setUseRemovedUnchecked();
         }
     }
     return true;
 }
 
 // Assuming ins is dead, push each dead operand of ins to the delete worklist.
 bool
-ValueNumberer::pushDeadInsOperands(MInstruction *ins)
+ValueNumberer::pushDeadInsOperands(MInstruction *ins,
+                                   UseRemovedOption useRemovedOption)
 {
     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();
+            if (useRemovedOption == SetUseRemoved)
+                op->setUseRemovedUnchecked();
         }
     }
     return true;
 }
 
 bool
-ValueNumberer::deleteDef(MDefinition *def)
+ValueNumberer::deleteDef(MDefinition *def,
+                         UseRemovedOption useRemovedOption)
 {
     IonSpew(IonSpew_GVN, "    Deleting %s%u", def->opName(), def->id());
     MOZ_ASSERT(IsDead(def), "Deleting non-dead definition");
     MOZ_ASSERT(!values_.has(def), "Deleting an instruction still in the set");
 
     if (def->isPhi()) {
         MPhi *phi = def->toPhi();
         MBasicBlock *phiBlock = phi->block();
-        if (!pushDeadPhiOperands(phi, phiBlock))
+        if (!pushDeadPhiOperands(phi, phiBlock, useRemovedOption))
              return false;
         MPhiIterator at(phiBlock->phisBegin(phi));
         phiBlock->discardPhiAt(at);
     } else {
         MInstruction *ins = def->toInstruction();
-        if (!pushDeadInsOperands(ins))
+        if (!pushDeadInsOperands(ins, useRemovedOption))
              return false;
         ins->block()->discard(ins);
     }
     return true;
 }
 
 // Recursively delete all the defs on the deadDefs_ worklist.
 bool
@@ -506,17 +513,17 @@ ValueNumberer::visitDefinition(MDefiniti
             // 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 (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);
+                mozilla::DebugOnly<bool> r = deleteDef(def, DontSetUseRemoved);
                 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;
         }
     }
--- a/js/src/jit/ValueNumbering.h
+++ b/js/src/jit/ValueNumbering.h
@@ -67,20 +67,28 @@ class ValueNumberer
     BlockWorklist unreachableBlocks_; // Worklist for unreachable blocks
     BlockWorklist remainingBlocks_;   // Blocks remaining with fewer preds
     size_t numBlocksDeleted_;         // Num deleted blocks in current tree
     bool rerun_;                      // Should we run another GVN iteration?
     bool blocksRemoved_;              // Have any blocks been removed?
     bool updateAliasAnalysis_;        // Do we care about AliasAnalysis?
     bool dependenciesBroken_;         // Have we broken AliasAnalysis?
 
+    enum UseRemovedOption {
+        DontSetUseRemoved,
+        SetUseRemoved
+    };
+
     bool deleteDefsRecursively(MDefinition *def);
-    bool pushDeadPhiOperands(MPhi *phi, const MBasicBlock *phiBlock);
-    bool pushDeadInsOperands(MInstruction *ins);
-    bool deleteDef(MDefinition *def);
+    bool pushDeadPhiOperands(MPhi *phi, const MBasicBlock *phiBlock,
+                             UseRemovedOption useRemovedOption = SetUseRemoved);
+    bool pushDeadInsOperands(MInstruction *ins,
+                             UseRemovedOption useRemovedOption = SetUseRemoved);
+    bool deleteDef(MDefinition *def,
+                   UseRemovedOption useRemovedOption = SetUseRemoved);
     bool processDeadDefs();
 
     bool removePredecessor(MBasicBlock *block, MBasicBlock *pred);
     bool removeBlocksRecursively(MBasicBlock *block, const MBasicBlock *root);
 
     MDefinition *simplified(MDefinition *def) const;
     MDefinition *leader(MDefinition *def);
     bool hasLeader(const MPhi *phi, const MBasicBlock *phiBlock) const;