Reland [SimplifyCFG] Delete the unnecessary range check for small mas…

…k operation (#70542) Fix the compile crash when the default result has no result for llvm/llvm-project#65835 Fixes llvm/llvm-project#65120 Reviewed By: zmodem, nikic
rvgpu · Nov 3, 2023 · 7c4180a · 7c4180a
1 parent 1021404
commit 7c4180a
Show file tree

Hide file tree

Showing 2 changed files with 102 additions and 16 deletions.
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6598,9 +6598,8 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
   // If the default destination is unreachable, or if the lookup table covers
   // all values of the conditional variable, branch directly to the lookup table
   // BB. Otherwise, check that the condition is within the case range.
-  const bool DefaultIsReachable =
+  bool DefaultIsReachable =
       !isa<UnreachableInst>(SI->getDefaultDest()->getFirstNonPHIOrDbg());
-  const bool GeneratingCoveredLookupTable = (MaxTableSize == TableSize);
 
   // Create the BB that does the lookups.
   Module &Mod = *CommonDest->getParent()->getParent();
@@ -6631,6 +6630,27 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder,
 
   BranchInst *RangeCheckBranch = nullptr;
 
+  // Grow the table to cover all possible index values to avoid the range check.
+  // It will use the default result to fill in the table hole later, so make
+  // sure it exist.
+  if (UseSwitchConditionAsTableIndex && HasDefaultResults) {
+    ConstantRange CR = computeConstantRange(TableIndex, /* ForSigned */ false);
+    // Grow the table shouldn't have any size impact by checking
+    // WouldFitInRegister.
+    // TODO: Consider growing the table also when it doesn't fit in a register
+    // if no optsize is specified.
+    if (all_of(ResultTypes, [&](const auto &KV) {
+          return SwitchLookupTable::WouldFitInRegister(
+              DL, CR.getUpper().getLimitedValue(), KV.second /* ResultType */);
+        })) {
+      // The default branch is unreachable after we enlarge the lookup table.
+      // Adjust DefaultIsReachable to reuse code path.
+      TableSize = CR.getUpper().getZExtValue();
+      DefaultIsReachable = false;
+    }
+  }
+
+  const bool GeneratingCoveredLookupTable = (MaxTableSize == TableSize);
   if (!DefaultIsReachable || GeneratingCoveredLookupTable) {
     Builder.CreateBr(LookupBB);
     if (DTU)

diff --git a/llvm/test/Transforms/SimplifyCFG/switch_mask.ll b/llvm/test/Transforms/SimplifyCFG/switch_mask.ll
@@ -3,18 +3,18 @@
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
+declare i1 @foo()
+
 ; https://alive2.llvm.org/ce/z/tuxLhJ
 define i1 @switch_lookup_with_small_i1(i64 %x) {
 ; CHECK-LABEL: @switch_lookup_with_small_i1(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[AND:%.*]] = and i64 [[X:%.*]], 15
-; CHECK-NEXT:    [[TMP0:%.*]] = icmp ult i64 [[AND]], 11
-; CHECK-NEXT:    [[SWITCH_CAST:%.*]] = trunc i64 [[AND]] to i11
-; CHECK-NEXT:    [[SWITCH_SHIFTAMT:%.*]] = mul nuw nsw i11 [[SWITCH_CAST]], 1
-; CHECK-NEXT:    [[SWITCH_DOWNSHIFT:%.*]] = lshr i11 -1018, [[SWITCH_SHIFTAMT]]
-; CHECK-NEXT:    [[SWITCH_MASKED:%.*]] = trunc i11 [[SWITCH_DOWNSHIFT]] to i1
-; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[TMP0]], i1 [[SWITCH_MASKED]], i1 false
-; CHECK-NEXT:    ret i1 [[TMP1]]
+; CHECK-NEXT:    [[SWITCH_CAST:%.*]] = trunc i64 [[AND]] to i16
+; CHECK-NEXT:    [[SWITCH_SHIFTAMT:%.*]] = mul nuw nsw i16 [[SWITCH_CAST]], 1
+; CHECK-NEXT:    [[SWITCH_DOWNSHIFT:%.*]] = lshr i16 1030, [[SWITCH_SHIFTAMT]]
+; CHECK-NEXT:    [[SWITCH_MASKED:%.*]] = trunc i16 [[SWITCH_DOWNSHIFT]] to i1
+; CHECK-NEXT:    ret i1 [[SWITCH_MASKED]]
 ;
 entry:
   %and = and i64 %x, 15
@@ -37,13 +37,11 @@ define i8 @switch_lookup_with_small_i8(i64 %x) {
 ; CHECK-LABEL: @switch_lookup_with_small_i8(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[X:%.*]], 5
-; CHECK-NEXT:    [[TMP0:%.*]] = icmp ult i64 [[REM]], 3
-; CHECK-NEXT:    [[SWITCH_CAST:%.*]] = trunc i64 [[REM]] to i24
-; CHECK-NEXT:    [[SWITCH_SHIFTAMT:%.*]] = mul nuw nsw i24 [[SWITCH_CAST]], 8
-; CHECK-NEXT:    [[SWITCH_DOWNSHIFT:%.*]] = lshr i24 460303, [[SWITCH_SHIFTAMT]]
-; CHECK-NEXT:    [[SWITCH_MASKED:%.*]] = trunc i24 [[SWITCH_DOWNSHIFT]] to i8
-; CHECK-NEXT:    [[TMP1:%.*]] = select i1 [[TMP0]], i8 [[SWITCH_MASKED]], i8 0
-; CHECK-NEXT:    ret i8 [[TMP1]]
+; CHECK-NEXT:    [[SWITCH_CAST:%.*]] = trunc i64 [[REM]] to i40
+; CHECK-NEXT:    [[SWITCH_SHIFTAMT:%.*]] = mul nuw nsw i40 [[SWITCH_CAST]], 8
+; CHECK-NEXT:    [[SWITCH_DOWNSHIFT:%.*]] = lshr i40 460303, [[SWITCH_SHIFTAMT]]
+; CHECK-NEXT:    [[SWITCH_MASKED:%.*]] = trunc i40 [[SWITCH_DOWNSHIFT]] to i8
+; CHECK-NEXT:    ret i8 [[SWITCH_MASKED]]
 ;
 entry:
   %rem = urem i64 %x, 5
@@ -107,3 +105,71 @@ lor.end:
   %0 = phi i8 [ 15, %sw.bb0 ], [ 6, %sw.bb1 ], [ 7, %sw.bb2 ], [ 0, %default ]
   ret i8 %0
 }
+
+; Negative test: The default branch is unreachable, also it has no result.
+define i1 @switch_lookup_with_small_i1_default_unreachable(i32 %x) {
+; CHECK-LABEL: @switch_lookup_with_small_i1_default_unreachable(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[AND:%.*]] = and i32 [[X:%.*]], 15
+; CHECK-NEXT:    ret i1 false
+;
+entry:
+  %and = and i32 %x, 15
+  switch i32 %and, label %default [
+  i32 4, label %phi.end
+  i32 2, label %phi.end
+  i32 10, label %phi.end
+  i32 9, label %phi.end
+  i32 1, label %sw.bb1.i
+  i32 3, label %sw.bb1.i
+  i32 5, label %sw.bb1.i
+  i32 0, label %sw.bb1.i
+  i32 6, label %sw.bb1.i
+  i32 7, label %sw.bb1.i
+  i32 8, label %sw.bb1.i
+  ]
+
+sw.bb1.i:                                     ; preds = %entry, %entry, %entry, %entry, %entry, %entry, %entry
+  br label %phi.end
+
+default:                                      ; preds = %entry
+  unreachable
+
+phi.end: ; preds = %sw.bb1.i, %entry, %entry, %entry, %entry
+  %retval = phi i1 [ false, %sw.bb1.i ], [ false, %entry ], [ false, %entry ], [ false, %entry ], [ false, %entry ]
+  ret i1 %retval
+}
+
+; Negative test: The result in default reachable, but its value is not const.
+define i1 @switch_lookup_with_small_i1_default_nonconst(i64 %x) {
+; CHECK-LABEL: @switch_lookup_with_small_i1_default_nonconst(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[AND:%.*]] = and i64 [[X:%.*]], 15
+; CHECK-NEXT:    switch i64 [[AND]], label [[DEFAULT:%.*]] [
+; CHECK-NEXT:    i64 10, label [[LOR_END:%.*]]
+; CHECK-NEXT:    i64 1, label [[LOR_END]]
+; CHECK-NEXT:    i64 2, label [[LOR_END]]
+; CHECK-NEXT:    ]
+; CHECK:       default:
+; CHECK-NEXT:    [[CALL:%.*]] = tail call i1 @foo()
+; CHECK-NEXT:    br label [[LOR_END]]
+; CHECK:       lor.end:
+; CHECK-NEXT:    [[TMP0:%.*]] = phi i1 [ true, [[ENTRY:%.*]] ], [ [[CALL]], [[DEFAULT]] ], [ true, [[ENTRY]] ], [ true, [[ENTRY]] ]
+; CHECK-NEXT:    ret i1 [[TMP0]]
+;
+entry:
+  %and = and i64 %x, 15
+  switch i64 %and, label %default [
+  i64 10, label %lor.end
+  i64 1, label %lor.end
+  i64 2, label %lor.end
+  ]
+
+default:                                          ; preds = %entry
+  %call = tail call i1 @foo()
+  br label %lor.end
+
+lor.end:                                          ; preds = %entry, %entry, %entry, %default
+  %0 = phi i1 [ true, %entry ], [ %call, %default ], [ true, %entry ], [ true, %entry ]
+  ret i1 %0
+}