[PredicateInfo] Invalidate an OrderedBasicBlock when instruction in it is deleted.

[trentxintong]

Summary

PredicateInfo uses OrderedBasicBlock and NewGVN which uses PredicateInfo should keep
it updated and correct.

[• dberlin]

Without a reason to do this specifically, it seems a waste of time

None of this data is used after initial building, and all of these are destroyed after newgvn ends, so updating them doesn't seem to accomplish anything?

[trentxintong]

In D34823#795758, @dberlin wrote:

Without a reason to do this specifically, it seems a waste of time

None of this data is used after initial building, and all of these are destroyed after newgvn ends, so updating them doesn't seem to accomplish anything?

I was making the Instruction * in OrdereBasicBlock AssertingVH to make sure things are updated properly when instructions are deleted. If they are not, we are essentially holding a dangling pointer which could in worst case result in invalid dominance relationship being returned. Then i found NewGVN is breaking the rule, i.e. erasing an instruction without updating OrderedBasicBlock in OrderedInstruction.

I agree the way NewGVN is written now is ok even without updating the OrderedBasicBlock. Maybe we should not make Instruction * in OBB AssertingVH. Instead should give user the freedom to invalidate if necessary.

[trentxintong]

This is no longer necessary as discussed on the list.

include/llvm/Transforms/Utils/PredicateInfo.h

public:
 
   void dump() const;
   void print(raw_ostream &) const;
 
   const PredicateBase *getPredicateInfoFor(const Value *V) const {
     return PredicateMap.lookup(V);
   }
 
+  // Inform the PredicateInfo analysis when an instruction is deleted
+  // in the function.
+  void updateErasedInst(const Instruction *Inst) {
+    // Need to invalidate the ordered basicblock for the instruction.
+    OI.invalidateBlock(Inst->getParent());
+  }
+
 protected:
   // Used by PredicateInfo annotater, dumpers, and wrapper pass.
   friend class PredicateInfoAnnotatedWriter;
   friend class PredicateInfoPrinterLegacyPass;
 
 private:
   void buildPredicateInfo();
   void processAssume(IntrinsicInst *, BasicBlock *, SmallPtrSetImpl<Value *> &);

lib/Transforms/Scalar/NewGVN.cpp

bool NewGVN::runGVN() {
 
   Changed |= eliminateInstructions(F);
 
   // Delete all instructions marked for deletion.
   for (Instruction *ToErase : InstructionsToErase) {
     if (!ToErase->use_empty())
       ToErase->replaceAllUsesWith(UndefValue::get(ToErase->getType()));
 
-    if (ToErase->getParent())
+    if (ToErase->getParent()) {
+      PredInfo->updateErasedInst(ToErase);
       ToErase->eraseFromParent();
     }
+  }
 
   // Delete all unreachable blocks.
   auto UnreachableBlockPred = [&](const BasicBlock &BB) {
     return !ReachableBlocks.count(&BB);
   };
 
   for (auto &BB : make_filter_range(F, UnreachableBlockPred)) {
     DEBUG(dbgs() << "We believe block " << getBlockName(&BB)