[Dominators] Make DTU be able to delete a BB before recalculation under Eager UpdateStrategy

[NutshellySima]

Summary

Previously, when user tries to delete a BasicBlock without applying updates under Eager UpdateStrategy (typically before recalculation),

DomTreeUpdater DTU(DT, ...::Eager);
... // Decide to remove BB
DTU.deleteBB(BB); // Call DT.eraseNode(BB) and find BB is not a leaf node.
*Asserts*
DTU.recalculate(F);

When we need this code pattern to be compatible with both Eager and Lazy Strategy instances of DTU, we cannot just replace DTU.deleteBB(BB) into BB->eraseFromParent() because DTU.recalculate() under Lazy Strategy checks whether there is any pending updates/DelBB to proceed.
Under Lazy Strategy, things won't be affected because deleteBB always goes after applying updates or automatically disables DT node removal during recalculation.
Thus, a new parameter is added to bypass this removal under only Eager UpdateStrategy in this special scenario (trying to delete a BasicBlock without applying any update under Eager UpdateStrategy).

DTU.deleteBB(BB, /*DisableNodeErasing*/ true);
DTU.recalculate(F);

[NutshellySima]

It's possible to do updates related to DelBB first then call deleteBB(DelBB) then do the recalculation, which will make the DTU interface less confusing to users.

[brzycki]

include/llvm/IR/DomTreeUpdater.h

150

It would be a good idea to add the reason why you need this flag, not just what it does. The comments at the top of this ticket would be a good start to add here.

The reason it's necessary is a subtle one and may not be obvious to users of your API.

include/llvm/IR/DomTreeUpdater.h

public:
   /// The Eager Strategy applies the update immediately while the Lazy Strategy
   /// queues the update. It is recommended to only use this method when you have
   /// 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. 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);
+  /// Under Eager UpdateStrategy, DelBB will be processed immediately; If
+  /// DisableNodeErasing is true, DelBB will not be erased from available trees.
+  /// It is used before a recalculation event. Under Lazy UpdateStrategy, DelBB
+  /// will be queued until a flush event and 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, bool DisableNodeErasing = false);
 
   /// 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. Assert if any instruction of DelBB is
-  /// modified while awaiting deletion. Multiple callbacks can be queued for one
-  /// DelBB under Lazy UpdateStrategy.
+  /// Under Eager UpdateStrategy, DelBB will be processed immediately; If
+  /// DisableNodeErasing is true, DelBB will not be erased from available trees.
+  /// It is used before a recalculation event. Under Lazy UpdateStrategy, DelBB
+  /// will be queued until a flush event and 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);
+                        std::function<void(BasicBlock *)> Callback,
+                        bool DisableNodeErasing = false);
 
   /// 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);
 
   /// Flush DomTree updates and return DomTree.

lib/IR/DomTreeUpdater.cpp

bool DomTreeUpdater::isBBPendingDeletion(llvm::BasicBlock *DelBB) const {
   return DeletedBBs.count(DelBB) != 0;
 }
 
 // The DT and PDT require the nodes related to updates
 // are not deleted when update functions are called.
 // So BasicBlock deletions must be pended when the
 // UpdateStrategy is Lazy. When the UpdateStrategy is
 // Eager, the BasicBlock will be deleted immediately.
-void DomTreeUpdater::deleteBB(BasicBlock *DelBB) {
+void DomTreeUpdater::deleteBB(BasicBlock *DelBB, bool DisableNodeErasing) {
   validateDeleteBB(DelBB);
   if (Strategy == UpdateStrategy::Lazy) {
     DeletedBBs.insert(DelBB);
     return;
   }
 
   DelBB->removeFromParent();
+  if (!DisableNodeErasing)
     eraseDelBBNode(DelBB);
   delete DelBB;
 }
 
 void DomTreeUpdater::callbackDeleteBB(
-    BasicBlock *DelBB, std::function<void(BasicBlock *)> Callback) {
+    BasicBlock *DelBB, std::function<void(BasicBlock *)> Callback,
+    bool DisableNodeErasing) {
   validateDeleteBB(DelBB);
   if (Strategy == UpdateStrategy::Lazy) {
     Callbacks.push_back(CallBackOnDeletion(DelBB, Callback));
     DeletedBBs.insert(DelBB);
     return;
   }
 
   DelBB->removeFromParent();
+  if (!DisableNodeErasing)
     eraseDelBBNode(DelBB);
   Callback(DelBB);
   delete DelBB;
 }
 
 void DomTreeUpdater::eraseDelBBNode(BasicBlock *DelBB) {
   if (DT && !IsRecalculatingDomTree)
     if (DT->getNode(DelBB))
       DT->eraseNode(DelBB);