[PATCH] D73204: [NFCI][LoopUnrollAndJam] Minor changes.

[Whitney Tsang via Phabricator via llvm-commits]

Whitney marked an inline comment as done.
Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp:525
   if (Count != 1) {
-    SmallVector<DominatorTree::UpdateType, 4> DTUpdates;
-    DTUpdates.emplace_back(DominatorTree::UpdateKind::Delete, ForeBlocksLast[0],
-                           SubLoopBlocksFirst[0]);
-    DTUpdates.emplace_back(DominatorTree::UpdateKind::Delete,
-                           SubLoopBlocksLast[0], AftBlocksFirst[0]);
-
-    DTUpdates.emplace_back(DominatorTree::UpdateKind::Insert,
-                           ForeBlocksLast.back(), SubLoopBlocksFirst[0]);
-    DTUpdates.emplace_back(DominatorTree::UpdateKind::Insert,
-                           SubLoopBlocksLast.back(), AftBlocksFirst[0]);
-    DTU.applyUpdatesPermissive(DTUpdates);
+    DT->changeImmediateDominator(SubLoopBlocksFirst[0], ForeBlocksLast.back());
+    DT->changeImmediateDominator(AftBlocksFirst[0], SubLoopBlocksLast.back());
----------------
fhahn wrote:
> Could you provide context on why not using DTU is preferred here? Is it because the existing copy/cloning code in the pass changes the DT in place? I just had a quick look and it looks like the DT is not accessed in between cloning until simplifying the loops after unrolling. 
> 
> DTU has the advantage that we can preserve the PDT also for almost no extra cost (just pass a PDT to the DTU additionally). Also, batch updates can be much faster, given that the update set is large enough. 
> 
> I think unless there's a strong reason, we should try to replace the existing uses of in-place updates with DTU update.
ok, I will revert my change here. The main reason I did that is it is easier to understand. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73204