Bug 1029830 - IonMonkey: GVN: Say "discard" instead of "delete" r=nbp

This commit is contained in:
Dan Gohman 2014-09-17 10:27:24 -07:00
parent 3c27b74805
commit 799b9163d3
2 changed files with 46 additions and 45 deletions

View File

@ -105,7 +105,7 @@ ValueNumberer::VisibleValues::overwrite(AddPtr p, MDefinition *def)
set_.rekeyInPlace(p, def); set_.rekeyInPlace(p, def);
} }
// The given def will be deleted, so remove it from any sets. // The given def will be discarded, so remove it from any sets.
void void
ValueNumberer::VisibleValues::forget(const MDefinition *def) ValueNumberer::VisibleValues::forget(const MDefinition *def)
{ {
@ -215,16 +215,16 @@ IsDominatorRefined(MBasicBlock *block)
return false; return false;
} }
// Delete the given instruction and anything in its use-def subtree which is no // Discard the given instruction and anything in its use-def subtree which is no
// longer needed. // longer needed.
bool bool
ValueNumberer::deleteDefsRecursively(MDefinition *def) ValueNumberer::discardDefsRecursively(MDefinition *def)
{ {
return deleteDef(def) && processDeadDefs(); return discardDef(def) && processDeadDefs();
} }
// Assuming phi is dead, release its operands. If an operand which is not // Assuming phi is dead, release its operands. If an operand which is not
// dominated by the phi becomes dead, push it to the delete worklist. // dominated by the phi becomes dead, push it to the discard worklist.
bool bool
ValueNumberer::releasePhiOperands(MPhi *phi, const MBasicBlock *phiBlock, ValueNumberer::releasePhiOperands(MPhi *phi, const MBasicBlock *phiBlock,
UseRemovedOption useRemovedOption) UseRemovedOption useRemovedOption)
@ -245,7 +245,7 @@ ValueNumberer::releasePhiOperands(MPhi *phi, const MBasicBlock *phiBlock,
} }
// Assuming ins is dead, release its operands. If an operand becomes dead, push // Assuming ins is dead, release its operands. If an operand becomes dead, push
// it to the delete worklist. // it to the discard worklist.
bool bool
ValueNumberer::releaseInsOperands(MInstruction *ins, ValueNumberer::releaseInsOperands(MInstruction *ins,
UseRemovedOption useRemovedOption) UseRemovedOption useRemovedOption)
@ -265,12 +265,12 @@ ValueNumberer::releaseInsOperands(MInstruction *ins,
} }
bool bool
ValueNumberer::deleteDef(MDefinition *def, ValueNumberer::discardDef(MDefinition *def,
UseRemovedOption useRemovedOption) UseRemovedOption useRemovedOption)
{ {
JitSpew(JitSpew_GVN, " Deleting %s%u", def->opName(), def->id()); JitSpew(JitSpew_GVN, " Discarding %s%u", def->opName(), def->id());
MOZ_ASSERT(IsDead(def), "Deleting non-dead definition"); MOZ_ASSERT(IsDead(def), "Discarding non-dead definition");
MOZ_ASSERT(!values_.has(def), "Deleting an instruction still in the set"); MOZ_ASSERT(!values_.has(def), "Discarding an instruction still in the set");
if (def->isPhi()) { if (def->isPhi()) {
MPhi *phi = def->toPhi(); MPhi *phi = def->toPhi();
@ -288,7 +288,7 @@ ValueNumberer::deleteDef(MDefinition *def,
return true; return true;
} }
// Recursively delete all the defs on the deadDefs_ worklist. // Recursively discard all the defs on the deadDefs_ worklist.
bool bool
ValueNumberer::processDeadDefs() ValueNumberer::processDeadDefs()
{ {
@ -296,20 +296,20 @@ ValueNumberer::processDeadDefs()
MDefinition *def = deadDefs_.popCopy(); MDefinition *def = deadDefs_.popCopy();
values_.forget(def); values_.forget(def);
if (!deleteDef(def)) if (!discardDef(def))
return false; return false;
} }
return true; return true;
} }
// Delete an edge from the CFG. Return true if the block becomes unreachable. // Remove an edge from the CFG. Return true if the block becomes unreachable.
bool bool
ValueNumberer::removePredecessor(MBasicBlock *block, MBasicBlock *pred) ValueNumberer::removePredecessor(MBasicBlock *block, MBasicBlock *pred)
{ {
bool isUnreachableLoop = false; bool isUnreachableLoop = false;
if (block->isLoopHeader()) { if (block->isLoopHeader()) {
if (block->loopPredecessor() == pred) { if (block->loopPredecessor() == pred) {
// Deleting the entry into the loop makes the loop unreachable. // Discarding the entry into the loop makes the loop unreachable.
isUnreachableLoop = true; isUnreachableLoop = true;
JitSpew(JitSpew_GVN, " Loop with header block%u is no longer reachable", block->id()); JitSpew(JitSpew_GVN, " Loop with header block%u is no longer reachable", block->id());
#ifdef DEBUG #ifdef DEBUG
@ -320,14 +320,14 @@ ValueNumberer::removePredecessor(MBasicBlock *block, MBasicBlock *pred)
} }
// TODO: Removing a predecessor removes operands from phis, and these // TODO: Removing a predecessor removes operands from phis, and these
// operands may become dead. We should detect this and delete them. // operands may become dead. We should detect this and discard them.
// In practice though, when this happens, we often end up re-running GVN // In practice though, when this happens, we often end up re-running GVN
// for other reasons anyway. // for other reasons anyway.
block->removePredecessor(pred); block->removePredecessor(pred);
return block->numPredecessors() == 0 || isUnreachableLoop; return block->numPredecessors() == 0 || isUnreachableLoop;
} }
// Delete the given block and any block in its dominator subtree. // Discard the given block and any block in its dominator subtree.
bool bool
ValueNumberer::removeBlocksRecursively(MBasicBlock *start, const MBasicBlock *dominatorRoot) ValueNumberer::removeBlocksRecursively(MBasicBlock *start, const MBasicBlock *dominatorRoot)
{ {
@ -363,25 +363,25 @@ ValueNumberer::removeBlocksRecursively(MBasicBlock *start, const MBasicBlock *do
} }
#ifdef DEBUG #ifdef DEBUG
JitSpew(JitSpew_GVN, " Deleting block%u%s%s%s", block->id(), JitSpew(JitSpew_GVN, " Discarding block%u%s%s%s", block->id(),
block->isLoopHeader() ? " (loop header)" : "", block->isLoopHeader() ? " (loop header)" : "",
block->isSplitEdge() ? " (split edge)" : "", block->isSplitEdge() ? " (split edge)" : "",
block->immediateDominator() == block ? " (dominator root)" : ""); block->immediateDominator() == block ? " (dominator root)" : "");
for (MDefinitionIterator iter(block); iter; iter++) { for (MDefinitionIterator iter(block); iter; iter++) {
MDefinition *def = *iter; MDefinition *def = *iter;
JitSpew(JitSpew_GVN, " Deleting %s%u", def->opName(), def->id()); JitSpew(JitSpew_GVN, " Discarding %s%u", def->opName(), def->id());
} }
MControlInstruction *control = block->lastIns(); MControlInstruction *control = block->lastIns();
JitSpew(JitSpew_GVN, " Deleting %s%u", control->opName(), control->id()); JitSpew(JitSpew_GVN, " Discarding %s%u", control->opName(), control->id());
#endif #endif
// Keep track of how many blocks within dominatorRoot's tree have been deleted. // Keep track of how many blocks within dominatorRoot's tree have been discarded.
if (dominatorRoot->dominates(block)) if (dominatorRoot->dominates(block))
++numBlocksDeleted_; ++numBlocksDiscarded_;
// TODO: Removing a block deletes the phis, instructions, and resume // TODO: Removing a block discards the phis, instructions, and resume
// points in the block, and their operands may become dead. We should // points in the block, and their operands may become dead. We should
// detect this and delete them. In practice though, when this happens, // detect this and discard them. In practice though, when this happens,
// we often end up re-running GVN for other reasons anyway (bug 1031412). // we often end up re-running GVN for other reasons anyway (bug 1031412).
graph_.removeBlockIncludingPhis(block); graph_.removeBlockIncludingPhis(block);
blocksRemoved_ = true; blocksRemoved_ = true;
@ -443,7 +443,7 @@ ValueNumberer::hasLeader(const MPhi *phi, const MBasicBlock *phiBlock) const
// Test whether there are any phis in the backedge's loop header which are // Test whether there are any phis in the backedge's loop header which are
// newly optimizable, as a result of optimizations done inside the loop. This // newly optimizable, as a result of optimizations done inside the loop. This
// is not a sparse approach, but restarting is rare enough in practice. // is not a sparse approach, but restarting is rare enough in practice.
// Termination is ensured by deleting the phi triggering the iteration. // Termination is ensured by discarding the phi triggering the iteration.
bool bool
ValueNumberer::loopHasOptimizablePhi(MBasicBlock *backedge) const ValueNumberer::loopHasOptimizablePhi(MBasicBlock *backedge) const
{ {
@ -480,11 +480,11 @@ ValueNumberer::visitDefinition(MDefinition *def)
// The node's foldsTo said |def| can be replaced by |rep|. If |def| is a // 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 // 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. // needed, so we can clear |def|'s guard flag and let it be discarded.
def->setNotGuardUnchecked(); def->setNotGuardUnchecked();
if (DeadIfUnused(def)) { if (DeadIfUnused(def)) {
if (!deleteDefsRecursively(def)) if (!discardDefsRecursively(def))
return false; return false;
} }
def = sim; def = sim;
@ -503,17 +503,17 @@ ValueNumberer::visitDefinition(MDefinition *def)
// The node's congruentTo said |def| is congruent to |rep|, and it's // 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|, // 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. // so we can clear |def|'s guard flag and let it be discarded.
def->setNotGuardUnchecked(); def->setNotGuardUnchecked();
if (DeadIfUnused(def)) { if (DeadIfUnused(def)) {
// deleteDef should not add anything to the deadDefs, as the // discardDef should not add anything to the deadDefs, as the
// redundant operation should have the same input operands. // redundant operation should have the same input operands.
mozilla::DebugOnly<bool> r = deleteDef(def, DontSetUseRemoved); mozilla::DebugOnly<bool> r = discardDef(def, DontSetUseRemoved);
MOZ_ASSERT(r, "deleteDef shouldn't have tried to add anything to the worklist, " MOZ_ASSERT(r, "discardDef shouldn't have tried to add anything to the worklist, "
"so it shouldn't have failed"); "so it shouldn't have failed");
MOZ_ASSERT(deadDefs_.empty(), MOZ_ASSERT(deadDefs_.empty(),
"deleteDef shouldn't have added anything to the worklist"); "discardDef shouldn't have added anything to the worklist");
} }
def = rep; def = rep;
} }
@ -589,9 +589,9 @@ ValueNumberer::visitBlock(MBasicBlock *block, const MBasicBlock *dominatorRoot)
for (MDefinitionIterator iter(block); iter; ) { for (MDefinitionIterator iter(block); iter; ) {
MDefinition *def = *iter++; MDefinition *def = *iter++;
// If the definition is dead, delete it. // If the definition is dead, discard it.
if (IsDead(def)) { if (IsDead(def)) {
if (!deleteDefsRecursively(def)) if (!discardDefsRecursively(def))
return false; return false;
continue; continue;
} }
@ -612,7 +612,7 @@ ValueNumberer::visitDominatorTree(MBasicBlock *dominatorRoot, size_t *totalNumVi
dominatorRoot == graph_.entryBlock() ? " (normal entry block)" : dominatorRoot == graph_.entryBlock() ? " (normal entry block)" :
dominatorRoot == graph_.osrBlock() ? " (OSR entry block)" : dominatorRoot == graph_.osrBlock() ? " (OSR entry block)" :
" (normal entry and OSR entry merge point)"); " (normal entry and OSR entry merge point)");
MOZ_ASSERT(numBlocksDeleted_ == 0, "numBlocksDeleted_ wasn't reset"); MOZ_ASSERT(numBlocksDiscarded_ == 0, "numBlocksDiscarded_ wasn't reset");
MOZ_ASSERT(dominatorRoot->immediateDominator() == dominatorRoot, MOZ_ASSERT(dominatorRoot->immediateDominator() == dominatorRoot,
"root is not a dominator tree root"); "root is not a dominator tree root");
@ -637,15 +637,15 @@ ValueNumberer::visitDominatorTree(MBasicBlock *dominatorRoot, size_t *totalNumVi
remainingBlocks_.clear(); remainingBlocks_.clear();
} }
++numVisited; ++numVisited;
MOZ_ASSERT(numVisited <= dominatorRoot->numDominated() - numBlocksDeleted_, MOZ_ASSERT(numVisited <= dominatorRoot->numDominated() - numBlocksDiscarded_,
"Visited blocks too many times"); "Visited blocks too many times");
if (numVisited >= dominatorRoot->numDominated() - numBlocksDeleted_) if (numVisited >= dominatorRoot->numDominated() - numBlocksDiscarded_)
break; break;
} }
*totalNumVisited += numVisited; *totalNumVisited += numVisited;
values_.clear(); values_.clear();
numBlocksDeleted_ = 0; numBlocksDiscarded_ = 0;
return true; return true;
} }
@ -679,7 +679,7 @@ ValueNumberer::ValueNumberer(MIRGenerator *mir, MIRGraph &graph)
deadDefs_(graph.alloc()), deadDefs_(graph.alloc()),
unreachableBlocks_(graph.alloc()), unreachableBlocks_(graph.alloc()),
remainingBlocks_(graph.alloc()), remainingBlocks_(graph.alloc()),
numBlocksDeleted_(0), numBlocksDiscarded_(0),
rerun_(false), rerun_(false),
blocksRemoved_(false), blocksRemoved_(false),
updateAliasAnalysis_(false), updateAliasAnalysis_(false),
@ -704,8 +704,9 @@ ValueNumberer::run(UpdateAliasAnalysisFlag updateAliasAnalysis)
uint64_t(graph_.numBlocks())); uint64_t(graph_.numBlocks()));
// Top level non-sparse iteration loop. If an iteration performs a // Top level non-sparse iteration loop. If an iteration performs a
// significant change, such as deleting a block which changes the dominator // significant change, such as discarding a block which changes the
// tree and may enable more optimization, this loop takes another iteration. // dominator tree and may enable more optimization, this loop takes another
// iteration.
int runs = 0; int runs = 0;
for (;;) { for (;;) {
if (!visitGraph()) if (!visitGraph())
@ -746,7 +747,7 @@ ValueNumberer::run(UpdateAliasAnalysisFlag updateAliasAnalysis)
// Enforce an arbitrary iteration limit. This is rarely reached, and // Enforce an arbitrary iteration limit. This is rarely reached, and
// isn't even strictly necessary, as the algorithm is guaranteed to // isn't even strictly necessary, as the algorithm is guaranteed to
// terminate on its own in a finite amount of time (since every time we // terminate on its own in a finite amount of time (since every time we
// re-run we delete the construct which triggered the re-run), but it // re-run we discard the construct which triggered the re-run), but it
// does help avoid slow compile times on pathlogical code. // does help avoid slow compile times on pathlogical code.
++runs; ++runs;
if (runs == 6) { if (runs == 6) {

View File

@ -66,7 +66,7 @@ class ValueNumberer
DefWorklist deadDefs_; // Worklist for deleting values DefWorklist deadDefs_; // Worklist for deleting values
BlockWorklist unreachableBlocks_; // Worklist for unreachable blocks BlockWorklist unreachableBlocks_; // Worklist for unreachable blocks
BlockWorklist remainingBlocks_; // Blocks remaining with fewer preds BlockWorklist remainingBlocks_; // Blocks remaining with fewer preds
size_t numBlocksDeleted_; // Num deleted blocks in current tree size_t numBlocksDiscarded_; // Num discarded blocks in current tree
bool rerun_; // Should we run another GVN iteration? bool rerun_; // Should we run another GVN iteration?
bool blocksRemoved_; // Have any blocks been removed? bool blocksRemoved_; // Have any blocks been removed?
bool updateAliasAnalysis_; // Do we care about AliasAnalysis? bool updateAliasAnalysis_; // Do we care about AliasAnalysis?
@ -77,13 +77,13 @@ class ValueNumberer
SetUseRemoved SetUseRemoved
}; };
bool deleteDefsRecursively(MDefinition *def); bool discardDefsRecursively(MDefinition *def);
bool releasePhiOperands(MPhi *phi, const MBasicBlock *phiBlock, bool releasePhiOperands(MPhi *phi, const MBasicBlock *phiBlock,
UseRemovedOption useRemovedOption = SetUseRemoved); UseRemovedOption useRemovedOption = SetUseRemoved);
bool releaseInsOperands(MInstruction *ins, bool releaseInsOperands(MInstruction *ins,
UseRemovedOption useRemovedOption = SetUseRemoved); UseRemovedOption useRemovedOption = SetUseRemoved);
bool deleteDef(MDefinition *def, bool discardDef(MDefinition *def,
UseRemovedOption useRemovedOption = SetUseRemoved); UseRemovedOption useRemovedOption = SetUseRemoved);
bool processDeadDefs(); bool processDeadDefs();
bool removePredecessor(MBasicBlock *block, MBasicBlock *pred); bool removePredecessor(MBasicBlock *block, MBasicBlock *pred);