[nfc] Incrementally update DominatorTreeAnalysis in FunctionPropertiesAnalysis

Author: mtrofin

llvm/include/llvm/Analysis/FunctionPropertiesAnalysis.h

@@ -15,6 +15,7 @@
 #define LLVM_ANALYSIS_FUNCTIONPROPERTIESANALYSIS_H
 
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/IR/Dominators.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
@@ -186,7 +187,12 @@ 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,6 +326,14 @@ 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
@@ -336,6 +344,11 @@ 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).
@@ -356,6 +369,45 @@ FunctionPropertiesUpdater::FunctionPropertiesUpdater(
     FPI.updateForBB(*BB, -1);
 }
 
+DominatorTree &FunctionPropertiesUpdater::getUpdatedDominatorTree(
+    FunctionAnalysisManager &FAM) const {
+  auto &DT =
+      FAM.getResult<DominatorTreeAnalysis>(const_cast<Function &>(Caller));
+
+  SetVector<const BasicBlock *> NewSucc;
+  NewSucc.insert(succ_begin(&CallSiteBB), succ_end(&CallSiteBB));
+
+  // tell the DomTree about the new edges
+  std::deque<const BasicBlock *> Worklist;
+  Worklist.push_back(&CallSiteBB);
+
+  // Build the list of edges to actually remove. Those are those edges in the
+  // DomTreeUpdates that cannot be found in the CFG anymore.
+  SmallVector<DominatorTree::UpdateType, 2> FinalDomTreeUpdates;
+  while (!Worklist.empty()) {
+    auto *BB = Worklist.front();
+    Worklist.pop_front();
+    assert(DT.getNode(BB));
+
+    for (auto *Succ : NewSucc) {
+      if (!DT.getNode(Succ))
+        Worklist.push_back(Succ);
+      FinalDomTreeUpdates.push_back({DominatorTree::UpdateKind::Insert,
+                                     const_cast<BasicBlock *>(BB),
+                                     const_cast<BasicBlock *>(Succ)});
+    }
+  }
+  for (auto &Upd : DomTreeUpdates)
+    if (!llvm::is_contained(successors(Upd.getFrom()), Upd.getTo()))
+      FinalDomTreeUpdates.push_back(Upd);
+
+  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
@@ -383,8 +435,7 @@ void FunctionPropertiesUpdater::finish(FunctionAnalysisManager &FAM) const {
   // remove E.
   SetVector<const BasicBlock *> Reinclude;
   SetVector<const BasicBlock *> Unreachable;
-  const auto &DT =
-      FAM.getResult<DominatorTreeAnalysis>(const_cast<Function &>(Caller));
+  auto &DT = getUpdatedDominatorTree(FAM);
 
   if (&CallSiteBB != &*Caller.begin())
     Reinclude.insert(&*Caller.begin());
@@ -427,6 +478,9 @@ 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,7 +288,6 @@ void MLInlineAdvisor::onSuccessfulInlining(const MLInlineAdvice &Advice,
   {
     PreservedAnalyses PA = PreservedAnalyses::all();
     PA.abandon<FunctionPropertiesAnalysis>();
-    PA.abandon<DominatorTreeAnalysis>();
     PA.abandon<LoopAnalysis>();
     FAM.invalidate(*Caller, PA);
   }

test-cfg-only.dot

@@ -0,0 +1,12 @@
+digraph "CFG for 'f' function" {
+	label="CFG for 'f' function";
+
+	Node0x5640f3daad80 [shape=record,label="{bb0|{<s0>T|<s1>F}}"];
+	Node0x5640f3daad80:s0 -> Node0x5640f3d8f320;
+	Node0x5640f3daad80:s1 -> Node0x5640f3d9eb90;
+	Node0x5640f3d8f320 [shape=record,label="{bb1}"];
+	Node0x5640f3d8f320 -> Node0x5640f3da2ea0;
+	Node0x5640f3d9eb90 [shape=record,label="{bb2}"];
+	Node0x5640f3d9eb90 -> Node0x5640f3da2ea0;
+	Node0x5640f3da2ea0 [shape=record,label="{bb3}"];
+}

test-full.dot

@@ -0,0 +1,12 @@
+digraph "CFG for 'f' function" {
+	label="CFG for 'f' function";
+
+	Node0x5640f3daad80 [shape=record,label="{bb0:\l|  %y1 = icmp eq i32 %x, 0\l  br i1 %y1, label %bb1, label %bb2\l|{<s0>T|<s1>F}}"];
+	Node0x5640f3daad80:s0 -> Node0x5640f3d8f320;
+	Node0x5640f3daad80:s1 -> Node0x5640f3d9eb90;
+	Node0x5640f3d8f320 [shape=record,label="{bb1:\l|  br label %bb3\l}"];
+	Node0x5640f3d8f320 -> Node0x5640f3da2ea0;
+	Node0x5640f3d9eb90 [shape=record,label="{bb2:\l|  br label %bb3\l}"];
+	Node0x5640f3d9eb90 -> Node0x5640f3da2ea0;
+	Node0x5640f3da2ea0 [shape=record,label="{bb3:\l|  %y2 = phi i32 [ 0, %bb1 ], [ 1, %bb2 ]\l  ret i32 %y2\l}"];
+}