[PATCH] D101289: [SimplifyCFG] Use applyUpdatesPermissive in HoistThenElseCodeToIf.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 26 12:58:36 PDT 2021


lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

LG with nits, thanks.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1613
 
+  SmallSetVector<BasicBlock *, 4> SuccsToUpdate;
   // Update any PHI nodes in our new successors.
----------------
You don't need this to be a vector.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1621-1622
 
-  if (DTU)
-    for (BasicBlock *Succ : successors(BI))
-      Updates.push_back({DominatorTree::Delete, BIParent, Succ});
+  if (DTU) {
+    for (BasicBlock *Succ : SuccsToUpdate)
+      Updates.push_back({DominatorTree::Insert, BIParent, Succ});
----------------



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1620-1622
   if (DTU)
     for (BasicBlock *Succ : successors(BI))
       Updates.push_back({DominatorTree::Delete, BIParent, Succ});
----------------
fhahn wrote:
> lebedev.ri wrote:
> > 
> Done! I think we also need handling for the `Insert` part, because `BB1` could also have duplicated successors.
Hmm. Only if you can add a test that will crash otherwise :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101289



More information about the llvm-commits mailing list