Skip to content

[profcheck] Require unknown metadata have an origin parameter #157594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 10, 2025
Merged

[profcheck] Require unknown metadata have an origin parameter #157594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions llvm/include/llvm/IR/ProfDataUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,15 @@ inline uint32_t scaleBranchCount(uint64_t Count, uint64_t Scale) {
/// do not accidentally drop profile info, and this API is called in cases where
/// the pass explicitly cannot provide that info. Defaulting it in would hide
/// bugs where the pass forgets to transfer over or otherwise specify profile
/// info.
LLVM_ABI void setExplicitlyUnknownBranchWeights(Instruction &I);
/// info. Use `PassName` to capture the pass name (i.e. DEBUG_TYPE) for
/// debuggability.
LLVM_ABI void setExplicitlyUnknownBranchWeights(Instruction &I,
StringRef PassName);

/// Analogous to setExplicitlyUnknownBranchWeights, but for functions and their
/// entry counts.
LLVM_ABI void setExplicitlyUnknownFunctionEntryCount(Function &F);
LLVM_ABI void setExplicitlyUnknownFunctionEntryCount(Function &F,
StringRef PassName);

LLVM_ABI bool isExplicitlyUnknownProfileMetadata(const MDNode &MD);
LLVM_ABI bool hasExplicitlyUnknownBranchWeights(const Instruction &I);
Expand Down
12 changes: 7 additions & 5 deletions llvm/lib/IR/ProfDataUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,24 +242,26 @@ bool extractProfTotalWeight(const Instruction &I, uint64_t &TotalVal) {
return extractProfTotalWeight(I.getMetadata(LLVMContext::MD_prof), TotalVal);
}

void setExplicitlyUnknownBranchWeights(Instruction &I) {
void setExplicitlyUnknownBranchWeights(Instruction &I, StringRef PassName) {
MDBuilder MDB(I.getContext());
I.setMetadata(
LLVMContext::MD_prof,
MDNode::get(I.getContext(),
MDB.createString(MDProfLabels::UnknownBranchWeightsMarker)));
{MDB.createString(MDProfLabels::UnknownBranchWeightsMarker),
MDB.createString(PassName)}));
}

void setExplicitlyUnknownFunctionEntryCount(Function &F) {
void setExplicitlyUnknownFunctionEntryCount(Function &F, StringRef PassName) {
MDBuilder MDB(F.getContext());
F.setMetadata(
LLVMContext::MD_prof,
MDNode::get(F.getContext(),
MDB.createString(MDProfLabels::UnknownBranchWeightsMarker)));
{MDB.createString(MDProfLabels::UnknownBranchWeightsMarker),
MDB.createString(PassName)}));
}

bool isExplicitlyUnknownProfileMetadata(const MDNode &MD) {
if (MD.getNumOperands() != 1)
if (MD.getNumOperands() != 2)
return false;
return MD.getOperand(0).equalsStr(MDProfLabels::UnknownBranchWeightsMarker);
}
Expand Down
25 changes: 18 additions & 7 deletions llvm/lib/IR/Verifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
void verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
const Value *V, bool IsIntrinsic, bool IsInlineAsm);
void verifyFunctionMetadata(ArrayRef<std::pair<unsigned, MDNode *>> MDs);

void verifyUnknownProfileMetadata(MDNode *MD);
void visitConstantExprsRecursively(const Constant *EntryC);
void visitConstantExpr(const ConstantExpr *CE);
void visitConstantPtrAuth(const ConstantPtrAuth *CPA);
Expand Down Expand Up @@ -2515,19 +2515,31 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
V);
}
}
void Verifier::verifyUnknownProfileMetadata(MDNode *MD) {
Check(MD->getNumOperands() == 2,
"'unknown' !prof should have a single additional operand", MD);
auto *PassName = dyn_cast<MDString>(MD->getOperand(1));
Check(PassName != nullptr,
"'unknown' !prof should have an additional operand of type "
"string");
Check(!PassName->getString().empty(),
"the 'unknown' !prof operand should not be an empty string");
}

void Verifier::verifyFunctionMetadata(
ArrayRef<std::pair<unsigned, MDNode *>> MDs) {
for (const auto &Pair : MDs) {
if (Pair.first == LLVMContext::MD_prof) {
MDNode *MD = Pair.second;
Check(MD->getNumOperands() >= 2,
"!prof annotations should have no less than 2 operands", MD);
// We may have functions that are synthesized by the compiler, e.g. in
// WPD, that we can't currently determine the entry count.
if (isExplicitlyUnknownProfileMetadata(*MD))
if (MD->getOperand(0).equalsStr(
MDProfLabels::UnknownBranchWeightsMarker)) {
verifyUnknownProfileMetadata(MD);
continue;

Check(MD->getNumOperands() >= 2,
"!prof annotations should have no less than 2 operands", MD);
}

// Check first operand.
Check(MD->getOperand(0) != nullptr, "first operand should not be null",
Expand Down Expand Up @@ -5054,8 +5066,7 @@ void Verifier::visitProfMetadata(Instruction &I, MDNode *MD) {
"'unknown' !prof should only appear on instructions on which "
"'branch_weights' would",
MD);
Check(MD->getNumOperands() == 1,
"'unknown' !prof should have no additional operands", MD);
verifyUnknownProfileMetadata(MD);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,7 @@ void DevirtModule::tryICallBranchFunnel(

if (!JT->getEntryCount().has_value()) {
// FIXME: we could pass through thinlto the necessary information.
setExplicitlyUnknownFunctionEntryCount(*JT);
setExplicitlyUnknownFunctionEntryCount(*JT, DEBUG_TYPE);
}
}

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Scalar/JumpTableToSwitch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ expandToSwitch(CallBase *CB, const JumpTableTy &JT, DomTreeUpdater &DTU,
setBranchWeights(*Switch, downscaleWeights(BranchWeights),
/*IsExpected=*/false);
} else
setExplicitlyUnknownBranchWeights(*Switch);
setExplicitlyUnknownBranchWeights(*Switch, DEBUG_TYPE);
if (PHI)
CB->replaceAllUsesWith(PHI);
CB->eraseFromParent();
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/JumpTableToSwitch/basic.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ define i32 @function_with_jump_table_addrspace_42(i32 %index) addrspace(42) {
; CHECK: [[META0]] = !{i64 5678}
; CHECK: [[META1]] = !{i64 5555}
; CHECK: [[PROF2]] = !{!"branch_weights", i32 0, i32 20, i32 5}
; CHECK: [[PROF3]] = !{!"unknown"}
; CHECK: [[PROF3]] = !{!"unknown", !"jump-table-to-switch"}
;.
; THRESHOLD-0: [[META0]] = !{i64 5678}
; THRESHOLD-0: [[META1]] = !{i64 5555}
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/PGOProfile/prof-inject-existing.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ no:
}

!0 = !{!"branch_weights", i32 1, i32 2}
!1 = !{!"unknown"}
!1 = !{!"unknown", !"test"}
; CHECK: define void @foo(i32 %i) !prof !0
; CHECK: br i1 %c, label %yes, label %no, !prof !1
; CHECK: !0 = !{!"function_entry_count", i64 1000}
; CHECK: !1 = !{!"branch_weights", i32 1, i32 2}
; CHECK: !2 = !{!"unknown"}
; CHECK: !2 = !{!"unknown", !"test"}
4 changes: 2 additions & 2 deletions llvm/test/Transforms/PGOProfile/prof-verify-existing.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ no:

!0 = !{!"function_entry_count", i32 1}
!1 = !{!"branch_weights", i32 1, i32 2}
!2 = !{!"unknown"}
!2 = !{!"unknown", !"test"}
; CHECK: define void @foo(i32 %i) !prof !0
; CHECK: br i1 %c, label %yes, label %no, !prof !1
; CHECK: !0 = !{!"function_entry_count", i64 1}
; CHECK: !1 = !{!"branch_weights", i32 1, i32 2}
; CHECK: !2 = !{!"unknown"}
; CHECK: !2 = !{!"unknown", !"test"}
4 changes: 2 additions & 2 deletions llvm/test/Transforms/WholeProgramDevirt/branch-funnel-profile.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
; NORETP: define hidden void @__typeid_typeid1_rv_0_branch_funnel(ptr nest %0, ...) !prof !11
; NORETP: define internal void @branch_funnel(ptr nest %0, ...) !prof !11
; NORETP: define internal void @branch_funnel.1(ptr nest %0, ...) !prof !11
; NORETP: !11 = !{!"unknown"}
; NORETP: !11 = !{!"unknown", !"wholeprogramdevirt"}

; O3: define hidden void @__typeid_typeid1_0_branch_funnel(ptr nest %0, ...) local_unnamed_addr #5 !prof !11
; O3: define hidden void @__typeid_typeid1_rv_0_branch_funnel(ptr nest %0, ...) local_unnamed_addr #5 !prof !11
Expand All @@ -27,7 +27,7 @@
; O3: define hidden void @__typeid_typeid3_rv_0_branch_funnel(ptr nest %0, ...) local_unnamed_addr #5 !prof !12
; O3: !10 = !{!"function_entry_count", i64 1000}
; O3: !11 = !{!"function_entry_count", i64 3000}
; O3: !12 = !{!"unknown"}
; O3: !12 = !{!"unknown", !"wholeprogramdevirt"}

target datalayout = "e-p:64:64"
target triple = "x86_64-unknown-linux-gnu"
Expand Down
38 changes: 33 additions & 5 deletions llvm/test/Verifier/branch-weight.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
; RUN: opt -passes=verify %t/unknown-correct.ll --disable-output

; RUN: not opt -passes=verify %t/unknown-invalid.ll --disable-output 2>&1 | FileCheck %s --check-prefix=EXTRA-ARGS
; RUN: not opt -passes=verify %t/unknown-noargs.ll --disable-output 2>&1 | FileCheck %s --check-prefix=NO-ARGS
; RUN: not opt -passes=verify %t/unknown-empty.ll --disable-output 2>&1 | FileCheck %s --check-prefix=EMPTY-ARGS
; RUN: opt -passes=verify %t/unknown-on-function1.ll -S -o - | FileCheck %s --check-prefix=ON-FUNCTION1
; RUN: not opt -passes=verify %t/unknown-on-function2.ll --disable-output 2>&1 | FileCheck %s --check-prefix=ON-FUNCTION2
; RUN: not opt -passes=verify %t/invalid-unknown-placement.ll --disable-output 2>&1 | FileCheck %s --check-prefix=INVALID-UNKNOWN-PLACEMENT
Expand Down Expand Up @@ -111,7 +113,7 @@ exit:
ret i32 %r
}

!0 = !{!"unknown"}
!0 = !{!"unknown", !"test"}

;--- unknown-invalid.ll
define void @test(i32 %a) {
Expand All @@ -124,14 +126,14 @@ no:
}

!0 = !{!"unknown", i32 12, i32 67}
; EXTRA-ARGS: 'unknown' !prof should have no additional operands
; EXTRA-ARGS: 'unknown' !prof should have a single additional operand

;--- unknown-on-function1.ll
define void @test() !prof !0 {
ret void
}

!0 = !{!"unknown"}
!0 = !{!"unknown", !"test"}
; ON-FUNCTION1: define void @test() !prof !0

;--- unknown-on-function2.ll
Expand All @@ -140,12 +142,38 @@ define void @test() !prof !0 {
}

!0 = !{!"unknown", i64 123}
; ON-FUNCTION2: first operand should be 'function_entry_count' or 'synthetic_function_entry_count'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to this error message (about entry count)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it became more specific.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a different check -- the original test case intends to check entry count being the first one, so perhaps change this test to have value second operand.

; ON-FUNCTION2: 'unknown' !prof should have an additional operand of type string

;--- invalid-unknown-placement.ll
define i32 @test() {
%r = add i32 1, 2, !prof !0
ret i32 %r
}
!0 = !{!"unknown"}
!0 = !{!"unknown", !"test"}
; INVALID-UNKNOWN-PLACEMENT: 'unknown' !prof should only appear on instructions on which 'branch_weights' would

;--- unknown-noargs.ll
define void @test(i32 %a) {
%c = icmp eq i32 %a, 0
br i1 %c, label %yes, label %no, !prof !0
yes:
ret void
no:
ret void
}

!0 = !{!"unknown"}
; NO-ARGS: 'unknown' !prof should have a single additional operand

;--- unknown-empty.ll
define void @test(i32 %a) {
%c = icmp eq i32 %a, 0
br i1 %c, label %yes, label %no, !prof !0
yes:
ret void
no:
ret void
}

!0 = !{!"unknown", !""}
; EMPTY-ARGS: the 'unknown' !prof operand should not be an empty string