fixup! respond to review

Author: michaelmaitland

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

@@ -7450,7 +7450,7 @@ struct SwitchSuccWrapper {
   // be important to equality though.
   unsigned SuccNum;
   BasicBlock *Dest;
-  DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> *PhiPredIVs;
+  DenseMap<PHINode *, SmallDenseMap<BasicBlock *, Value *, 8>> *PhiPredIVs;
 };
 
 namespace llvm {
@@ -7473,14 +7473,13 @@ template <> struct DenseMapInfo<const SwitchSuccWrapper *> {
     assert(Succ->size() == 1 && "Expected just a single branch in the BB");
 
     // Since we assume the BB is just a single BranchInst with a single
-    // succsessor, we hash as the BB and the incoming Values of its successor
+    // successor, we hash as the BB and the incoming Values of its successor
     // PHIs. Initially, we tried to just use the successor BB as the hash, but
-    // this had poor performance. We find that the extra computation of getting
-    // the incoming PHI values here leads to better performance on overall Set
-    // performance. We also tried to build a map from BB -> Succs.IncomingValues
-    // ahead of time and passing it in SwitchSuccWrapper, but this slowed down
-    // the average compile time without having any impact on the worst case
-    // compile time.
+    // including the incoming PHI values leads to better performance.
+    // We also tried to build a map from BB -> Succs.IncomingValues ahead of
+    // time and passing it in SwitchSuccWrapper, but this slowed down the
+    // average compile time without having any impact on the worst case compile
+    // time.
     BasicBlock *BB = BI->getSuccessor(0);
     SmallVector<Value *> PhiValsForBB;
     for (PHINode &Phi : BB->phis())
@@ -7535,7 +7534,7 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
   // getIncomingValueForBlock is O(|Preds|).
   SmallPtrSet<PHINode *, 8> Phis;
   SmallPtrSet<BasicBlock *, 8> Seen;
-  DenseMap<PHINode *, DenseMap<BasicBlock *, Value *>> PhiPredIVs;
+  DenseMap<PHINode *, SmallDenseMap<BasicBlock *, Value *, 8>> PhiPredIVs;
   SmallVector<SwitchSuccWrapper> Cases;
   Cases.reserve(SI->getNumSuccessors());
 
@@ -7547,10 +7546,6 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
     if (BB->size() != 1)
       continue;
 
-    Instruction *T = BB->getTerminator();
-    if (!T)
-      continue;
-
     // FIXME: This case needs some extra care because the terminators other than
     // SI need to be updated.
     if (BB->hasNPredecessorsOrMore(2))
@@ -7559,7 +7554,7 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
     // FIXME: Relax that the terminator is a BranchInst by checking for equality
     // on other kinds of terminators. We decide to only support unconditional
     // branches for now for compile time reasons.
-    auto *BI = dyn_cast<BranchInst>(T);
+    auto *BI = dyn_cast<BranchInst>(BB->getTerminator());
     if (!BI || BI->isConditional())
       continue;
 
@@ -7577,7 +7572,7 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
   PhiPredIVs.reserve(Phis.size());
   for (PHINode *Phi : Phis) {
     PhiPredIVs[Phi] =
-        DenseMap<BasicBlock *, Value *>(Phi->getNumIncomingValues());
+        SmallDenseMap<BasicBlock *, Value *, 8>(Phi->getNumIncomingValues());
     for (auto &IV : Phi->incoming_values())
       PhiPredIVs[Phi].insert({Phi->getIncomingBlock(IV), IV.get()});
   }
@@ -7590,8 +7585,7 @@ bool SimplifyCFGOpt::simplifyDuplicateSwitchArms(SwitchInst *SI,
   // SwitchSuccWrapper instead of just BasicBlock because we'd like to pass
   // around information to isEquality, getHashValue, and when doing the
   // replacement with better performance.
-  DenseSet<const SwitchSuccWrapper *, DenseMapInfo<const SwitchSuccWrapper *>>
-      ReplaceWith;
+  DenseSet<const SwitchSuccWrapper *> ReplaceWith;
   ReplaceWith.reserve(Cases.size());
 
   SmallVector<DominatorTree::UpdateType> Updates;

llvm/test/Transforms/SimplifyCFG/switch-dup-bbs.ll

@@ -1,6 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt < %s -passes=simplifycfg -S | FileCheck %s -check-prefix=SIMPLIFY-CFG
-; RUN: opt < %s -O3 -S | FileCheck %s -check-prefix=O3
+; RUN: opt < %s -passes=simplifycfg -simplifycfg-require-and-preserve-domtree=1 -S \
+; RUN:   | FileCheck %s -check-prefix=SIMPLIFY-CFG
 
 define i32 @switch_all_duplicate_arms(i32 %0, i32 %1, i32 %2, i32 %3) {
 ; SIMPLIFY-CFG-LABEL: define i32 @switch_all_duplicate_arms(
@@ -14,12 +14,6 @@ define i32 @switch_all_duplicate_arms(i32 %0, i32 %1, i32 %2, i32 %3) {
 ; SIMPLIFY-CFG:       [[BB6]]:
 ; SIMPLIFY-CFG-NEXT:    [[TMP8:%.*]] = phi i32 [ [[TMP3]], [[TMP4:%.*]] ], [ [[TMP2]], %[[BB5]] ]
 ; SIMPLIFY-CFG-NEXT:    ret i32 [[TMP8]]
-;
-; O3-LABEL: define i32 @switch_all_duplicate_arms(
-; O3-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]], i32 [[TMP2:%.*]], i32 [[TMP3:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
-; O3-NEXT:    [[SWITCH:%.*]] = icmp ult i32 [[TMP1]], 2
-; O3-NEXT:    [[TMP8:%.*]] = select i1 [[SWITCH]], i32 [[TMP2]], i32 [[TMP3]]
-; O3-NEXT:    ret i32 [[TMP8]]
 ;
   switch i32 %1, label %7 [
   i32 0, label %5
@@ -52,21 +46,6 @@ define i32 @switch_some_duplicate_arms(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4) {
 ; SIMPLIFY-CFG:       [[BB8]]:
 ; SIMPLIFY-CFG-NEXT:    [[TMP10:%.*]] = phi i32 [ [[TMP3]], [[TMP5:%.*]] ], [ [[TMP4]], %[[BB7]] ], [ [[TMP2]], %[[BB6]] ]
 ; SIMPLIFY-CFG-NEXT:    ret i32 [[TMP10]]
-;
-; O3-LABEL: define i32 @switch_some_duplicate_arms(
-; O3-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]], i32 [[TMP2:%.*]], i32 [[TMP3:%.*]], i32 [[TMP4:%.*]]) local_unnamed_addr #[[ATTR0]] {
-; O3-NEXT:    switch i32 [[TMP1]], label %[[BB8:.*]] [
-; O3-NEXT:      i32 0, label %[[BB6:.*]]
-; O3-NEXT:      i32 1, label %[[BB6]]
-; O3-NEXT:      i32 2, label %[[BB7:.*]]
-; O3-NEXT:    ]
-; O3:       [[BB6]]:
-; O3-NEXT:    br label %[[BB8]]
-; O3:       [[BB7]]:
-; O3-NEXT:    br label %[[BB8]]
-; O3:       [[BB8]]:
-; O3-NEXT:    [[TMP10:%.*]] = phi i32 [ [[TMP3]], [[TMP5:%.*]] ], [ [[TMP4]], %[[BB7]] ], [ [[TMP2]], %[[BB6]] ]
-; O3-NEXT:    ret i32 [[TMP10]]
 ;
   switch i32 %1, label %9 [
   i32 0, label %6
@@ -104,22 +83,6 @@ define i32 @switch_duplicate_arms_multipred(i1 %0, i32 %1, i32 %2, i32 %3, i32 %
 ; SIMPLIFY-CFG:       [[BB9]]:
 ; SIMPLIFY-CFG-NEXT:    [[TMP10:%.*]] = phi i32 [ [[TMP4]], %[[BB6]] ], [ [[TMP3]], %[[BB8]] ], [ [[TMP3]], %[[BB7]] ]
 ; SIMPLIFY-CFG-NEXT:    ret i32 [[TMP10]]
-;
-; O3-LABEL: define i32 @switch_duplicate_arms_multipred(
-; O3-SAME: i1 [[TMP0:%.*]], i32 [[TMP1:%.*]], i32 [[TMP2:%.*]], i32 [[TMP3:%.*]], i32 [[TMP4:%.*]]) local_unnamed_addr #[[ATTR0]] {
-; O3-NEXT:    br i1 [[TMP0]], label %[[BB6:.*]], label %[[BB7:.*]]
-; O3:       [[BB6]]:
-; O3-NEXT:    switch i32 [[TMP2]], label %[[BB9:.*]] [
-; O3-NEXT:      i32 0, label %[[BB7]]
-; O3-NEXT:      i32 1, label %[[BB8:.*]]
-; O3-NEXT:    ]
-; O3:       [[BB7]]:
-; O3-NEXT:    br label %[[BB9]]
-; O3:       [[BB8]]:
-; O3-NEXT:    br label %[[BB9]]
-; O3:       [[BB9]]:
-; O3-NEXT:    [[TMP10:%.*]] = phi i32 [ [[TMP4]], %[[BB6]] ], [ [[TMP3]], %[[BB8]] ], [ [[TMP3]], %[[BB7]] ]
-; O3-NEXT:    ret i32 [[TMP10]]
 ;
   br i1 %0, label %6, label %7
 6:
@@ -145,12 +108,6 @@ define i32 @switch_dup_default(i32 %0, i32 %1, i32 %2, i32 %3) {
 ; SIMPLIFY-CFG-NEXT:    [[COND:%.*]] = icmp eq i32 [[TMP1]], 0
 ; SIMPLIFY-CFG-NEXT:    [[TMP8:%.*]] = select i1 [[COND]], i32 [[TMP3]], i32 [[TMP2]]
 ; SIMPLIFY-CFG-NEXT:    ret i32 [[TMP8]]
-;
-; O3-LABEL: define i32 @switch_dup_default(
-; O3-SAME: i32 [[TMP0:%.*]], i32 [[TMP1:%.*]], i32 [[TMP2:%.*]], i32 [[TMP3:%.*]]) local_unnamed_addr #[[ATTR0]] {
-; O3-NEXT:    [[COND:%.*]] = icmp eq i32 [[TMP1]], 0
-; O3-NEXT:    [[TMP8:%.*]] = select i1 [[COND]], i32 [[TMP3]], i32 [[TMP2]]
-; O3-NEXT:    ret i32 [[TMP8]]
 ;
   switch i32 %1, label %7 [
   i32 0, label %5