[llvm-branch-commits] [SimplifyCFG] Fix profcheck failure from #213302 (PR #220811)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Sep 2 22:20:30 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Aiden Grossman (boomanaiden154)
<details>
<summary>Changes</summary>
This patch fixes the profcheck failure from that patch by explicitly
updating the branch weights given we can infer what the values should
be.
---
Full diff: https://github.com/llvm/llvm-project/pull/220811.diff
2 Files Affected:
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+29-2)
- (modified) llvm/test/Transforms/SimplifyCFG/switch-select-remap.ll (+19-50)
``````````diff
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 2a3a88713920b..6f4200a381f6c 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -5098,8 +5098,6 @@ bool SimplifyCFGOpt::simplifySwitchOnSelectRemap(SwitchInst *SI,
BasicBlock *BB = SI->getParent();
if (OldDest != DestFork) {
- // Case list is changing so we should drop stale profile weights.
- SI->setMetadata(LLVMContext::MD_prof, nullptr);
if (!IsDefault)
OldDest->removePredecessor(BB);
if (IsDefault)
@@ -5117,6 +5115,35 @@ bool SimplifyCFGOpt::simplifySwitchOnSelectRemap(SwitchInst *SI,
if (DTU && !OldDestStillTargeted)
DTU->applyUpdates({{DominatorTree::Delete, BB, OldDest}});
}
+
+ // Update the profile information on the switch ifwe had a profile
+ // for both it and the select instruction. We only need to do this
+ // in the case where we add a case to the switch.
+ if (IsDefault) {
+ SmallVector<uint32_t> SwitchWeights;
+ bool SwitchHasBranchWeights = extractBranchWeights(*SI, SwitchWeights);
+ SmallVector<uint32_t> SelectWeights;
+ bool SelectHasBranchWeights =
+ extractBranchWeights(*Select, SelectWeights);
+ if (SwitchHasBranchWeights && SelectHasBranchWeights &&
+ !ProfcheckDisableMetadataFixes) {
+ // Update the branch weights of the switch by multiplying all of them by
+ // the weight of the false branch of the select. Then add the new switch
+ // case at the end by multiplying the weight of the true branch of the
+ // select by the total weight of the switch.
+ uint64_t SwitchTotalWeight = sum_of(SwitchWeights, uint64_t{0});
+ SmallVector<uint64_t> NewSwitchWeights;
+ NewSwitchWeights.reserve(SwitchWeights.size() + 1);
+ for (uint32_t SwitchWeight : SwitchWeights)
+ NewSwitchWeights.push_back(SwitchWeight * SelectWeights[1]);
+ NewSwitchWeights.push_back(SwitchTotalWeight * SelectWeights[0]);
+ setFittedBranchWeights(*SI, NewSwitchWeights, false);
+ } else if (SwitchHasBranchWeights) {
+ // If we only have branch weights on the switch, we cannot reconstruct
+ // branch weights correctly, so mark them as unknown.
+ setExplicitlyUnknownBranchWeightsIfProfiled(*SI, DEBUG_TYPE);
+ }
+ }
}
// X replaces the condition so compare/select are now dead.
diff --git a/llvm/test/Transforms/SimplifyCFG/switch-select-remap.ll b/llvm/test/Transforms/SimplifyCFG/switch-select-remap.ll
index 5b20d75ae0d16..07baa71f719b5 100644
--- a/llvm/test/Transforms/SimplifyCFG/switch-select-remap.ll
+++ b/llvm/test/Transforms/SimplifyCFG/switch-select-remap.ll
@@ -4,14 +4,14 @@
; The compared value 4 has no explicit case, and the remapped value 6 maps to
; a real (non-default) case, so switching on %x needs a new explicit case for
; 4 pointing to bb2.
-define void @test_remap_add_case(i8 %x) {
+define void @test_remap_add_case(i8 %x) !prof !0 {
; CHECK-LABEL: define void @test_remap_add_case(
-; CHECK-SAME: i8 [[X:%.*]]) {
+; CHECK-SAME: i8 [[X:%.*]]) !prof [[PROF0:![0-9]+]] {
; CHECK-NEXT: switch i8 [[X]], label %[[BB1:.*]] [
; CHECK-NEXT: i8 6, label %[[BB2:.*]]
; CHECK-NEXT: i8 10, label %[[BB3:.*]]
; CHECK-NEXT: i8 4, label %[[BB2]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF1:![0-9]+]]
; CHECK: [[BB1]]:
; CHECK-NEXT: call void @func1()
; CHECK-NEXT: unreachable
@@ -23,11 +23,11 @@ define void @test_remap_add_case(i8 %x) {
; CHECK-NEXT: unreachable
;
%cmp = icmp eq i8 %x, 4
- %key = select i1 %cmp, i8 6, i8 %x
+ %key = select i1 %cmp, i8 6, i8 %x, !prof !1
switch i8 %key, label %bb1 [
i8 6, label %bb2
i8 10, label %bb3
- ]
+ ], !prof !2
bb1:
call void @func1()
@@ -47,14 +47,14 @@ bb3:
; body are removed in the same run: this pass edits the CFG through a
; DomTreeUpdater, so a follow-up SimplifyCFG iteration cleans it up
; immediately instead of needing a separate pass.
-define void @test_remap_retarget_case(i8 %x) {
+define void @test_remap_retarget_case(i8 %x) !prof !0 {
; CHECK-LABEL: define void @test_remap_retarget_case(
-; CHECK-SAME: i8 [[X:%.*]]) {
+; CHECK-SAME: i8 [[X:%.*]]) !prof [[PROF0]] {
; CHECK-NEXT: switch i8 [[X]], label %[[BB1:.*]] [
; CHECK-NEXT: i8 4, label %[[BB2:.*]]
; CHECK-NEXT: i8 6, label %[[BB2]]
; CHECK-NEXT: i8 10, label %[[BB3:.*]]
-; CHECK-NEXT: ]
+; CHECK-NEXT: ], !prof [[PROF2:![0-9]+]]
; CHECK: [[BB1]]:
; CHECK-NEXT: call void @func1()
; CHECK-NEXT: unreachable
@@ -66,12 +66,12 @@ define void @test_remap_retarget_case(i8 %x) {
; CHECK-NEXT: unreachable
;
%cmp = icmp eq i8 %x, 4
- %key = select i1 %cmp, i8 6, i8 %x
+ %key = select i1 %cmp, i8 6, i8 %x, !prof !1
switch i8 %key, label %bb1 [
i8 4, label %bb4
i8 6, label %bb2
i8 10, label %bb3
- ]
+ ], !prof !3
bb1:
call void @func1()
@@ -295,45 +295,6 @@ default:
unreachable
}
-; The fold changes the case list (a case may be added, or retargeted to a
-; different successor), so any existing branch-weight metadata would
-; mislabel the new layout - it must be dropped rather than kept stale.
-define void @test_remap_drops_branch_weights(i8 %x) {
-; CHECK-LABEL: define void @test_remap_drops_branch_weights(
-; CHECK-SAME: i8 [[X:%.*]]) {
-; CHECK-NEXT: switch i8 [[X]], label %[[BB1:.*]] [
-; CHECK-NEXT: i8 6, label %[[BB2:.*]]
-; CHECK-NEXT: i8 10, label %[[BB3:.*]]
-; CHECK-NEXT: i8 4, label %[[BB2]]
-; CHECK-NEXT: ]
-; CHECK: [[BB1]]:
-; CHECK-NEXT: call void @func1()
-; CHECK-NEXT: unreachable
-; CHECK: [[BB2]]:
-; CHECK-NEXT: call void @func2()
-; CHECK-NEXT: unreachable
-; CHECK: [[BB3]]:
-; CHECK-NEXT: call void @func3()
-; CHECK-NEXT: unreachable
-;
- %cmp = icmp eq i8 %x, 4
- %key = select i1 %cmp, i8 6, i8 %x
- switch i8 %key, label %bb1 [
- i8 6, label %bb2
- i8 10, label %bb3
- ], !prof !0
-
-bb1:
- call void @func1()
- unreachable
-bb2:
- call void @func2()
- unreachable
-bb3:
- call void @func3()
- unreachable
-}
-
; Negative test: %key (the select) is used by more than just the switch, so
; folding it away wouldn't actually remove the compare/select sequence -
; leave it alone.
@@ -464,4 +425,12 @@ declare void @func3()
declare void @func4()
declare void @use(i32)
-!0 = !{!"branch_weights", i32 1, i32 2, i32 3}
+!0 = !{!"function_entry_count", i32 10}
+!1 = !{!"branch_weights", i32 2, i32 3}
+!2 = !{!"branch_weights", i32 5, i32 7, i32 11}
+!3 = !{!"branch_weights", i32 5, i32 7, i32 11, i32 13}
+;.
+; CHECK: [[PROF0]] = !{!"function_entry_count", i32 10}
+; CHECK: [[PROF1]] = !{!"branch_weights", i32 15, i32 21, i32 33, i32 46}
+; CHECK: [[PROF2]] = !{!"branch_weights", i32 5, i32 7, i32 11, i32 13}
+;.
``````````
</details>
https://github.com/llvm/llvm-project/pull/220811
More information about the llvm-branch-commits
mailing list