[PATCH] D141487: [LoopUnroll] Directly update DT instead of DTU.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 2 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h:98
+                               bool PredecessorWithTwoSuccessors = false,
+                               DominatorTree *DT = nullptr);
 
----------------
nikic wrote:
> Here and elsewhere, please add a comment -- normally, if there's a DT rather than DTU argument, I'd expect the DT to be used rather than updated.
Good point, added a comment here and for `changeToUnreachable`


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:239
+    DomTreeNode *PredNode = DT->getNode(PredBB);
+    DomTreeNode *BBNode = DT->getNode(BB);
+    SmallVector<DomTreeNode *> Children(BBNode->children());
----------------
nikic wrote:
> Do we need to check whether PredNode / BBNode are null? A bit degenerate, but we might be merging two unreachable blocks, no?
Added a check. I think it should be sufficient to check if PRedNode is reachable (!= nullptr), as `BB` must have `PredBB` as unique predecessor.

Also added a check to updateDominatorTreeUsingPredecessors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141487