[llvm] 282324a - [GVN] Fix verifyRemoved() verification

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-06-12T15:14:27+02:00
New Revision: 282324aa4a6c29d5ce31c66f8def15d9bd8e84e4

URL: https://github.com/llvm/llvm-project/commit/282324aa4a6c29d5ce31c66f8def15d9bd8e84e4
DIFF: https://github.com/llvm/llvm-project/commit/282324aa4a6c29d5ce31c66f8def15d9bd8e84e4.diff

LOG: [GVN] Fix verifyRemoved() verification

Fix the verification failure reported in
https://reviews.llvm.org/D141712#4413647. We need to remove the
load from the VN table as well, not just the leader table.

Also make sure that this verification always runs when assertions
are enabled, rather than only when -debug is passed.

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/GVN.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 318788db40c5e..69e64bdb0de0e 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -721,10 +721,8 @@ void GVNPass::ValueTable::erase(Value *V) {
 /// verifyRemoved - Verify that the value is removed from all internal data
 /// structures.
 void GVNPass::ValueTable::verifyRemoved(const Value *V) const {
-  for (DenseMap<Value*, uint32_t>::const_iterator
-         I = valueNumbering.begin(), E = valueNumbering.end(); I != E; ++I) {
-    assert(I->first != V && "Inst still occurs in value numbering map!");
-  }
+  assert(!valueNumbering.contains(V) &&
+         "Inst still occurs in value numbering map!");
 }
 
 //===----------------------------------------------------------------------===//
@@ -1476,6 +1474,7 @@ void GVNPass::eliminatePartiallyRedundantLoad(
         replaceValuesPerBlockEntry(ValuesPerBlock, OldLoad, NewLoad);
         if (uint32_t ValNo = VN.lookup(OldLoad, false))
           removeFromLeaderTable(ValNo, OldLoad, OldLoad->getParent());
+        VN.erase(OldLoad);
         removeInstruction(OldLoad);
       }
     }
@@ -2984,7 +2983,9 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) {
     PREInstr = CurInst->clone();
     if (!performScalarPREInsertion(PREInstr, PREPred, CurrentBlock, ValNo)) {
       // If we failed insertion, make sure we remove the instruction.
-      LLVM_DEBUG(verifyRemoved(PREInstr));
+#ifndef NDEBUG
+      verifyRemoved(PREInstr);
+#endif
       PREInstr->deleteValue();
       return false;
     }
@@ -3123,7 +3124,9 @@ void GVNPass::removeInstruction(Instruction *I) {
   if (MD) MD->removeInstruction(I);
   if (MSSAU)
     MSSAU->removeMemoryAccess(I);
-  LLVM_DEBUG(verifyRemoved(I));
+#ifndef NDEBUG
+  verifyRemoved(I);
+#endif
   ICF->removeInstruction(I);
   I->eraseFromParent();
 }