From e86a303dd4e191f7b60cb93ab08fb846b3f295a9 Mon Sep 17 00:00:00 2001 From: zhongyunde 00443407 Date: Fri, 1 Sep 2023 23:05:45 -0400 Subject: [PATCH 1/2] [SimplifyCFG] Delete the unnecessary range check for small mask operation When the small mask value little than 64, we can eliminate the checking for upper limit of the range by enlarge the lookup table size to the maximum index value. (Then the final table size grows to the next pow2 value) ``` bool f(unsigned x) { switch (x % 8) { case 0: return 1; case 1: return 0; case 2: return 0; case 3: return 1; case 4: return 1; case 5: return 0; case 6: return 1; // This would remove the range check: case 7: return 0; } return 0; } ``` Use WouldFitInRegister instead of fitsInLegalInteger to support more result type beside bool. Fixes https://github.com/llvm/llvm-project/issues/65120 --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 22 +++++++++++++++-- .../Transforms/SimplifyCFG/switch_mask.ll | 24 ++++++++----------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 5ef3a5292af54..14da5bd126438 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,25 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder, BranchInst *RangeCheckBranch = nullptr; + // Grow the table to cover all possible index values to avoid the range check. + if (UseSwitchConditionAsTableIndex) { + 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 when 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 8c97a0660d070..123519bc69211 100644 --- a/llvm/test/Transforms/SimplifyCFG/switch_mask.ll +++ b/llvm/test/Transforms/SimplifyCFG/switch_mask.ll @@ -8,13 +8,11 @@ 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 +35,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 From 64be3823864ce9286c74e09b12b1f18a1fcbb1a3 Mon Sep 17 00:00:00 2001 From: zhongyunde 00443407 Date: Sat, 28 Oct 2023 02:52:19 -0400 Subject: [PATCH 2/2] [SimplifyCFG] Fix the compile crash when default branch is unreachable This modification will squash to the previous commit if accepted. Seperate it just to make it more clearly to review. --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 6 +- .../Transforms/SimplifyCFG/switch_mask.ll | 70 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 14da5bd126438..b58bcd35f940f 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -6631,7 +6631,9 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder, BranchInst *RangeCheckBranch = nullptr; // Grow the table to cover all possible index values to avoid the range check. - if (UseSwitchConditionAsTableIndex) { + // 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. @@ -6641,7 +6643,7 @@ static bool SwitchToLookupTable(SwitchInst *SI, IRBuilder<> &Builder, return SwitchLookupTable::WouldFitInRegister( DL, CR.getUpper().getLimitedValue(), KV.second /* ResultType */); })) { - // The default branch is unreachable when we enlarge the lookup table. + // The default branch is unreachable after we enlarge the lookup table. // Adjust DefaultIsReachable to reuse code path. TableSize = CR.getUpper().getZExtValue(); DefaultIsReachable = false; diff --git a/llvm/test/Transforms/SimplifyCFG/switch_mask.ll b/llvm/test/Transforms/SimplifyCFG/switch_mask.ll index 123519bc69211..53ee21eae223c 100644 --- a/llvm/test/Transforms/SimplifyCFG/switch_mask.ll +++ b/llvm/test/Transforms/SimplifyCFG/switch_mask.ll @@ -3,6 +3,8 @@ 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( @@ -103,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 +}