Skip to content

[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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

antoniofrighetto
Copy link
Contributor

It may be desirable not to carry out simplifySwitchOfPowersOfTwo transform during early pipeline invocations, so as to let other optimizations occur first.

@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Antonio Frighetto (antoniofrighetto)

Changes

It may be desirable not to carry out simplifySwitchOfPowersOfTwo transform during early pipeline invocations, so as to let other optimizations occur first.


Full diff: https://github.com/llvm/llvm-project/pull/145477.diff

7 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h (+5)
  • (modified) llvm/lib/Passes/PassBuilder.cpp (+2)
  • (modified) llvm/lib/Passes/PassBuilderPipelines.cpp (+5-2)
  • (modified) llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp (+2-1)
  • (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+2-1)
  • (modified) llvm/test/Other/new-pm-print-pipeline.ll (+2-2)
  • (modified) llvm/test/Transforms/SimplifyCFG/X86/switch-of-powers-of-two.ll (+59-8)
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 [

Copy link
Contributor

@nikic nikic left a 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.

@antoniofrighetto
Copy link
Contributor Author

Can you please add a PhaseOrdering test that demonstrates why this is useful?

To simply confirm this, as a PhaseOrdering test, do you have in mind a reduced version of _ZN8zvariant19framing_offset_size17FramingOffsetSize12write_offset17h7af96b41ab784441E (bench/zed-rs/original/8bnapxt4ilkd5y3egr7fzm1sv.ll, where the regression appears), and test it with default O2, correct?

@nikic
Copy link
Contributor

nikic commented Jun 24, 2025

Can you please add a PhaseOrdering test that demonstrates why this is useful?

To simply confirm this, as a PhaseOrdering test, do you have in mind a reduced version of _ZN8zvariant19framing_offset_size17FramingOffsetSize12write_offset17h7af96b41ab784441E (bench/zed-rs/original/8bnapxt4ilkd5y3egr7fzm1sv.ll, where the regression appears), and test it with default O2, correct?

Yes

…ations

It may be desirable not to carry out `simplifySwitchOfPowersOfTwo`
transform during early pipeline invocations, so as to let other
optimizations occur first.
@antoniofrighetto antoniofrighetto force-pushed the feature/switch-pow2-optional branch from 17d7484 to 319ccfb Compare June 24, 2025 14:16
@antoniofrighetto
Copy link
Contributor Author

Added, thanks (not exactly testing the original regression, but still managed to show an improvement w/ ConvertSwitchToLookupTable on).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants