Skip to content

Conversation

@madhur13490
Copy link
Contributor

The option is useful for some of the experiments we're doing. I don't see any harm in having such option and the ability to disable the pass.

The option is useful for some of the experiments we're doing.
I don't see any harm in having such option and the ability to disable
the pass.
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Madhur Amilkanthwar (madhur13490)

Changes

The option is useful for some of the experiments we're doing. I don't see any harm in having such option and the ability to disable the pass.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp (+7)
  • (added) llvm/test/Transforms/SimplifyCFG/disable-simplify-cfg.ll (+13)
diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
index 4e437e9abeb43..74e4dd2bd991a 100644
--- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
+++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -85,6 +85,10 @@ static cl::opt<bool> UserSpeculateUnpredictables(
     "speculate-unpredictables", cl::Hidden, cl::init(false),
     cl::desc("Speculate unpredictable branches (default = false)"));
 
+static cl::opt<bool> DisableSimplifyCFG(
+    "disable-simplify-cfg", cl::Hidden, cl::init(false),
+    cl::desc("Disable simplify cfg"));
+
 STATISTIC(NumSimpl, "Number of blocks simplified");
 
 static bool
@@ -307,6 +311,9 @@ static bool simplifyFunctionCFG(Function &F, const TargetTransformInfo &TTI,
           (DT && DT->verify(DominatorTree::VerificationLevel::Full))) &&
          "Original domtree is invalid?");
 
+  if (DisableSimplifyCFG)
+    return false;
+
   bool Changed = simplifyFunctionCFGImpl(F, TTI, DT, Options);
 
   assert((!RequireAndPreserveDomTree ||
diff --git a/llvm/test/Transforms/SimplifyCFG/disable-simplify-cfg.ll b/llvm/test/Transforms/SimplifyCFG/disable-simplify-cfg.ll
new file mode 100644
index 0000000000000..f5cef9314e283
--- /dev/null
+++ b/llvm/test/Transforms/SimplifyCFG/disable-simplify-cfg.ll
@@ -0,0 +1,13 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt < %s -passes=simplifycfg -disable-simplify-cfg=true -S | FileCheck %s
+
+define void @test1() {
+; CHECK-LABEL: define void @test1() {
+; CHECK-NEXT:    br label %[[BB1:.*]]
+; CHECK:       [[BB1]]:
+; CHECK-NEXT:    ret void
+;
+  br label %1
+  ret void
+}
+

@github-actions
Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 418e07b7e679246a3a4ab9a5a8c119eb4ba4623d 664881c9ee2704e6b3a322b75409eefe686435b2 --extensions cpp -- llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
View the diff from clang-format here.
diff --git a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
index 74e4dd2bd9..01e73a294f 100644
--- a/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
+++ b/llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
@@ -85,9 +85,9 @@ static cl::opt<bool> UserSpeculateUnpredictables(
     "speculate-unpredictables", cl::Hidden, cl::init(false),
     cl::desc("Speculate unpredictable branches (default = false)"));
 
-static cl::opt<bool> DisableSimplifyCFG(
-    "disable-simplify-cfg", cl::Hidden, cl::init(false),
-    cl::desc("Disable simplify cfg"));
+static cl::opt<bool> DisableSimplifyCFG("disable-simplify-cfg", cl::Hidden,
+                                        cl::init(false),
+                                        cl::desc("Disable simplify cfg"));
 
 STATISTIC(NumSimpl, "Number of blocks simplified");
 

@nikic
Copy link
Contributor

nikic commented Mar 12, 2025

The option is useful for some of the experiments we're doing. I don't see any harm in having such option and the ability to disable the pass.

Could you please provide some more details what this is useful for?

@madhur13490
Copy link
Contributor Author

The option is useful for some of the experiments we're doing. I don't see any harm in having such option and the ability to disable the pass.

Could you please provide some more details what this is useful for?

This pass aggressively converts if-else to selects. Disabling the pass and thus select generation for some of the internal benchmarks would help us narrow down the issues.

@madhur13490
Copy link
Contributor Author

@nikic Gentle ping!

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.

I don't think adding an option to disable simplifycfg makes a lot of sense. This is a core pass that does a lot more than just if conversion.

This seems like something that should be kept downstream for experimentation purposes.

@madhur13490
Copy link
Contributor Author

@nikic Thanks. I haven't looked in the code enough but do you think disabling just the if-conversion is possible?

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