[Dominators] Assert if there is modification to DelBB while it is awaiting deletion

[NutshellySima]

Summary

Previously, passes use

DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
DTU.deleteBB(DelBB);

to delete a BasicBlock.
But passes which don't have the ability to update DomTree (e.g. tailcallelim, simplifyCFG) cannot recognize a DelBB awaiting deletion and will continue to process this DelBB.
This is a simple approach to notify devs of passes which may use DTU in the future to deal with deleted BasicBlocks under Lazy Strategy correctly.

[kuhar]

lib/IR/DomTreeUpdater.cpp

141 ↗

(On Diff #157032)

This deserves a comment -- it's not clear why such a block would have exactly one instruction that is UnreachableInst

[NutshellySima]

Address comments.

[kuhar]

Thanks for the comment!
LGTM.

[brzycki]

Delayed LGTM. It's a good assert to know if someone tries to alter DelBB after it's marked for deletion.

llvm/trunk/include/llvm/IR/DomTreeUpdater.h

public:
   /// exactly one deletion (and no insertions) and want to discard an invalid
   /// update.
   void deleteEdgeRelaxed(BasicBlock *From, BasicBlock *To);
 
   /// Delete DelBB. DelBB will be removed from its Parent and
   /// erased from available trees if it exists and finally get deleted.
   /// Under Eager UpdateStrategy, DelBB will be processed immediately.
   /// Under Lazy UpdateStrategy, DelBB will be queued until a flush event and
-  /// all available trees are up-to-date. When both DT and PDT are nullptrs,
-  /// DelBB will be queued until flush() is called.
+  /// all available trees are up-to-date. Assert if any instruction of DelBB is
+  /// modified while awaiting deletion. When both DT and PDT are nullptrs, DelBB
+  /// will be queued until flush() is called.
   void deleteBB(BasicBlock *DelBB);
 
   /// Delete DelBB. DelBB will be removed from its Parent and
   /// erased from available trees if it exists. Then the callback will
   /// be called. Finally, DelBB will be deleted.
   /// Under Eager UpdateStrategy, DelBB will be processed immediately.
   /// Under Lazy UpdateStrategy, DelBB will be queued until a flush event and
-  /// all available trees are up-to-date.
-  /// Multiple callbacks can be queued for one DelBB under Lazy UpdateStrategy.
+  /// all available trees are up-to-date. Assert if any instruction of DelBB is
+  /// modified while awaiting deletion. Multiple callbacks can be queued for one
+  /// DelBB under Lazy UpdateStrategy.
   void callbackDeleteBB(BasicBlock *DelBB,
                         std::function<void(BasicBlock *)> Callback);
 
   /// Recalculate all available trees.
   /// Under Lazy Strategy, available trees will only be recalculated if there
   /// are pending updates or there is BasicBlock awaiting deletion. Returns true
   /// if at least one tree is recalculated.
   bool recalculate(Function &F);

llvm/trunk/lib/IR/DomTreeUpdater.cpp

if (!hasPendingUpdates())
     forceFlushDeletedBB();
 }
 
 bool DomTreeUpdater::forceFlushDeletedBB() {
   if (DeletedBBs.empty())
     return false;
 
   for (auto *BB : DeletedBBs) {
+    // After calling deleteBB or callbackDeleteBB under Lazy UpdateStrategy,
+    // validateDeleteBB() removes all instructions of DelBB and adds an
+    // UnreachableInst as its terminator. So we check whether the BasicBlock to
+    // delete only has an UnreachableInst inside.
+    assert(BB->getInstList().size() == 1 &&
+           isa<UnreachableInst>(BB->getTerminator()) &&
+           "DelBB has been modified while awaiting deletion.");
     BB->removeFromParent();
     eraseDelBBNode(BB);
     delete BB;
   }
   DeletedBBs.clear();
   Callbacks.clear();
   return true;
 }