[PATCH] D92200: Ensure SplitEdge to return the new block between the two given blocks

Bangtian Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 27 11:51:34 PST 2020

kobeliu85 added inline comments.

Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:512
     assert(SP == BB && "CFG broken");
     SP = nullptr;
+    return SplitBlock(Succ, &Succ->front(), DT, LI, MSSAU, "", /*Before=*/true);
hoy wrote:
> Nit: Is resetting `SP` still needed?
I also don't understand why it is needed in the first place, and it is from the original code, so I didn't want to change it.  
Whether do you know about it and if so could you explain why?

Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:595
+  if (DT) {
+    DomTreeUpdater DTU(DT, DomTreeUpdater::UpdateStrategy::Lazy);
+    SmallVector<DominatorTree::UpdateType, 8> DTUpdates;
hoy wrote:
> Can we just change the dominator tree edges directly instead of going through the dominator updater?
> like 
> ```
> OldIdom = OldNode->getIDom();
> DT->changeImmediateDominator(OldNode, NewNode);
> DT->changeImmediateDominator(NewNode, OldIdom)
> ```
 DomTreeUpdater provides a uniform way to update dominator tree related data structures, e.g. DT, MSSA, PDT.
My first version implementation modified the DT directly and then switched to DTUpdater by following the suggestion from another reviewer. 

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list