Skip to content

[IR][PGO] Verify the structure of VP metadata. #145584

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions llvm/include/llvm/IR/ProfDataUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ LLVM_ABI bool hasProfMD(const Instruction &I);
/// Checks if an MDNode contains Branch Weight Metadata
LLVM_ABI bool isBranchWeightMD(const MDNode *ProfileData);

/// Checks if an MDNode contains value profiling Metadata
LLVM_ABI bool isValueProfileMD(const MDNode *ProfileData);

/// Checks if an instructions has Branch Weight Metadata
///
/// \param I The instruction to check
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/IR/ProfDataUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ bool isBranchWeightMD(const MDNode *ProfileData) {
return isTargetMD(ProfileData, MDProfLabels::BranchWeights, MinBWOps);
}

static bool isValueProfileMD(const MDNode *ProfileData) {
bool isValueProfileMD(const MDNode *ProfileData) {
return isTargetMD(ProfileData, MDProfLabels::ValueProfile, MinVPOps);
}

Expand Down
23 changes: 21 additions & 2 deletions llvm/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
#include "llvm/IR/Value.h"
#include "llvm/InitializePasses.h"
#include "llvm/Pass.h"
#include "llvm/ProfileData/InstrProf.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is potentially cyclic deps. Could we reorganize ProfileData or this?

#include "llvm/Support/AMDGPUAddrSpace.h"
#include "llvm/Support/AtomicOrdering.h"
#include "llvm/Support/Casting.h"
Expand Down Expand Up @@ -5032,9 +5033,27 @@ void Verifier::visitProfMetadata(Instruction &I, MDNode *MD) {
Check(mdconst::dyn_extract<ConstantInt>(MDO),
"!prof brunch_weights operand is not a const int");
}
} else if (ProfName == MDProfLabels::ValueProfile) {
Check(isValueProfileMD(MD), "invalid value profiling metadata", MD);
ConstantInt *KindInt = mdconst::dyn_extract<ConstantInt>(MD->getOperand(1));
Check(KindInt, "VP !prof missing kind argument", MD);

auto Kind = KindInt->getZExtValue();
Check(Kind >= InstrProfValueKind::IPVK_First &&
Kind <= InstrProfValueKind::IPVK_Last,
"Invalid VP !prof kind", MD);
Check(MD->getNumOperands() % 2 == 1,
"VP !prof should have an even number "
"of arguments after 'VP'",
MD);
if (Kind == InstrProfValueKind::IPVK_IndirectCallTarget ||
Kind == InstrProfValueKind::IPVK_MemOPSize)
Check(isa<CallBase>(I),
"VP !prof indirect call or memop size expected to be applied to "
"CallBase instructions only",
MD);
} else {
Check(ProfName == MDProfLabels::ValueProfile,
"expected either branch_weights or VP profile name", MD);
CheckFailed("expected either branch_weights or VP profile name", MD);
}
}

Expand Down
38 changes: 38 additions & 0 deletions llvm/test/Verifier/value-profile.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
; Test MD_prof "VP" validation

; RUN: split-file %s %t
; RUN: opt -passes=verify %t/valid.ll --disable-output
; RUN: not opt -passes=verify %t/invalid-kind.ll --disable-output 2>&1 | FileCheck %s --check-prefix=INVALID-KIND
; RUN: not opt -passes=verify %t/invalid-count.ll --disable-output 2>&1 | FileCheck %s --check-prefix=INVALID-COUNT
; RUN: not opt -passes=verify %t/invalid-place.ll --disable-output 2>&1 | FileCheck %s --check-prefix=INVALID-PLACE

;--- valid.ll
define void @test(ptr %0) {
call void %0(), !prof !0
ret void
}
!0 = !{!"VP", i32 0, i32 20, i64 1234, i64 10, i64 5678, i64 5}

;--- invalid-kind.ll
define void @test(ptr %0) {
call void %0(), !prof !0
ret void
}
!0 = !{!"VP", i32 3, i32 20, i64 1234, i64 10, i64 5678, i64 5}
; INVALID-KIND: Invalid VP !prof kind

;--- invalid-count.ll
define void @test(ptr %0) {
call void %0(), !prof !0
ret void
}
!0 = !{!"VP", i32 1, i64 1234, i64 10, i64 5678, i64 5}
; INVALID-COUNT: VP !prof should have an even number of arguments after 'VP'

;--- invalid-place.ll
define i32 @test(i32 %0) {
%r = add i32 %0, 1, !prof !0
ret i32 %r
}
!0 = !{!"VP", i32 1, i32 20, i64 1234, i64 10, i64 5678, i64 5}
; INVALID-PLACE: VP !prof indirect call or memop size expected to be applied to CallBase instructions only
Loading