[llvm] r298188 - NewGVN: Greatly enhance the ability of the NewGVN verifier to detect
Daniel Berlin via llvm-commits
llvm-commits at lists.llvm.org
Sat Mar 18 08:41:40 PDT 2017
Author: dannyb
Date: Sat Mar 18 10:41:40 2017
New Revision: 298188
URL: http://llvm.org/viewvc/llvm-project?rev=298188&view=rev
Log:
NewGVN: Greatly enhance the ability of the NewGVN verifier to detect
issues, subsuming previous verifier.
Modified:
llvm/trunk/include/llvm/Transforms/Scalar/GVNExpression.h
llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
Modified: llvm/trunk/include/llvm/Transforms/Scalar/GVNExpression.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Scalar/GVNExpression.h?rev=298188&r1=298187&r2=298188&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Scalar/GVNExpression.h (original)
+++ llvm/trunk/include/llvm/Transforms/Scalar/GVNExpression.h Sat Mar 18 10:41:40 2017
@@ -65,7 +65,7 @@ public:
static unsigned getEmptyKey() { return ~0U; }
static unsigned getTombstoneKey() { return ~1U; }
-
+ bool operator!=(const Expression &Other) const { return !(*this == Other); }
bool operator==(const Expression &Other) const {
if (getOpcode() != Other.getOpcode())
return false;
Modified: llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp?rev=298188&r1=298187&r2=298188&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp Sat Mar 18 10:41:40 2017
@@ -176,6 +176,30 @@ struct CongruenceClass {
: ID(ID), RepLeader(Leader), DefiningExpr(E) {}
};
+// Return true if two congruence classes are equivalent to each other. This
+// means
+// that every field but the ID number and the dead field are equivalent.
+bool areClassesEquivalent(const CongruenceClass *A, const CongruenceClass *B) {
+ if (A == B)
+ return true;
+ if ((A && !B) || (B && !A))
+ return false;
+
+ if (std::tie(A->StoreCount, A->RepLeader, A->RepStoredValue,
+ A->RepMemoryAccess) != std::tie(B->StoreCount, B->RepLeader,
+ B->RepStoredValue,
+ B->RepMemoryAccess))
+ return false;
+ if (A->DefiningExpr != B->DefiningExpr)
+ if (!A->DefiningExpr || !B->DefiningExpr ||
+ *A->DefiningExpr != *B->DefiningExpr)
+ return false;
+ // We need some ordered set
+ std::set<Value *> AMembers(A->Members.begin(), A->Members.end());
+ std::set<Value *> BMembers(B->Members.begin(), B->Members.end());
+ return AMembers == BMembers;
+}
+
namespace llvm {
template <> struct DenseMapInfo<const Expression *> {
static const Expression *getEmptyKey() {
@@ -353,7 +377,6 @@ private:
bool setMemoryAccessEquivTo(MemoryAccess *From, CongruenceClass *To);
MemoryAccess *lookupMemoryAccessEquiv(MemoryAccess *) const;
bool isMemoryAccessTop(const MemoryAccess *) const;
-
// Ranking
unsigned int getRank(const Value *) const;
bool shouldSwapOperands(const Value *, const Value *) const;
@@ -387,13 +410,20 @@ private:
void markLeaderChangeTouched(CongruenceClass *CC);
void addPredicateUsers(const PredicateBase *, Instruction *);
+ // Main loop of value numbering
+ void iterateTouchedInstructions();
+
// Utilities.
void cleanupTables();
std::pair<unsigned, unsigned> assignDFSNumbers(BasicBlock *, unsigned);
void updateProcessedCount(Value *V);
void verifyMemoryCongruency() const;
- void verifyComparisons(Function &F);
+ void verifyIterationSettled(Function &F);
bool singleReachablePHIPath(const MemoryAccess *, const MemoryAccess *) const;
+ BasicBlock *getBlockForValue(Value *V) const;
+ // Debug counter info. When verifying, we have to reset the value numbering
+ // debug counter to the same state it started in to get the same results.
+ std::pair<int, int> StartingVNCounter;
};
} // end anonymous namespace
@@ -432,6 +462,16 @@ static std::string getBlockName(const Ba
}
#endif
+// Get the basic block from an instruction/memory value.
+BasicBlock *NewGVN::getBlockForValue(Value *V) const {
+ if (auto *I = dyn_cast<Instruction>(V))
+ return I->getParent();
+ else if (auto *MP = dyn_cast<MemoryPhi>(V))
+ return MP->getBlock();
+ llvm_unreachable("Should have been able to figure out a block for our value");
+ return nullptr;
+}
+
PHIExpression *NewGVN::createPHIExpression(Instruction *I) {
BasicBlock *PHIBlock = I->getParent();
auto *PN = cast<PHINode>(I);
@@ -1901,29 +1941,121 @@ void NewGVN::verifyMemoryCongruency() co
}
}
-// Re-evaluate all the comparisons after value numbering and ensure they don't
-// change. If they changed, we didn't mark them touched properly.
-void NewGVN::verifyComparisons(Function &F) {
+// Verify that the sparse propagation we did actually found the maximal fixpoint
+// We do this by storing the value to class mapping, touching all instructions,
+// and redoing the iteration to see if anything changed.
+void NewGVN::verifyIterationSettled(Function &F) {
#ifndef NDEBUG
- for (auto &BB : F) {
- if (!ReachableBlocks.count(&BB))
- continue;
- for (auto &I : BB) {
- if (InstrDFS.lookup(&I) == 0)
+ if (DebugCounter::isCounterSet(VNCounter))
+ DebugCounter::setCounterValue(VNCounter, StartingVNCounter);
+
+ // Note that we have to store the actual classes, as we may change existing
+ // classes during iteration. This is because our memory iteration propagation
+ // is not perfect, and so may waste a little work. But it should generate
+ // exactly the same congruence classes we have now, with different IDs.
+ std::map<const Value *, CongruenceClass> BeforeIteration;
+
+ for (auto &KV : ValueToClass) {
+ if (auto *I = dyn_cast<Instruction>(KV.first))
+ // Skip unused/dead instructions.
+ if (InstrDFS.lookup(I) == 0)
continue;
- if (isa<CmpInst>(&I)) {
- auto *CurrentVal = ValueToClass.lookup(&I);
- valueNumberInstruction(&I);
- assert(CurrentVal == ValueToClass.lookup(&I) &&
- "Re-evaluating comparison changed value");
- }
+ BeforeIteration.insert({KV.first, *KV.second});
+ }
+
+ TouchedInstructions.set();
+ TouchedInstructions.reset(0);
+ iterateTouchedInstructions();
+ DenseSet<std::pair<const CongruenceClass *, const CongruenceClass *>>
+ EqualClasses;
+ for (const auto &KV : ValueToClass) {
+ if (auto *I = dyn_cast<Instruction>(KV.first))
+ // Skip unused/dead instructions.
+ if (InstrDFS.lookup(I) == 0)
+ continue;
+ // We could sink these uses, but i think this adds a bit of clarity here as
+ // to what we are comparing.
+ auto *BeforeCC = &BeforeIteration.find(KV.first)->second;
+ auto *AfterCC = KV.second;
+ // Note that the classes can't change at this point, so we memoize the set
+ // that are equal.
+ if (!EqualClasses.count({BeforeCC, AfterCC})) {
+ assert(areClassesEquivalent(BeforeCC, AfterCC) &&
+ "Value number changed after main loop completed!");
+ EqualClasses.insert({BeforeCC, AfterCC});
}
}
#endif
}
+// This is the main value numbering loop, it iterates over the initial touched
+// instruction set, propagating value numbers, marking things touched, etc,
+// until the set of touched instructions is completely empty.
+void NewGVN::iterateTouchedInstructions() {
+ unsigned int Iterations = 0;
+ // Figure out where touchedinstructions starts
+ int FirstInstr = TouchedInstructions.find_first();
+ // Nothing set, nothing to iterate, just return.
+ if (FirstInstr == -1)
+ return;
+ BasicBlock *LastBlock = getBlockForValue(DFSToInstr[FirstInstr]);
+ while (TouchedInstructions.any()) {
+ ++Iterations;
+ // Walk through all the instructions in all the blocks in RPO.
+ // TODO: As we hit a new block, we should push and pop equalities into a
+ // table lookupOperandLeader can use, to catch things PredicateInfo
+ // might miss, like edge-only equivalences.
+ for (int InstrNum = TouchedInstructions.find_first(); InstrNum != -1;
+ InstrNum = TouchedInstructions.find_next(InstrNum)) {
+
+ // This instruction was found to be dead. We don't bother looking
+ // at it again.
+ if (InstrNum == 0) {
+ TouchedInstructions.reset(InstrNum);
+ continue;
+ }
+
+ Value *V = DFSToInstr[InstrNum];
+ BasicBlock *CurrBlock = getBlockForValue(V);
+
+ // If we hit a new block, do reachability processing.
+ if (CurrBlock != LastBlock) {
+ LastBlock = CurrBlock;
+ bool BlockReachable = ReachableBlocks.count(CurrBlock);
+ const auto &CurrInstRange = BlockInstRange.lookup(CurrBlock);
+
+ // If it's not reachable, erase any touched instructions and move on.
+ if (!BlockReachable) {
+ TouchedInstructions.reset(CurrInstRange.first, CurrInstRange.second);
+ DEBUG(dbgs() << "Skipping instructions in block "
+ << getBlockName(CurrBlock)
+ << " because it is unreachable\n");
+ continue;
+ }
+ updateProcessedCount(CurrBlock);
+ }
+
+ if (auto *MP = dyn_cast<MemoryPhi>(V)) {
+ DEBUG(dbgs() << "Processing MemoryPhi " << *MP << "\n");
+ valueNumberMemoryPhi(MP);
+ } else if (auto *I = dyn_cast<Instruction>(V)) {
+ valueNumberInstruction(I);
+ } else {
+ llvm_unreachable("Should have been a MemoryPhi or Instruction");
+ }
+ updateProcessedCount(V);
+ // Reset after processing (because we may mark ourselves as touched when
+ // we propagate equalities).
+ TouchedInstructions.reset(InstrNum);
+ }
+ }
+ NumGVNMaxIterations = std::max(NumGVNMaxIterations.getValue(), Iterations);
+}
+
// This is the main transformation entry point.
bool NewGVN::runGVN() {
+ if (DebugCounter::isCounterSet(VNCounter))
+ StartingVNCounter = DebugCounter::getCounterValue(VNCounter);
bool Changed = false;
NumFuncArgs = F.arg_size();
MSSAWalker = MSSA->getWalker();
@@ -1992,71 +2124,10 @@ bool NewGVN::runGVN() {
ReachableBlocks.insert(&F.getEntryBlock());
initializeCongruenceClasses(F);
-
- unsigned int Iterations = 0;
- // We start out in the entry block.
- BasicBlock *LastBlock = &F.getEntryBlock();
- while (TouchedInstructions.any()) {
- ++Iterations;
- // Walk through all the instructions in all the blocks in RPO.
- // TODO: As we hit a new block, we should push and pop equalities into a
- // table lookupOperandLeader can use, to catch things PredicateInfo
- // might miss, like edge-only equivalences.
- for (int InstrNum = TouchedInstructions.find_first(); InstrNum != -1;
- InstrNum = TouchedInstructions.find_next(InstrNum)) {
-
- // This instruction was found to be dead. We don't bother looking
- // at it again.
- if (InstrNum == 0) {
- TouchedInstructions.reset(InstrNum);
- continue;
- }
-
- Value *V = DFSToInstr[InstrNum];
- BasicBlock *CurrBlock = nullptr;
-
- if (auto *I = dyn_cast<Instruction>(V))
- CurrBlock = I->getParent();
- else if (auto *MP = dyn_cast<MemoryPhi>(V))
- CurrBlock = MP->getBlock();
- else
- llvm_unreachable("DFSToInstr gave us an unknown type of instruction");
-
- // If we hit a new block, do reachability processing.
- if (CurrBlock != LastBlock) {
- LastBlock = CurrBlock;
- bool BlockReachable = ReachableBlocks.count(CurrBlock);
- const auto &CurrInstRange = BlockInstRange.lookup(CurrBlock);
-
- // If it's not reachable, erase any touched instructions and move on.
- if (!BlockReachable) {
- TouchedInstructions.reset(CurrInstRange.first, CurrInstRange.second);
- DEBUG(dbgs() << "Skipping instructions in block "
- << getBlockName(CurrBlock)
- << " because it is unreachable\n");
- continue;
- }
- updateProcessedCount(CurrBlock);
- }
-
- if (auto *MP = dyn_cast<MemoryPhi>(V)) {
- DEBUG(dbgs() << "Processing MemoryPhi " << *MP << "\n");
- valueNumberMemoryPhi(MP);
- } else if (auto *I = dyn_cast<Instruction>(V)) {
- valueNumberInstruction(I);
- } else {
- llvm_unreachable("Should have been a MemoryPhi or Instruction");
- }
- updateProcessedCount(V);
- // Reset after processing (because we may mark ourselves as touched when
- // we propagate equalities).
- TouchedInstructions.reset(InstrNum);
- }
- }
- NumGVNMaxIterations = std::max(NumGVNMaxIterations.getValue(), Iterations);
+ iterateTouchedInstructions();
#ifndef NDEBUG
verifyMemoryCongruency();
- verifyComparisons(F);
+ verifyIterationSettled(F);
#endif
Changed |= eliminateInstructions(F);
@@ -2093,13 +2164,6 @@ static bool alwaysAvailable(Value *V) {
return isa<Constant>(V) || isa<Argument>(V);
}
-// Get the basic block from an instruction/value.
-static BasicBlock *getBlockForValue(Value *V) {
- if (auto *I = dyn_cast<Instruction>(V))
- return I->getParent();
- return nullptr;
-}
-
struct NewGVN::ValueDFS {
int DFSIn = 0;
int DFSOut = 0;
More information about the llvm-commits
mailing list