-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[SimplifyCFG] Do not run simplifySwitchOfPowersOfTwo
in early invocations
#145477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[SimplifyCFG] Do not run simplifySwitchOfPowersOfTwo
in early invocations
#145477
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Antonio Frighetto (antoniofrighetto) ChangesIt may be desirable not to carry out Full diff: https://github.com/llvm/llvm-project/pull/145477.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h b/llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h
index ee3cc950cdb50..376c5ab934244 100644
--- a/llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h
+++ b/llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h
@@ -32,6 +32,7 @@ struct SimplifyCFGOptions {
bool SimplifyCondBranch = true;
bool SpeculateBlocks = true;
bool SpeculateUnpredictables = false;
+ bool SwitchToCttzAndReducedTable = true;
AssumptionCache *AC = nullptr;
@@ -85,6 +86,10 @@ struct SimplifyCFGOptions {
SpeculateUnpredictables = B;
return *this;
}
+ SimplifyCFGOptions &convertSwitchToCttzAndReducedTable(bool B) {
+ SwitchToCttzAndReducedTable = B;
+ return *this;
+ }
};
} // namespace llvm
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 4603eaff8ade9..8e149b73be992 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1001,6 +1001,8 @@ Expected<SimplifyCFGOptions> parseSimplifyCFGOptions(StringRef Params) {
Result.sinkCommonInsts(Enable);
} else if (ParamName == "speculate-unpredictables") {
Result.speculateUnpredictables(Enable);
+ } else if (ParamName == "switch-to-cttz") {
+ Result.convertSwitchToCttzAndReducedTable(Enable);
} else if (Enable && ParamName.consume_front("bonus-inst-threshold=")) {
APInt BonusInstThreshold;
if (ParamName.getAsInteger(0, BonusInstThreshold))
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index c83d2dc1f1514..087987385314a 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -1121,7 +1121,8 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
// Compare/branch metadata may alter the behavior of passes like
// SimplifyCFG.
EarlyFPM.addPass(LowerExpectIntrinsicPass());
- EarlyFPM.addPass(SimplifyCFGPass());
+ EarlyFPM.addPass(SimplifyCFGPass(
+ SimplifyCFGOptions().convertSwitchToCttzAndReducedTable(false)));
EarlyFPM.addPass(SROAPass(SROAOptions::ModifyCFG));
EarlyFPM.addPass(EarlyCSEPass());
if (Level == OptimizationLevel::O3)
@@ -1190,7 +1191,9 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
GlobalCleanupPM.addPass(InstCombinePass());
invokePeepholeEPCallbacks(GlobalCleanupPM, Level);
GlobalCleanupPM.addPass(
- SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(true)));
+ SimplifyCFGPass(SimplifyCFGOptions()
+ .convertSwitchRangeToICmp(true)
+ .convertSwitchToCttzAndReducedTable(false)));
MPM.addPass(createModuleToFunctionPassAdaptor(std::move(GlobalCleanupPM),
PTO.EagerlyInvalidateAnalyses));
diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
index a09303bb4469f..4d56993cbd39f 100644
--- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
+++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -366,7 +366,8 @@ void SimplifyCFGPass::printPipeline(
OS << (Options.SpeculateBlocks ? "" : "no-") << "speculate-blocks;";
OS << (Options.SimplifyCondBranch ? "" : "no-") << "simplify-cond-branch;";
OS << (Options.SpeculateUnpredictables ? "" : "no-")
- << "speculate-unpredictables";
+ << "speculate-unpredictables;";
+ OS << (Options.SwitchToCttzAndReducedTable ? "" : "no-") << "switch-to-cttz";
OS << '>';
}
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index e205551658aa5..decce248063ac 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -7604,7 +7604,8 @@ bool SimplifyCFGOpt::simplifySwitch(SwitchInst *SI, IRBuilder<> &Builder) {
switchToLookupTable(SI, Builder, DTU, DL, TTI))
return requestResimplify();
- if (simplifySwitchOfPowersOfTwo(SI, Builder, DL, TTI))
+ if (Options.SwitchToCttzAndReducedTable &&
+ simplifySwitchOfPowersOfTwo(SI, Builder, DL, TTI))
return requestResimplify();
if (reduceSwitchRange(SI, Builder, DL, TTI))
diff --git a/llvm/test/Other/new-pm-print-pipeline.ll b/llvm/test/Other/new-pm-print-pipeline.ll
index db398d68fd426..8e4efafbc9cb2 100644
--- a/llvm/test/Other/new-pm-print-pipeline.ll
+++ b/llvm/test/Other/new-pm-print-pipeline.ll
@@ -49,8 +49,8 @@
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(print<stack-lifetime><may>,print<stack-lifetime><must>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-17
; CHECK-17: function(print<stack-lifetime><may>,print<stack-lifetime><must>)
-; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;switch-to-lookup;keep-loops;hoist-common-insts;hoist-loads-stores-with-cond-faulting;sink-common-insts;speculate-blocks;simplify-cond-branch;speculate-unpredictables>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-hoist-loads-stores-with-cond-faulting;no-sink-common-insts;no-speculate-blocks;no-simplify-cond-branch;no-speculate-unpredictables>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-18
-; CHECK-18: function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;no-switch-range-to-icmp;switch-to-lookup;keep-loops;hoist-common-insts;hoist-loads-stores-with-cond-faulting;sink-common-insts;speculate-blocks;simplify-cond-branch;speculate-unpredictables>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-range-to-icmp;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-hoist-loads-stores-with-cond-faulting;no-sink-common-insts;no-speculate-blocks;no-simplify-cond-branch;no-speculate-unpredictables>)
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;switch-to-lookup;keep-loops;hoist-common-insts;hoist-loads-stores-with-cond-faulting;sink-common-insts;speculate-blocks;simplify-cond-branch;speculate-unpredictables;switch-to-cttz>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-hoist-loads-stores-with-cond-faulting;no-sink-common-insts;no-speculate-blocks;no-simplify-cond-branch;no-speculate-unpredictables;no-switch-to-cttz>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-18
+; CHECK-18: function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;no-switch-range-to-icmp;switch-to-lookup;keep-loops;hoist-common-insts;hoist-loads-stores-with-cond-faulting;sink-common-insts;speculate-blocks;simplify-cond-branch;speculate-unpredictables;switch-to-cttz>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-range-to-icmp;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-hoist-loads-stores-with-cond-faulting;no-sink-common-insts;no-speculate-blocks;no-simplify-cond-branch;no-speculate-unpredictables;no-switch-to-cttz>)
; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only>,loop-vectorize<interleave-forced-only;vectorize-forced-only>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-19
; CHECK-19: function(loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only;>,loop-vectorize<interleave-forced-only;vectorize-forced-only;>)
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/switch-of-powers-of-two.ll b/llvm/test/Transforms/SimplifyCFG/X86/switch-of-powers-of-two.ll
index 529826758c138..0e6ef0f5323ad 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/switch-of-powers-of-two.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/switch-of-powers-of-two.ll
@@ -1,16 +1,67 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
-; RUN: opt -passes='simplifycfg<switch-to-lookup>' -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck %s
+; RUN: opt -passes='simplifycfg<no-switch-to-cttz>' -S < %s | FileCheck --check-prefix=NOFOLD %s
+; RUN: opt -passes='simplifycfg' -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck --check-prefix=SWITCH-TO-CTTZ %s
+; RUN: opt -passes='simplifycfg<switch-to-lookup>' -simplifycfg-require-and-preserve-domtree=1 -S < %s | FileCheck --check-prefix=SWITCH-TO-CTTZ-AND-TO-LOOKUP %s
target triple = "x86_64-unknown-linux-gnu"
define i32 @switch_of_powers_two(i32 %arg) {
-; CHECK-LABEL: define i32 @switch_of_powers_two(
-; CHECK-SAME: i32 [[ARG:%.*]]) {
-; CHECK-NEXT: [[ENTRY:.*:]]
-; CHECK-NEXT: [[TMP0:%.*]] = call i32 @llvm.cttz.i32(i32 [[ARG]], i1 true)
-; CHECK-NEXT: [[SWITCH_GEP:%.*]] = getelementptr inbounds [7 x i32], ptr @switch.table.switch_of_powers_two, i32 0, i32 [[TMP0]]
-; CHECK-NEXT: [[SWITCH_LOAD:%.*]] = load i32, ptr [[SWITCH_GEP]], align 4
-; CHECK-NEXT: ret i32 [[SWITCH_LOAD]]
+; NOFOLD-LABEL: define i32 @switch_of_powers_two(
+; NOFOLD-SAME: i32 [[ARG:%.*]]) {
+; NOFOLD-NEXT: [[ENTRY:.*]]:
+; NOFOLD-NEXT: switch i32 [[ARG]], label %[[DEFAULT_CASE:.*]] [
+; NOFOLD-NEXT: i32 1, label %[[RETURN:.*]]
+; NOFOLD-NEXT: i32 8, label %[[BB2:.*]]
+; NOFOLD-NEXT: i32 16, label %[[BB3:.*]]
+; NOFOLD-NEXT: i32 32, label %[[BB4:.*]]
+; NOFOLD-NEXT: i32 64, label %[[BB5:.*]]
+; NOFOLD-NEXT: ]
+; NOFOLD: [[DEFAULT_CASE]]:
+; NOFOLD-NEXT: unreachable
+; NOFOLD: [[BB2]]:
+; NOFOLD-NEXT: br label %[[RETURN]]
+; NOFOLD: [[BB3]]:
+; NOFOLD-NEXT: br label %[[RETURN]]
+; NOFOLD: [[BB4]]:
+; NOFOLD-NEXT: br label %[[RETURN]]
+; NOFOLD: [[BB5]]:
+; NOFOLD-NEXT: br label %[[RETURN]]
+; NOFOLD: [[RETURN]]:
+; NOFOLD-NEXT: [[PHI:%.*]] = phi i32 [ 2, %[[BB2]] ], [ 1, %[[BB3]] ], [ 0, %[[BB4]] ], [ 42, %[[BB5]] ], [ 3, %[[ENTRY]] ]
+; NOFOLD-NEXT: ret i32 [[PHI]]
+;
+; SWITCH-TO-CTTZ-LABEL: define i32 @switch_of_powers_two(
+; SWITCH-TO-CTTZ-SAME: i32 [[ARG:%.*]]) {
+; SWITCH-TO-CTTZ-NEXT: [[ENTRY:.*]]:
+; SWITCH-TO-CTTZ-NEXT: [[TMP0:%.*]] = call i32 @llvm.cttz.i32(i32 [[ARG]], i1 true)
+; SWITCH-TO-CTTZ-NEXT: switch i32 [[TMP0]], label %[[DEFAULT_CASE:.*]] [
+; SWITCH-TO-CTTZ-NEXT: i32 0, label %[[RETURN:.*]]
+; SWITCH-TO-CTTZ-NEXT: i32 3, label %[[BB2:.*]]
+; SWITCH-TO-CTTZ-NEXT: i32 4, label %[[BB3:.*]]
+; SWITCH-TO-CTTZ-NEXT: i32 5, label %[[BB4:.*]]
+; SWITCH-TO-CTTZ-NEXT: i32 6, label %[[BB5:.*]]
+; SWITCH-TO-CTTZ-NEXT: ]
+; SWITCH-TO-CTTZ: [[DEFAULT_CASE]]:
+; SWITCH-TO-CTTZ-NEXT: unreachable
+; SWITCH-TO-CTTZ: [[BB2]]:
+; SWITCH-TO-CTTZ-NEXT: br label %[[RETURN]]
+; SWITCH-TO-CTTZ: [[BB3]]:
+; SWITCH-TO-CTTZ-NEXT: br label %[[RETURN]]
+; SWITCH-TO-CTTZ: [[BB4]]:
+; SWITCH-TO-CTTZ-NEXT: br label %[[RETURN]]
+; SWITCH-TO-CTTZ: [[BB5]]:
+; SWITCH-TO-CTTZ-NEXT: br label %[[RETURN]]
+; SWITCH-TO-CTTZ: [[RETURN]]:
+; SWITCH-TO-CTTZ-NEXT: [[PHI:%.*]] = phi i32 [ 2, %[[BB2]] ], [ 1, %[[BB3]] ], [ 0, %[[BB4]] ], [ 42, %[[BB5]] ], [ 3, %[[ENTRY]] ]
+; SWITCH-TO-CTTZ-NEXT: ret i32 [[PHI]]
+;
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-LABEL: define i32 @switch_of_powers_two(
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-SAME: i32 [[ARG:%.*]]) {
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-NEXT: [[ENTRY:.*:]]
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-NEXT: [[TMP0:%.*]] = call i32 @llvm.cttz.i32(i32 [[ARG]], i1 true)
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-NEXT: [[SWITCH_GEP:%.*]] = getelementptr inbounds [7 x i32], ptr @switch.table.switch_of_powers_two, i32 0, i32 [[TMP0]]
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-NEXT: [[SWITCH_LOAD:%.*]] = load i32, ptr [[SWITCH_GEP]], align 4
+; SWITCH-TO-CTTZ-AND-TO-LOOKUP-NEXT: ret i32 [[SWITCH_LOAD]]
;
entry:
switch i32 %arg, label %default_case [
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a PhaseOrdering test that demonstrates why this is useful?
Also, can we reasonably bind this to one of the existing switch options (like ConvertSwitchToLookupTable)? We have sooo many options already.
To simply confirm this, as a PhaseOrdering test, do you have in mind a reduced version of |
Yes |
…ations It may be desirable not to carry out `simplifySwitchOfPowersOfTwo` transform during early pipeline invocations, so as to let other optimizations occur first.
17d7484
to
319ccfb
Compare
Added, thanks (not exactly testing the original regression, but still managed to show an improvement w/ ConvertSwitchToLookupTable on). |
It may be desirable not to carry out
simplifySwitchOfPowersOfTwo
transform during early pipeline invocations, so as to let other optimizations occur first.