Revert "[nfc][mlgo] Incrementally update DominatorTreeAnalysis in Fun…

…ctionPropertiesAnalysis (llvm#104867)"

This seems to cause asserts in our builds:

  llvm/include/llvm/Support/GenericDomTreeConstruction.h:927:
  static void llvm::DomTreeBuilder::SemiNCAInfo<llvm::DominatorTreeBase<BasicBlock, false>>::DeleteEdge(DomTreeT &, const BatchUpdatePtr, const NodePtr, const NodePtr) [DomTreeT = llvm::DominatorTreeBase<BasicBlock, false>]:
  Assertion `!IsSuccessor(To, From) && "Deleted edge still exists in the CFG!"' failed.

and

  llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:390:
  DominatorTree &llvm::FunctionPropertiesUpdater::getUpdatedDominatorTree(FunctionAnalysisManager &) const:
  Assertion `DT.getNode(BB)' failed.

See comment on the PR.

> We need the dominator tree analysis for loop info analysis, which we need to get features like most nested loop and number of top level loops. Invalidating and recomputing these from scratch after each successful inlining can sometimes lead to lengthy compile times. We don't need to recompute from scratch, though, since we have some boundary information about where the changes to the CFG happen; moreover, for dom tree, the API supports incrementally updating the analysis result.
>
> This change addresses the dom tree part. The loop info is still recomputed from scratch. This does reduce the compile time quite significantly already, though (~5x in a specific case)
>
> The loop info change might be more involved and would follow in a subsequent PR.

This reverts commit a2a5508 and the
follow-up commit cdd11d6.

llvm/include/llvm/Analysis/FunctionPropertiesAnalysis.h

@@ -15,7 +15,6 @@
 #define LLVM_ANALYSIS_FUNCTIONPROPERTIESANALYSIS_H
 
 #include "llvm/ADT/DenseSet.h"
-#include "llvm/IR/Dominators.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
@@ -187,12 +186,7 @@ class FunctionPropertiesUpdater {
   static bool isUpdateValid(Function &F, const FunctionPropertiesInfo &FPI,
                             FunctionAnalysisManager &FAM);
 
-  DominatorTree &getUpdatedDominatorTree(FunctionAnalysisManager &FAM) const;
-
   DenseSet<const BasicBlock *> Successors;
-
-  // Edges we might potentially need to remove from the dominator tree.
-  SmallVector<DominatorTree::UpdateType, 2> DomTreeUpdates;
 };
 } // namespace llvm
 #endif // LLVM_ANALYSIS_FUNCTIONPROPERTIESANALYSIS_H

llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp

@@ -326,14 +326,6 @@ FunctionPropertiesUpdater::FunctionPropertiesUpdater(
   // with the CB BB ('Entry') between which the inlined callee will be pasted.
   Successors.insert(succ_begin(&CallSiteBB), succ_end(&CallSiteBB));
 
-  // the outcome of the inlining may be that some edges get lost (DCEd BBs
-  // because inlining brought some constant, for example). We don't know which
-  // edges will be removed, so we list all of them as potentially removable.
-  for (auto *Succ : successors(&CallSiteBB))
-    DomTreeUpdates.emplace_back(DominatorTree::UpdateKind::Delete,
-                                const_cast<BasicBlock *>(&CallSiteBB),
-                                const_cast<BasicBlock *>(Succ));
-
   // Inlining only handles invoke and calls. If this is an invoke, and inlining
   // it pulls another invoke, the original landing pad may get split, so as to
   // share its content with other potential users. So the edge up to which we
@@ -344,11 +336,6 @@ FunctionPropertiesUpdater::FunctionPropertiesUpdater(
   if (const auto *II = dyn_cast<InvokeInst>(&CB)) {
     const auto *UnwindDest = II->getUnwindDest();
     Successors.insert(succ_begin(UnwindDest), succ_end(UnwindDest));
-    // Same idea as above, we pretend we lose all these edges.
-    for (auto *Succ : successors(UnwindDest))
-      DomTreeUpdates.emplace_back(DominatorTree::UpdateKind::Delete,
-                                  const_cast<BasicBlock *>(UnwindDest),
-                                  const_cast<BasicBlock *>(Succ));
   }
 
   // Exclude the CallSiteBB, if it happens to be its own successor (1-BB loop).
@@ -369,30 +356,6 @@ FunctionPropertiesUpdater::FunctionPropertiesUpdater(
     FPI.updateForBB(*BB, -1);
 }
 
-DominatorTree &FunctionPropertiesUpdater::getUpdatedDominatorTree(
-    FunctionAnalysisManager &FAM) const {
-  auto &DT =
-      FAM.getResult<DominatorTreeAnalysis>(const_cast<Function &>(Caller));
-
-  SmallVector<DominatorTree::UpdateType, 2> FinalDomTreeUpdates;
-
-  for (auto &Upd : DomTreeUpdates)
-    FinalDomTreeUpdates.push_back(Upd);
-
-  DenseSet<const BasicBlock *> Inserted;
-  for (auto *Succ : successors(&CallSiteBB))
-    if (Inserted.insert(Succ).second)
-      FinalDomTreeUpdates.push_back({DominatorTree::UpdateKind::Insert,
-                                     const_cast<BasicBlock *>(&CallSiteBB),
-                                     const_cast<BasicBlock *>(Succ)});
-
-  DT.applyUpdates(FinalDomTreeUpdates);
-#ifdef EXPENSIVE_CHECKS
-  assert(DT.verify(DominatorTree::VerificationLevel::Full));
-#endif
-  return DT;
-}
-
 void FunctionPropertiesUpdater::finish(FunctionAnalysisManager &FAM) const {
   // Update feature values from the BBs that were copied from the callee, or
   // might have been modified because of inlining. The latter have been
@@ -420,7 +383,8 @@ void FunctionPropertiesUpdater::finish(FunctionAnalysisManager &FAM) const {
   // remove E.
   SetVector<const BasicBlock *> Reinclude;
   SetVector<const BasicBlock *> Unreachable;
-  auto &DT = getUpdatedDominatorTree(FAM);
+  const auto &DT =
+      FAM.getResult<DominatorTreeAnalysis>(const_cast<Function &>(Caller));
 
   if (&CallSiteBB != &*Caller.begin())
     Reinclude.insert(&*Caller.begin());
@@ -463,9 +427,6 @@ void FunctionPropertiesUpdater::finish(FunctionAnalysisManager &FAM) const {
 
   const auto &LI = FAM.getResult<LoopAnalysis>(const_cast<Function &>(Caller));
   FPI.updateAggregateStats(Caller, LI);
-#ifdef EXPENSIVE_CHECKS
-  assert(isUpdateValid(Caller, FPI, FAM));
-#endif
 }
 
 bool FunctionPropertiesUpdater::isUpdateValid(Function &F,

llvm/lib/Analysis/MLInlineAdvisor.cpp

@@ -288,6 +288,7 @@ void MLInlineAdvisor::onSuccessfulInlining(const MLInlineAdvice &Advice,
   {
     PreservedAnalyses PA = PreservedAnalyses::all();
     PA.abandon<FunctionPropertiesAnalysis>();
+    PA.abandon<DominatorTreeAnalysis>();
     PA.abandon<LoopAnalysis>();
     FAM.invalidate(*Caller, PA);
   }