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

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/ADCE.cpp:582
   bool Changed = false;
+  SmallVector<DominatorTree::UpdateType, 16> DeletedEdges;
 
----------------
kuhar wrote:
> How did you pick this constant? 
Randomly to be honest.

It was 4 for the incremental updates and I figured we need to increase this size while batching more updates together while still hitting the fast path of the container.


================
Comment at: llvm/lib/Transforms/Scalar/ADCE.cpp:636
 
+  if (!DeletedEdges.empty())
+    DomTreeUpdater(DT, &PDT, DomTreeUpdater::UpdateStrategy::Eager)
----------------
kuhar wrote:
> How about we move this as an early exit in `DomTreeUpdater::applyUpdates`?
It is, more or less, already there (this is buried in the domtreeconstruction update code), I just didn't want to assume anything on the client side, plus we save on the instantiation of the DomTreeUpdater all together.

The first iteration of the patch didn't have this check.
Put differently, I am happy to get rid of it if you believe it's not helpful.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116610/new/

https://reviews.llvm.org/D116610