From 8afe1338e5a9e0832761c93a2b630783b578973d Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 7 Nov 2023 10:48:19 +0100 Subject: [PATCH] Revert "Reland [SimplifyCFG] Delete the unnecessary range check for small mask operation (#70542)" This caused https://github.com/llvm/llvm-project/issues/71329 > 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 This reverts commit 7c4180a36a905b7ed46c09df77af1b65e356f92a. --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 24 +---- .../Transforms/SimplifyCFG/switch_mask.ll | 94 +++---------------- 2 files changed, 16 insertions(+), 102 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index b58bcd35f940..5ef3a5292af5 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -6598,8 +6598,9 @@ 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. - bool DefaultIsReachable = + const bool DefaultIsReachable = !isa(SI->getDefaultDest()->getFirstNonPHIOrDbg()); + const bool GeneratingCoveredLookupTable = (MaxTableSize == TableSize); // Create the BB that does the lookups. Module &Mod = *CommonDest->getParent()->getParent(); @@ -6630,27 +6631,6 @@ 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 53ee21eae223..8c97a0660d07 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: [[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]] +; 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]] ; entry: %and = and i64 %x, 15 @@ -37,11 +37,13 @@ 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: [[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]] +; 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]] ; entry: %rem = urem i64 %x, 5 @@ -105,71 +107,3 @@ 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 -}