From 7c4180a36a905b7ed46c09df77af1b65e356f92a Mon Sep 17 00:00:00 2001 From: Allen Date: Fri, 3 Nov 2023 09:12:29 +0800 Subject: [PATCH] Reland [SimplifyCFG] Delete the unnecessary range check for small mask operation (#70542) Fix the compile crash when the default result has no result for https://github.com/llvm/llvm-project/pull/65835 Fixes https://github.com/llvm/llvm-project/issues/65120 Reviewed By: zmodem, nikic --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 24 ++++- .../Transforms/SimplifyCFG/switch_mask.ll | 94 ++++++++++++++++--- 2 files changed, 102 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 5ef3a5292af5..b58bcd35f940 100644 --- 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(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 index 8c97a0660d07..53ee21eae223 100644 --- 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 +}