From: Daniel Berlin Date: Sat, 18 Mar 2017 15:41:40 +0000 (+0000) Subject: NewGVN: Greatly enhance the ability of the NewGVN verifier to detect X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=14281a41e1496bbc9cb3a832a132d3cee7272b4b;p=llvm NewGVN: Greatly enhance the ability of the NewGVN verifier to detect issues, subsuming previous verifier. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@298188 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Transforms/Scalar/GVNExpression.h b/include/llvm/Transforms/Scalar/GVNExpression.h index c967bb3adc1..ad5bb40a035 100644 --- a/include/llvm/Transforms/Scalar/GVNExpression.h +++ b/include/llvm/Transforms/Scalar/GVNExpression.h @@ -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; diff --git a/lib/Transforms/Scalar/NewGVN.cpp b/lib/Transforms/Scalar/NewGVN.cpp index 0fa9b1d714a..cefbbc92f82 100644 --- a/lib/Transforms/Scalar/NewGVN.cpp +++ b/lib/Transforms/Scalar/NewGVN.cpp @@ -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 AMembers(A->Members.begin(), A->Members.end()); + std::set BMembers(B->Members.begin(), B->Members.end()); + return AMembers == BMembers; +} + namespace llvm { template <> struct DenseMapInfo { 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 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 StartingVNCounter; }; } // end anonymous namespace @@ -432,6 +462,16 @@ static std::string getBlockName(const BasicBlock *B) { } #endif +// Get the basic block from an instruction/memory value. +BasicBlock *NewGVN::getBlockForValue(Value *V) const { + if (auto *I = dyn_cast(V)) + return I->getParent(); + else if (auto *MP = dyn_cast(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(I); @@ -1901,29 +1941,121 @@ void NewGVN::verifyMemoryCongruency() const { } } -// 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 BeforeIteration; + + for (auto &KV : ValueToClass) { + if (auto *I = dyn_cast(KV.first)) + // Skip unused/dead instructions. + if (InstrDFS.lookup(I) == 0) continue; - if (isa(&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> + EqualClasses; + for (const auto &KV : ValueToClass) { + if (auto *I = dyn_cast(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(V)) { + DEBUG(dbgs() << "Processing MemoryPhi " << *MP << "\n"); + valueNumberMemoryPhi(MP); + } else if (auto *I = dyn_cast(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(V)) - CurrBlock = I->getParent(); - else if (auto *MP = dyn_cast(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(V)) { - DEBUG(dbgs() << "Processing MemoryPhi " << *MP << "\n"); - valueNumberMemoryPhi(MP); - } else if (auto *I = dyn_cast(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(V) || isa(V); } -// Get the basic block from an instruction/value. -static BasicBlock *getBlockForValue(Value *V) { - if (auto *I = dyn_cast(V)) - return I->getParent(); - return nullptr; -} - struct NewGVN::ValueDFS { int DFSIn = 0; int DFSOut = 0;