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

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h:98
+                               bool PredecessorWithTwoSuccessors = false,
+                               DominatorTree *DT = nullptr);
 
----------------
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.


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:239
+    DomTreeNode *PredNode = DT->getNode(PredBB);
+    DomTreeNode *BBNode = DT->getNode(BB);
+    SmallVector<DomTreeNode *> Children(BBNode->children());
----------------
Do we need to check whether PredNode / BBNode are null? A bit degenerate, but we might be merging two unreachable blocks, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141487