Skip to content

Conversation

@mtrofin
Copy link
Member

@mtrofin mtrofin commented Oct 31, 2025

PR #161000 introduced a bug whereby the IR would become invalid by having an unconditional branch have !prof​attached to it. This only became evident in PR #165744, because the IR of test/Transforms/SimplifyCFG/pr165301.ll​was simple enough to both (1) introduce the unconditional branch, and (2) survive in that fashion until the end of the pass (simplifycfg) and thus trip the verifier.

Copy link
Member Author

mtrofin commented Oct 31, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mtrofin mtrofin force-pushed the users/mtrofin/10-31-_simplifycfg_don_t_propagate_weights_to_unconditional_branches_in_turnswitchrangeintoicmp_ branch from d87a30b to 8a24df8 Compare October 31, 2025 22:35
@mtrofin mtrofin marked this pull request as ready for review October 31, 2025 22:38
@llvmbot
Copy link
Member

llvmbot commented Oct 31, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Mircea Trofin (mtrofin)

Changes

PR #161000 introduced a bug whereby the IR would become invalid by having an unconditional branch have !prof​attached to it. This only became evident in PR #165744, because the IR of test/Transforms/SimplifyCFG/pr165301.ll​was simple enough to both (1) introduce the unconditional branch, and (2) survive in that fashion until the end of the pass (simplifycfg) and thus trip the verifier.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+1-1)
  • (modified) llvm/test/Transforms/SimplifyCFG/pr165301.ll (+9-4)
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 6addcfab15125..cbc604e87cf1a 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5956,7 +5956,7 @@ bool SimplifyCFGOpt::turnSwitchRangeIntoICmp(SwitchInst *SI,
   }
 
   // Update weight for the newly-created conditional branch.
-  if (hasBranchWeightMD(*SI)) {
+  if (hasBranchWeightMD(*SI) && NewBI->isConditional()) {
     SmallVector<uint64_t, 8> Weights;
     getBranchWeights(SI, Weights);
     if (Weights.size() == 1 + SI->getNumCases()) {
diff --git a/llvm/test/Transforms/SimplifyCFG/pr165301.ll b/llvm/test/Transforms/SimplifyCFG/pr165301.ll
index 4a539d77af3cb..1df655250f57e 100644
--- a/llvm/test/Transforms/SimplifyCFG/pr165301.ll
+++ b/llvm/test/Transforms/SimplifyCFG/pr165301.ll
@@ -1,11 +1,11 @@
-; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-globals all --version 6
 ; RUN: opt -S -passes="simplifycfg<switch-range-to-icmp>" < %s | FileCheck %s
 
 ; Make sure there's no use after free when removing incoming values from PHI nodes
 
-define i32 @pr165301(i1 %cond) {
+define i32 @pr165301(i1 %cond) !prof !0 {
 ; CHECK-LABEL: define i32 @pr165301(
-; CHECK-SAME: i1 [[COND:%.*]]) {
+; CHECK-SAME: i1 [[COND:%.*]]) !prof [[PROF0:![0-9]+]] {
 ; CHECK-NEXT:  [[ENTRY:.*:]]
 ; CHECK-NEXT:    br label %[[SWITCHBB:.*]]
 ; CHECK:       [[SWITCHBB]]:
@@ -18,9 +18,14 @@ switchbb:
   switch i1 %cond, label %default [
   i1 false, label %switchbb
   i1 true, label %switchbb
-  ]
+  ], !prof !1
 
 default:
   %phi.lcssa = phi i32 [ 0, %switchbb ]
   ret i32 %phi.lcssa
 }
+!0 = !{!"function_entry_count", i32 10}
+!1 = !{!"branch_weights", i32 2, i32 3, i32 5}
+;.
+; CHECK: [[PROF0]] = !{!"function_entry_count", i32 10}
+;.

…gate_weights_to_unconditional_branches_in_turnswitchrangeintoicmp_
@mtrofin mtrofin enabled auto-merge (squash) October 31, 2025 22:41
mtrofin added a commit that referenced this pull request Oct 31, 2025
A remaining failing one, under SimplifyCFG (which is pass that we did fix) is covered in #165931
mtrofin added a commit that referenced this pull request Oct 31, 2025
A remaining failing one, under SimplifyCFG (which is pass that we did fix) is covered in #165931
@mtrofin mtrofin merged commit 6adef40 into main Oct 31, 2025
7 of 9 checks passed
@mtrofin mtrofin deleted the users/mtrofin/10-31-_simplifycfg_don_t_propagate_weights_to_unconditional_branches_in_turnswitchrangeintoicmp_ branch October 31, 2025 23:11
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
…5933)

A remaining failing one, under SimplifyCFG (which is pass that we did fix) is covered in llvm#165931
DEBADRIBASAK pushed a commit to DEBADRIBASAK/llvm-project that referenced this pull request Nov 3, 2025
…urnSwitchRangeIntoICmp` (llvm#165931)

PR llvm#161000 introduced a bug whereby the IR would become invalid by having an unconditional branch have `!prof`​attached to it. This only became evident in PR llvm#165744, because the IR of `test/Transforms/SimplifyCFG/pr165301.ll`​was simple enough to both (1) introduce the unconditional branch, and (2) survive in that fashion until the end of the pass (simplifycfg) and thus trip the verifier.
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