[SimplifyCFG] SimplifyTerminatorOnSelect(): fix/tune DomTree updates
authorRoman Lebedev <lebedev.ri@gmail.com>
Sun, 3 Jan 2021 18:17:11 +0000 (21:17 +0300)
committerRoman Lebedev <lebedev.ri@gmail.com>
Sun, 3 Jan 2021 22:02:02 +0000 (01:02 +0300)
We only need to remove non-TrueBB/non-FalseBB successors,
and we only need to do that once. We don't need to insert
any new edges, because no new successors will be added.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/SimplifyCFG/SimplifyTerminatorOnSelect-domtree-preservation-edgecase.ll

index d4f7775..f02ff22 100644 (file)
@@ -3849,7 +3849,6 @@ bool SimplifyCFGOpt::SimplifyTerminatorOnSelect(Instruction *OldTerm,
                                                 uint32_t TrueWeight,
                                                 uint32_t FalseWeight) {
   auto *BB = OldTerm->getParent();
-
   // Remove any superfluous successor edges from the CFG.
   // First, figure out which successors to preserve.
   // If TrueBB and FalseBB are equal, only try to preserve one copy of that
@@ -3857,7 +3856,7 @@ bool SimplifyCFGOpt::SimplifyTerminatorOnSelect(Instruction *OldTerm,
   BasicBlock *KeepEdge1 = TrueBB;
   BasicBlock *KeepEdge2 = TrueBB != FalseBB ? FalseBB : nullptr;
 
-  SmallVector<DominatorTree::UpdateType, 4> Updates;
+  SmallSetVector<BasicBlock *, 2> RemovedSuccessors;
 
   // Then remove the rest.
   for (BasicBlock *Succ : successors(OldTerm)) {
@@ -3869,7 +3868,9 @@ bool SimplifyCFGOpt::SimplifyTerminatorOnSelect(Instruction *OldTerm,
     else {
       Succ->removePredecessor(BB,
                               /*KeepOneInputPHIs=*/true);
-      Updates.push_back({DominatorTree::Delete, BB, Succ});
+
+      if (Succ != TrueBB && Succ != FalseBB)
+        RemovedSuccessors.insert(Succ);
     }
   }
 
@@ -3882,13 +3883,10 @@ bool SimplifyCFGOpt::SimplifyTerminatorOnSelect(Instruction *OldTerm,
       // We were only looking for one successor, and it was present.
       // Create an unconditional branch to it.
       Builder.CreateBr(TrueBB);
-      Updates.push_back({DominatorTree::Insert, BB, TrueBB});
     } else {
       // We found both of the successors we were looking for.
       // Create a conditional branch sharing the condition of the select.
       BranchInst *NewBI = Builder.CreateCondBr(Cond, TrueBB, FalseBB);
-      Updates.push_back({DominatorTree::Insert, BB, TrueBB});
-      Updates.push_back({DominatorTree::Insert, BB, FalseBB});
       if (TrueWeight != FalseWeight)
         setBranchWeights(NewBI, TrueWeight, FalseWeight);
     }
@@ -3903,17 +3901,22 @@ bool SimplifyCFGOpt::SimplifyTerminatorOnSelect(Instruction *OldTerm,
     if (!KeepEdge1) {
       // Only TrueBB was found.
       Builder.CreateBr(TrueBB);
-      Updates.push_back({DominatorTree::Insert, BB, TrueBB});
     } else {
       // Only FalseBB was found.
       Builder.CreateBr(FalseBB);
-      Updates.push_back({DominatorTree::Insert, BB, FalseBB});
     }
   }
 
   EraseTerminatorAndDCECond(OldTerm);
-  if (DTU)
-    DTU->applyUpdatesPermissive(Updates);
+
+  if (DTU) {
+    SmallVector<DominatorTree::UpdateType, 2> Updates;
+    Updates.reserve(RemovedSuccessors.size());
+    for (auto *RemovedSuccessor : RemovedSuccessors)
+      Updates.push_back({DominatorTree::Delete, BB, RemovedSuccessor});
+    DTU->applyUpdates(Updates);
+  }
+
   return true;
 }
 
index 752de63..792a1a5 100644 (file)
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -S -simplifycfg -simplifycfg-require-and-preserve-domtree=0 < %s | FileCheck %s
+; RUN: opt -S -simplifycfg -simplifycfg-require-and-preserve-domtree=1 < %s | FileCheck %s
 
 declare void @widget()
 declare void @baz(i8)