[PATCH] D116610: [ADCE][NFC] Batch DT updates together

Quentin Colombet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 4 11:22:11 PST 2022


qcolombet created this revision.
qcolombet added reviewers: NutshellySima, nikic, mkazantsev, kuhar.
Herald added a subscriber: hiraditya.
qcolombet requested review of this revision.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116610

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


Index: llvm/lib/Transforms/Scalar/ADCE.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/ADCE.cpp
+++ llvm/lib/Transforms/Scalar/ADCE.cpp
@@ -579,6 +579,7 @@
   // Don't compute the post ordering unless we needed it.
   bool HavePostOrder = false;
   bool Changed = false;
+  SmallVector<DominatorTree::UpdateType, 16> DeletedEdges;
 
   for (auto *BB : BlocksWithDeadTerminators) {
     auto &Info = BlockInfo[BB];
@@ -617,7 +618,6 @@
     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 @@
       }
     }
 
-    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;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116610.397342.patch
Type: text/x-patch
Size: 1203 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220104/70db80a8/attachment.bin>


More information about the llvm-commits mailing list