Merge pull request #4728 from shajrawi/enum_unique_values

[SR-2512] Ignore duplicate conditions when optimizing switch statement

lib/SILOptimizer/Transforms/SimplifyCFG.cpp

@@ -3190,6 +3190,7 @@ bool simplifyToSelectValue(SILBasicBlock *MergeBlock, unsigned ArgNum,
   }
 
   SmallVector<std::pair<SILValue, SILValue>, 8> Cases;
+  llvm::SmallDenseMap<SILValue, SILValue> CaseLiteralsToResultMap;
   SILValue defaultResult;
 
   // The block of the first input value compare. It dominates all other blocks
@@ -3202,7 +3203,23 @@ bool simplifyToSelectValue(SILBasicBlock *MergeBlock, unsigned ArgNum,
       auto *BrInst = cast<CondBranchInst>(CaseInfo.CmpOrDefault->getTerminator());
       if (FoundCmpBlocks.count(BrInst->getFalseBB()) != 1)
         return false;
-      Cases.push_back({CaseInfo.Literal, CaseInfo.Result});
+      // Ignore duplicate cases
+      if (CaseLiteralsToResultMap.find(CaseInfo.Literal) ==
+          CaseLiteralsToResultMap.end()) {
+        CaseLiteralsToResultMap.insert({CaseInfo.Literal, CaseInfo.Result});
+        Cases.push_back({CaseInfo.Literal, CaseInfo.Result});
+      } else {
+        // Check if the result value matches
+        EnumInst *PrevResult =
+            dyn_cast<EnumInst>(CaseLiteralsToResultMap[CaseInfo.Literal]);
+        assert(PrevResult && "Prev. case result is not an EnumInst");
+        EnumInst *CurrResult = dyn_cast<EnumInst>(CaseInfo.Result);
+        assert(CurrResult && "Curr. case result is not an EnumInst");
+        if (PrevResult->getElement() != CurrResult->getElement()) {
+          // result value does not match - bail
+          return false;
+        }
+      }
       SILBasicBlock *Pred = CaseInfo.CmpOrDefault->getSinglePredecessor();
       if (!Pred || FoundCmpBlocks.count(Pred) == 0) {
         // There may be only a single block whose predecessor we didn't see. And

test/SILOptimizer/simplify_cfg_unique_values.sil

@@ -0,0 +1,90 @@
+// RUN: %target-sil-opt -enable-sil-verify-all %s -simplify-cfg | %FileCheck %s
+
+sil_stage canonical
+
+import Builtin
+import Swift
+
+enum DupCaseEnum {
+  case firstCase
+  case secondCase
+}
+
+// CHECK-LABEL: sil @performSwitch : $@convention(thin) (Int, @thin DupCaseEnum.Type) -> DupCaseEnum {
+// CHECK: bb0(%0 : $Int, %1 : $@thin DupCaseEnum.Type):
+// CHECK:   select_value
+// CHECK:   br bb1
+// CHECK: bb1:
+// CHECK:   return
+sil @performSwitch : $@convention(thin) (Int, @thin DupCaseEnum.Type) -> DupCaseEnum {
+// %0                                             // users: %9, %5, %3, %2
+bb0(%0 : $Int, %1 : $@thin DupCaseEnum.Type):
+  %4 = integer_literal $Builtin.Int64, 0 // user: %6
+  %5 = struct_extract %0 : $Int, #Int._value // user: %6
+  %6 = builtin "cmp_eq_Int64"(%4 : $Builtin.Int64, %5 : $Builtin.Int64) : $Builtin.Int1 // users: %10, %7
+  cond_br %6, bb6, bb1                   // id: %7
+
+bb1:                                              // Preds: bb0
+  br bb2                                 // id: %8
+
+bb2:                                              // Preds: bb1
+  cond_br %6, bb5, bb3                   // id: %10
+
+bb3:                                              // Preds: bb2
+  br bb4                                 // id: %11
+
+bb4:                                              // Preds: bb3
+  %12 = enum $DupCaseEnum, #DupCaseEnum.secondCase!enumelt // user: %13
+  br bb7(%12 : $DupCaseEnum)                // id: %13
+
+bb5:                                              // Preds: bb2
+  %14 = enum $DupCaseEnum, #DupCaseEnum.firstCase!enumelt // user: %15
+  br bb7(%14 : $DupCaseEnum)                // id: %15
+
+bb6:                                              // Preds: bb0
+  %16 = enum $DupCaseEnum, #DupCaseEnum.firstCase!enumelt // user: %17
+  br bb7(%16 : $DupCaseEnum)                // id: %17
+
+// %18                                            // user: %19
+bb7(%18 : $DupCaseEnum):                             // Preds: bb6 bb5 bb4
+  return %18 : $DupCaseEnum                 // id: %19
+}
+
+// CHECK-LABEL: sil @performSwitch_bail_out : $@convention(thin) (Int, @thin DupCaseEnum.Type) -> DupCaseEnum {
+// CHECK: bb0(%0 : $Int, %1 : $@thin DupCaseEnum.Type):
+// CHECK-NOT:   select_value
+// CHECK-NOT:   br bb1
+// CHECK:   cond_br
+sil @performSwitch_bail_out : $@convention(thin) (Int, @thin DupCaseEnum.Type) -> DupCaseEnum {
+// %0                                             // users: %9, %5, %3, %2
+bb0(%0 : $Int, %1 : $@thin DupCaseEnum.Type):
+  %4 = integer_literal $Builtin.Int64, 0 // user: %6
+  %5 = struct_extract %0 : $Int, #Int._value // user: %6
+  %6 = builtin "cmp_eq_Int64"(%4 : $Builtin.Int64, %5 : $Builtin.Int64) : $Builtin.Int1 // users: %10, %7
+  cond_br %6, bb6, bb1                   // id: %7
+
+bb1:                                              // Preds: bb0
+  br bb2                                 // id: %8
+
+bb2:                                              // Preds: bb1
+  cond_br %6, bb5, bb3                   // id: %10
+
+bb3:                                              // Preds: bb2
+  br bb4                                 // id: %11
+
+bb4:                                              // Preds: bb3
+  %12 = enum $DupCaseEnum, #DupCaseEnum.secondCase!enumelt // user: %13
+  br bb7(%12 : $DupCaseEnum)                // id: %13
+
+bb5:                                              // Preds: bb2
+  %14 = enum $DupCaseEnum, #DupCaseEnum.secondCase!enumelt // user: %15
+  br bb7(%14 : $DupCaseEnum)                // id: %15
+
+bb6:                                              // Preds: bb0
+  %16 = enum $DupCaseEnum, #DupCaseEnum.firstCase!enumelt // user: %17
+  br bb7(%16 : $DupCaseEnum)                // id: %17
+
+// %18                                            // user: %19
+bb7(%18 : $DupCaseEnum):                             // Preds: bb6 bb5 bb4
+  return %18 : $DupCaseEnum                 // id: %19
+}