[llvm] cdbad62 - [ADCE][NFC] Batch DT updates together

[Quentin Colombet via llvm-commits]

Author: Quentin Colombet
Date: 2022-01-05T14:05:20-08:00
New Revision: cdbad62c526c5b7e13f634b9d6bc54f2a01aabc0

URL: https://github.com/llvm/llvm-project/commit/cdbad62c526c5b7e13f634b9d6bc54f2a01aabc0
DIFF: https://github.com/llvm/llvm-project/commit/cdbad62c526c5b7e13f634b9d6bc54f2a01aabc0.diff

LOG: [ADCE][NFC] Batch DT updates together

This patch delayed the updates of the dominator tree to the very end of
the pass instead of doing that in small increments after each basic
block.

This improves the runtime of the pass in particular in pathological
cases because now the updater sees the full extend of the updates and
can decide whether it is faster to apply the changes incrementally or
just recompute the full tree from scratch.

Put differently, thanks to this patch, we can take advantage of the
improvements that Chijun Sima <simachijun at gmail.com> made in the
dominator tree updater a while ago with commit 32fd196cbf4d: "Teach the
DominatorTree fallback to recalculation when applying updates to speedup
JT (PR37929)".

This change is NFC but can improve the runtime of the compiler
dramatically in some pathological cases (where the pass was pushing a
lot (several thousands) of small updates (less than 6)).

For instance on the motivating example we went from 300+ sec to less
than a second.

Differential Revision: https://reviews.llvm.org/D116610

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/ADCE.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/ADCE.cpp b/llvm/lib/Transforms/Scalar/ADCE.cpp
index b693acceb3f6c..1cda206a7e147 100644
--- a/llvm/lib/Transforms/Scalar/ADCE.cpp
+++ b/llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -579,6 +579,7 @@ bool AggressiveDeadCodeElimination::updateDeadRegions() {
   // Don't compute the post ordering unless we needed it.
   bool HavePostOrder = false;
   bool Changed = false;
+  SmallVector<DominatorTree::UpdateType, 10> DeletedEdges;
 
   for (auto *BB : BlocksWithDeadTerminators) {
     auto &Info = BlockInfo[BB];
@@ -617,7 +618,6 @@ bool AggressiveDeadCodeElimination::updateDeadRegions() {
     makeUnconditional(BB, PreferredSucc->BB);
 
     // Inform the dominators about the deleted CFG edges.
-    SmallVector<DominatorTree::UpdateType, 4> DeletedEdges;
     for (auto *Succ : RemovedSuccessors) {
       // It might have happened that the same successor appeared multiple times
       // and the CFG edge wasn't really removed.
@@ -629,13 +629,14 @@ bool AggressiveDeadCodeElimination::updateDeadRegions() {
       }
     }
 
-    DomTreeUpdater(DT, &PDT, DomTreeUpdater::UpdateStrategy::Eager)
-        .applyUpdates(DeletedEdges);
-
     NumBranchesRemoved += 1;
     Changed = true;
   }
 
+  if (!DeletedEdges.empty())
+    DomTreeUpdater(DT, &PDT, DomTreeUpdater::UpdateStrategy::Eager)
+        .applyUpdates(DeletedEdges);
+
   return Changed;
 }