[IR] Handle expected tag in switch branch weights.#200025
Merged
jlebar merged 3 commits intoMay 27, 2026
Merged
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-ir Author: Justin Lebar (jlebar) ChangesSwitch branch weight metadata has an optional This bug was found by a large run of Opus 4.7 looking for bugs in LLVM. Full diff: https://github.com/llvm/llvm-project/pull/200025.diff 2 Files Affected:
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 088f85c4851cd..6282e949b15ca 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -4259,11 +4259,13 @@ void SwitchInstProfUpdateWrapper::setSuccessorWeight(
SwitchInstProfUpdateWrapper::CaseWeightOpt
SwitchInstProfUpdateWrapper::getSuccessorWeight(const SwitchInst &SI,
unsigned idx) {
- if (MDNode *ProfileData = getBranchWeightMDNode(SI))
- if (ProfileData->getNumOperands() == SI.getNumSuccessors() + 1)
- return mdconst::extract<ConstantInt>(ProfileData->getOperand(idx + 1))
+ if (MDNode *ProfileData = getBranchWeightMDNode(SI)) {
+ unsigned Offset = getBranchWeightOffset(ProfileData);
+ if (ProfileData->getNumOperands() == SI.getNumSuccessors() + Offset)
+ return mdconst::extract<ConstantInt>(ProfileData->getOperand(idx + Offset))
->getValue()
.getZExtValue();
+ }
return std::nullopt;
}
diff --git a/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch-profmd.ll b/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch-profmd.ll
index 34257ffdc3699..48c24b7455675 100644
--- a/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch-profmd.ll
+++ b/llvm/test/Transforms/SimpleLoopUnswitch/trivial-unswitch-profmd.ll
@@ -221,8 +221,151 @@ loop_exit3:
ret i32 0
}
+; Test for a switch statement with the optional "expected" tag in its
+; branch-weight metadata.
+define i32 @test5_expected(ptr %var, i32 %cond1, i32 %cond2) {
+; CHECK-LABEL: @test5_expected(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: switch i32 [[COND2:%.*]], label [[ENTRY_SPLIT:%.*]] [
+; CHECK-NEXT: i32 2, label [[LOOP_EXIT2:%.*]]
+; CHECK-NEXT: ], !prof ![[MD0]]
+; CHECK: entry.split:
+; CHECK-NEXT: br label [[LOOP_BEGIN:%.*]]
+; CHECK: loop_begin:
+; CHECK-NEXT: [[VAR_VAL:%.*]] = load i32, ptr [[VAR:%.*]]
+; CHECK-NEXT: switch i32 [[COND2]], label [[LOOP2:%.*]] [
+; CHECK-NEXT: i32 0, label [[LOOP0:%.*]]
+; CHECK-NEXT: i32 1, label [[LOOP1:%.*]]
+; CHECK-NEXT: ], !prof ![[MD1]]
+; CHECK: loop0:
+; CHECK-NEXT: call void @some_func()
+; CHECK-NEXT: br label [[LOOP_LATCH:%.*]]
+; CHECK: loop1:
+; CHECK-NEXT: call void @some_func()
+; CHECK-NEXT: br label [[LOOP_LATCH]]
+; CHECK: loop2:
+; CHECK-NEXT: call void @some_func()
+; CHECK-NEXT: br label [[LOOP_LATCH]]
+; CHECK: loop_latch:
+; CHECK-NEXT: br label [[LOOP_BEGIN]]
+; CHECK: loop_exit1:
+; CHECK-NEXT: ret i32 0
+; CHECK: loop_exit2:
+; CHECK-NEXT: ret i32 0
+; CHECK: loop_exit3:
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ br label %loop_begin
+
+loop_begin:
+ %var_val = load i32, ptr %var
+ switch i32 %cond2, label %loop2 [
+ i32 0, label %loop0
+ i32 1, label %loop1
+ i32 2, label %loop_exit2
+ ], !prof !{!"branch_weights", !"expected", i32 99, i32 100, i32 101, i32 102}
+
+loop0:
+ call void @some_func()
+ br label %loop_latch
+
+loop1:
+ call void @some_func()
+ br label %loop_latch
+
+loop2:
+ call void @some_func()
+ br label %loop_latch
+
+loop_latch:
+ br label %loop_begin
+
+loop_exit1:
+ ret i32 0
+
+loop_exit2:
+ ret i32 0
+
+loop_exit3:
+ ret i32 0
+}
+
+; Default destination is the loop exit and the "expected" origin tag is present.
+; Before the fix the default weight (300) was lost and replaced with 0.
+define i32 @test6_expected_default_exit(ptr %var, i32 %cond1, i32 %cond2) {
+; CHECK-LABEL: @test6_expected_default_exit(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: switch i32 [[COND2:%.*]], label [[LOOP_EXIT2:%.*]] [
+; CHECK-NEXT: i32 0, label [[ENTRY_SPLIT:%.*]]
+; CHECK-NEXT: i32 1, label [[ENTRY_SPLIT]]
+; CHECK-NEXT: i32 2, label [[ENTRY_SPLIT]]
+; CHECK-NEXT: ], !prof ![[MD5:[0-9]+]]
+; CHECK: entry.split:
+; CHECK-NEXT: br label [[LOOP_BEGIN:%.*]]
+; CHECK: loop_begin:
+; CHECK-NEXT: [[VAR_VAL:%.*]] = load i32, ptr [[VAR:%.*]]
+; CHECK-NEXT: switch i32 [[COND2]], label [[LOOP2:%.*]] [
+; CHECK-NEXT: i32 0, label [[LOOP0:%.*]]
+; CHECK-NEXT: i32 1, label [[LOOP1:%.*]]
+; CHECK-NEXT: ], !prof ![[MD3]]
+; CHECK: loop0:
+; CHECK-NEXT: call void @some_func()
+; CHECK-NEXT: br label [[LOOP_LATCH:%.*]]
+; CHECK: loop1:
+; CHECK-NEXT: call void @some_func()
+; CHECK-NEXT: br label [[LOOP_LATCH]]
+; CHECK: loop2:
+; CHECK-NEXT: call void @some_func()
+; CHECK-NEXT: br label [[LOOP_LATCH]]
+; CHECK: loop_latch:
+; CHECK-NEXT: br label [[LOOP_BEGIN]]
+; CHECK: loop_exit1:
+; CHECK-NEXT: ret i32 0
+; CHECK: loop_exit2:
+; CHECK-NEXT: ret i32 0
+; CHECK: loop_exit3:
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ br label %loop_begin
+
+loop_begin:
+ %var_val = load i32, ptr %var
+ switch i32 %cond2, label %loop_exit2 [
+ i32 0, label %loop0
+ i32 1, label %loop1
+ i32 2, label %loop2
+ ], !prof !{!"branch_weights", !"expected", i32 300, i32 100, i32 101, i32 102}
+
+loop0:
+ call void @some_func()
+ br label %loop_latch
+
+loop1:
+ call void @some_func()
+ br label %loop_latch
+
+loop2:
+ call void @some_func()
+ br label %loop_latch
+
+loop_latch:
+ br label %loop_begin
+
+loop_exit1:
+ ret i32 0
+
+loop_exit2:
+ ret i32 0
+
+loop_exit3:
+ ret i32 0
+}
+
; CHECK: ![[MD0]] = !{!"branch_weights", i32 300, i32 102}
; CHECK: ![[MD1]] = !{!"branch_weights", i32 99, i32 100, i32 101}
; CHECK: ![[MD2]] = !{!"branch_weights", i32 99, i32 100, i32 101, i32 102}
; CHECK: ![[MD3]] = !{!"branch_weights", i32 102, i32 100, i32 101}
; CHECK: ![[MD4]] = !{!"branch_weights", i32 99, i32 113, i32 142, i32 100, i32 101, i32 102}
+; CHECK: ![[MD5]] = !{!"branch_weights", i32 300, i32 100, i32 101, i32 102}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Switch branch weight metadata has an optional `expected` tag. SwitchInstProfUpdateWrapper::getSuccessorWeight() did not handle this tag; if it was present, it would return nullopt, effectively ignoring the metadata. This bug was found by a large run of Opus 4.7 looking for bugs in LLVM.
jlebar
force-pushed
the
fix2-232-loop-unswitch-switch-default-weight
branch
from
0ff102d to
35afc30
Compare
mtrofin
reviewed
May 27, 2026
Member
Author
|
Thank you for the review! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switch branch weight metadata has an optional
expectedtag.SwitchInstProfUpdateWrapper::getSuccessorWeight() did not handle this
tag; if it was present, it would return nullopt, effectively ignoring
the metadata.
This bug was found by a large run of Opus 4.7 looking for bugs in LLVM.