Skip to content

Commit 98ec5f0

Browse files
mtrofinmtrofin
committed
[pgo] add means to specify "unknown" MD_prof
1 parent 90da616 commit 98ec5f0

File tree

4 files changed

+194
-25
lines changed

4 files changed

+194
-25
lines changed

‎llvm/include/llvm/IR/ProfDataUtils.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ struct MDProfLabels {
2727
static const char *FunctionEntryCount;
2828
static const char *SyntheticFunctionEntryCount;
2929
static const char *ExpectedBranchWeights;
30+
static const char *UnknownBranchWeightsMarker;
3031
};
3132

3233
/// Checks if an Instruction has MD_prof Metadata
@@ -143,6 +144,18 @@ LLVM_ABI bool extractProfTotalWeight(const Instruction &I,
143144
LLVM_ABI void setBranchWeights(Instruction &I, ArrayRef<uint32_t> Weights,
144145
bool IsExpected);
145146

147+
/// Specify that the branch weights for this terminator cannot be known at
148+
/// compile time. This should only be called by passes, and never as a default
149+
/// behavior in e.g. MDBuilder. The goal is to use this info to validate passes
150+
/// do not accidentally drop profile info, and this API is called in cases where
151+
/// the pass explicitly cannot provide that info. Defaulting it in would hide
152+
/// bugs where the pass forgets to transfer over or otherwise specify profile
153+
/// info.
154+
LLVM_ABI void setExplicitlyUnknownBranchWeights(Instruction &I);
155+
156+
LLVM_ABI bool isExplicitlyUnknownBranchWeightsMetadata(const MDNode &MD);
157+
LLVM_ABI bool hasExplicitlyUnknownBranchWeights(const Instruction &I);
158+
146159
/// Scaling the profile data attached to 'I' using the ratio of S/T.
147160
LLVM_ABI void scaleProfData(Instruction &I, uint64_t S, uint64_t T);
148161

‎llvm/lib/IR/ProfDataUtils.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ const char *MDProfLabels::ValueProfile = "VP";
9494
const char *MDProfLabels::FunctionEntryCount = "function_entry_count";
9595
const char *MDProfLabels::SyntheticFunctionEntryCount =
9696
"synthetic_function_entry_count";
97+
const char *MDProfLabels::UnknownBranchWeightsMarker = "unknown";
9798

9899
bool hasProfMD(const Instruction &I) {
99100
return I.hasMetadata(LLVMContext::MD_prof);
@@ -241,6 +242,27 @@ bool extractProfTotalWeight(const Instruction &I, uint64_t &TotalVal) {
241242
return extractProfTotalWeight(I.getMetadata(LLVMContext::MD_prof), TotalVal);
242243
}
243244

245+
void setExplicitlyUnknownBranchWeights(Instruction &I) {
246+
MDBuilder MDB(I.getContext());
247+
I.setMetadata(
248+
LLVMContext::MD_prof,
249+
MDNode::get(I.getContext(),
250+
MDB.createString(MDProfLabels::UnknownBranchWeightsMarker)));
251+
}
252+
253+
bool isExplicitlyUnknownBranchWeightsMetadata(const MDNode &MD) {
254+
if (MD.getNumOperands() != 1)
255+
return false;
256+
return MD.getOperand(0).equalsStr(MDProfLabels::UnknownBranchWeightsMarker);
257+
}
258+
259+
bool hasExplicitlyUnknownBranchWeights(const Instruction &I) {
260+
auto *MD = I.getMetadata(LLVMContext::MD_prof);
261+
if (!MD)
262+
return false;
263+
return isExplicitlyUnknownBranchWeightsMetadata(*MD);
264+
}
265+
244266
void setBranchWeights(Instruction &I, ArrayRef<uint32_t> Weights,
245267
bool IsExpected) {
246268
MDBuilder MDB(I.getContext());

‎llvm/lib/IR/Verifier.cpp‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2527,6 +2527,12 @@ void Verifier::verifyFunctionMetadata(
25272527
for (const auto &Pair : MDs) {
25282528
if (Pair.first == LLVMContext::MD_prof) {
25292529
MDNode *MD = Pair.second;
2530+
if (isExplicitlyUnknownBranchWeightsMetadata(*MD)) {
2531+
CheckFailed("'unknown' !prof metadata should appear only on "
2532+
"instructions supporting the 'branch_weights' metadata",
2533+
MD);
2534+
continue;
2535+
}
25302536
Check(MD->getNumOperands() >= 2,
25312537
"!prof annotations should have no less than 2 operands", MD);
25322538

@@ -4989,37 +4995,53 @@ void Verifier::visitDereferenceableMetadata(Instruction& I, MDNode* MD) {
49894995
}
49904996

49914997
void Verifier::visitProfMetadata(Instruction &I, MDNode *MD) {
4992-
Check(MD->getNumOperands() >= 2,
4993-
"!prof annotations should have no less than 2 operands", MD);
4994-
4998+
auto GetBranchingTerminatorNumOperands = [&]() {
4999+
unsigned ExpectedNumOperands = 0;
5000+
if (BranchInst *BI = dyn_cast<BranchInst>(&I))
5001+
ExpectedNumOperands = BI->getNumSuccessors();
5002+
else if (SwitchInst *SI = dyn_cast<SwitchInst>(&I))
5003+
ExpectedNumOperands = SI->getNumSuccessors();
5004+
else if (isa<CallInst>(&I))
5005+
ExpectedNumOperands = 1;
5006+
else if (IndirectBrInst *IBI = dyn_cast<IndirectBrInst>(&I))
5007+
ExpectedNumOperands = IBI->getNumDestinations();
5008+
else if (isa<SelectInst>(&I))
5009+
ExpectedNumOperands = 2;
5010+
else if (CallBrInst *CI = dyn_cast<CallBrInst>(&I))
5011+
ExpectedNumOperands = CI->getNumSuccessors();
5012+
return ExpectedNumOperands;
5013+
};
5014+
Check(MD->getNumOperands() >= 1,
5015+
"!prof annotations should have at least 1 operand", MD);
49955016
// Check first operand.
49965017
Check(MD->getOperand(0) != nullptr, "first operand should not be null", MD);
49975018
Check(isa<MDString>(MD->getOperand(0)),
49985019
"expected string with name of the !prof annotation", MD);
49995020
MDString *MDS = cast<MDString>(MD->getOperand(0));
50005021
StringRef ProfName = MDS->getString();
50015022

5023+
if (ProfName == MDProfLabels::UnknownBranchWeightsMarker) {
5024+
Check(GetBranchingTerminatorNumOperands() != 0 || isa<InvokeInst>(I),
5025+
"'unknown' !prof should only appear on instructions on which "
5026+
"'branch_weights' would",
5027+
MD);
5028+
Check(MD->getNumOperands() == 1,
5029+
"'unknown' !prof should have no additional operands", MD);
5030+
return;
5031+
}
5032+
5033+
Check(MD->getNumOperands() >= 2,
5034+
"!prof annotations should have no less than 2 operands", MD);
5035+
50025036
// Check consistency of !prof branch_weights metadata.
50035037
if (ProfName == MDProfLabels::BranchWeights) {
50045038
unsigned NumBranchWeights = getNumBranchWeights(*MD);
50055039
if (isa<InvokeInst>(&I)) {
50065040
Check(NumBranchWeights == 1 || NumBranchWeights == 2,
50075041
"Wrong number of InvokeInst branch_weights operands", MD);
50085042
} else {
5009-
unsigned ExpectedNumOperands = 0;
5010-
if (BranchInst *BI = dyn_cast<BranchInst>(&I))
5011-
ExpectedNumOperands = BI->getNumSuccessors();
5012-
else if (SwitchInst *SI = dyn_cast<SwitchInst>(&I))
5013-
ExpectedNumOperands = SI->getNumSuccessors();
5014-
else if (isa<CallInst>(&I))
5015-
ExpectedNumOperands = 1;
5016-
else if (IndirectBrInst *IBI = dyn_cast<IndirectBrInst>(&I))
5017-
ExpectedNumOperands = IBI->getNumDestinations();
5018-
else if (isa<SelectInst>(&I))
5019-
ExpectedNumOperands = 2;
5020-
else if (CallBrInst *CI = dyn_cast<CallBrInst>(&I))
5021-
ExpectedNumOperands = CI->getNumSuccessors();
5022-
else
5043+
const unsigned ExpectedNumOperands = GetBranchingTerminatorNumOperands();
5044+
if (ExpectedNumOperands == 0)
50235045
CheckFailed("!prof branch_weights are not allowed for this instruction",
50245046
MD);
50255047

‎llvm/test/Verifier/branch-weight.ll‎

Lines changed: 120 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,65 @@
11
; Test MD_prof validation
22

33
; RUN: split-file %s %t
4+
45
; RUN: opt -passes=verify %t/valid.ll --disable-output
5-
; RUN: not opt -passes=verify %t/invalid1.ll --disable-output 2>&1 | FileCheck %s
6-
; RUN: not opt -passes=verify %t/invalid2.ll --disable-output 2>&1 | FileCheck %s
6+
7+
; RUN: not opt -passes=verify %t/wrong-count.ll --disable-output 2>&1 | FileCheck %s --check-prefix=WRONG-COUNT
8+
; RUN: not opt -passes=verify %t/invalid-name1.ll --disable-output 2>&1 | FileCheck %s
9+
; RUN: not opt -passes=verify %t/invalid-name2.ll --disable-output 2>&1 | FileCheck %s
10+
11+
; RUN: opt -passes=verify %t/unknown-correct.ll --disable-output
12+
13+
; RUN: not opt -passes=verify %t/unknown-invalid.ll --disable-output 2>&1 | FileCheck %s --check-prefix=EXTRA-ARGS
14+
; RUN: not opt -passes=verify %t/unknown-on-function1.ll --disable-output 2>&1 | FileCheck %s --check-prefix=ON-FUNCTION1
15+
; RUN: not opt -passes=verify %t/unknown-on-function2.ll --disable-output 2>&1 | FileCheck %s --check-prefix=ON-FUNCTION2
16+
; RUN: not opt -passes=verify %t/invalid-unknown-placement.ll --disable-output 2>&1 | FileCheck %s --check-prefix=INVALID-UNKNOWN-PLACEMENT
717

818
;--- valid.ll
9-
define void @test(i1 %0) {
10-
br i1 %0, label %2, label %3, !prof !0
11-
2:
19+
declare void @to_invoke()
20+
declare i32 @__gxx_personality_v0(...)
21+
22+
define void @invoker() personality ptr @__gxx_personality_v0 {
23+
invoke void @to_invoke() to label %exit unwind label %lpad, !prof !0
24+
lpad:
25+
%ll = landingpad {ptr, i32}
26+
cleanup
1227
ret void
13-
3:
28+
exit:
1429
ret void
1530
}
31+
32+
define i32 @test(i32 %a) {
33+
%c = icmp eq i32 %a, 0
34+
br i1 %c, label %yes, label %exit, !prof !0
35+
yes:
36+
switch i32 %a, label %exit [ i32 1, label %case_b
37+
i32 2, label %case_c], !prof !1
38+
case_b:
39+
br label %exit
40+
case_c:
41+
br label %exit
42+
exit:
43+
%r = select i1 %c, i32 1, i32 2, !prof !0
44+
ret i32 %r
45+
}
1646
!0 = !{!"branch_weights", i32 1, i32 2}
47+
!1 = !{!"branch_weights", i32 1, i32 2, i32 3}
48+
49+
;--- wrong-count.ll
50+
define void @test(i32 %a) {
51+
%c = icmp eq i32 %a, 0
52+
br i1 %c, label %yes, label %no, !prof !0
53+
yes:
54+
ret void
55+
no:
56+
ret void
57+
}
58+
!0 = !{!"branch_weights", i32 1, i32 2, i32 3}
1759

18-
;--- invalid1.ll
60+
; WRONG-COUNT: Wrong number of operands
61+
62+
;--- invalid-name1.ll
1963
define void @test(i1 %0) {
2064
br i1 %0, label %2, label %3, !prof !0
2165
2:
@@ -25,7 +69,7 @@ define void @test(i1 %0) {
2569
}
2670
!0 = !{!"invalid", i32 1, i32 2}
2771

28-
;--- invalid2.ll
72+
;--- invalid-name2.ll
2973
define void @test(i1 %0) {
3074
br i1 %0, label %2, label %3, !prof !0
3175
2:
@@ -37,3 +81,71 @@ define void @test(i1 %0) {
3781
!0 = !{!"function_entry_count", i32 1}
3882

3983
; CHECK: expected either branch_weights or VP profile name
84+
85+
;--- unknown-correct.ll
86+
declare void @to_invoke()
87+
declare i32 @__gxx_personality_v0(...)
88+
89+
define void @invoker() personality ptr @__gxx_personality_v0 {
90+
invoke void @to_invoke() to label %exit unwind label %lpad, !prof !0
91+
lpad:
92+
%ll = landingpad {ptr, i32}
93+
cleanup
94+
ret void
95+
exit:
96+
ret void
97+
}
98+
99+
define i32 @test(i32 %a) {
100+
%c = icmp eq i32 %a, 0
101+
br i1 %c, label %yes, label %exit, !prof !0
102+
yes:
103+
switch i32 %a, label %exit [ i32 1, label %case_b
104+
i32 2, label %case_c], !prof !0
105+
case_b:
106+
br label %exit
107+
case_c:
108+
br label %exit
109+
exit:
110+
%r = select i1 %c, i32 1, i32 2, !prof !0
111+
ret i32 %r
112+
}
113+
114+
!0 = !{!"unknown"}
115+
116+
;--- unknown-invalid.ll
117+
define void @test(i32 %a) {
118+
%c = icmp eq i32 %a, 0
119+
br i1 %c, label %yes, label %no, !prof !0
120+
yes:
121+
ret void
122+
no:
123+
ret void
124+
}
125+
126+
!0 = !{!"unknown", i32 12, i32 67}
127+
; EXTRA-ARGS: 'unknown' !prof should have no additional operands
128+
129+
;--- unknown-on-function1.ll
130+
define void @test() !prof !0 {
131+
ret void
132+
}
133+
134+
!0 = !{!"unknown"}
135+
; ON-FUNCTION1: 'unknown' !prof metadata should appear only on instructions supporting the 'branch_weights' metadata
136+
137+
;--- unknown-on-function2.ll
138+
define void @test() !prof !0 {
139+
ret void
140+
}
141+
142+
!0 = !{!"unknown", i64 123}
143+
; ON-FUNCTION2: first operand should be 'function_entry_count' or 'synthetic_function_entry_count'
144+
145+
;--- invalid-unknown-placement.ll
146+
define i32 @test() {
147+
%r = add i32 1, 2, !prof !0
148+
ret i32 %r
149+
}
150+
!0 = !{!"unknown"}
151+
; INVALID-UNKNOWN-PLACEMENT: 'unknown' !prof should only appear on instructions on which 'branch_weights' would

0 commit comments

Comments
 (0)