Skip to content

[IR][PGO] Verify invalid MD_prof metadata on instructions #145576

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
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/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5008,6 +5008,9 @@ void Verifier::visitProfMetadata(Instruction &I, MDNode *MD) {
Check(mdconst::dyn_extract<ConstantInt>(MDO),
"!prof brunch_weights operand is not a const int");
}
} else {
Check(ProfName == "VP", "expected either branch_weights or VP profile name",
MD);
}
}

Expand Down
44 changes: 5 additions & 39 deletions llvm/test/Transforms/SimplifyCFG/preserve-branchweights.ll
Original file line number Diff line number Diff line change
Expand Up @@ -38,45 +38,12 @@ Z:
ret void
}

; Make sure the metadata name string is "branch_weights" before propagating it.

define void @fake_weights(i1 %a, i1 %b) {
; CHECK-LABEL: @fake_weights(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[A_NOT:%.*]] = xor i1 [[A:%.*]], true
; CHECK-NEXT: [[C:%.*]] = or i1 [[B:%.*]], false
; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[A_NOT]], i1 [[C]], i1 false
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF1:![0-9]+]]
; CHECK: common.ret:
; CHECK-NEXT: ret void
; CHECK: Y:
; CHECK-NEXT: call void @helper(i32 0)
; CHECK-NEXT: br label [[COMMON_RET:%.*]]
; CHECK: Z:
; CHECK-NEXT: call void @helper(i32 1)
; CHECK-NEXT: br label [[COMMON_RET]]
;
entry:
br i1 %a, label %Y, label %X, !prof !12
X:
%c = or i1 %b, false
br i1 %c, label %Z, label %Y, !prof !1

Y:
call void @helper(i32 0)
ret void

Z:
call void @helper(i32 1)
ret void
}

define void @test2(i1 %a, i1 %b) {
; CHECK-LABEL: @test2(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[C:%.*]] = or i1 [[B:%.*]], false
; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[A:%.*]], i1 [[C]], i1 false
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF2:![0-9]+]]
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF1:![0-9]+]]
; CHECK: common.ret:
; CHECK-NEXT: ret void
; CHECK: Y:
Expand Down Expand Up @@ -107,7 +74,7 @@ define void @test3(i1 %a, i1 %b) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[C:%.*]] = or i1 [[B:%.*]], false
; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[A:%.*]], i1 [[C]], i1 false
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF1]]
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF2:![0-9]+]]
; CHECK: common.ret:
; CHECK-NEXT: ret void
; CHECK: Y:
Expand Down Expand Up @@ -138,7 +105,7 @@ define void @test4(i1 %a, i1 %b) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[C:%.*]] = or i1 [[B:%.*]], false
; CHECK-NEXT: [[OR_COND:%.*]] = select i1 [[A:%.*]], i1 [[C]], i1 false
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF1]]
; CHECK-NEXT: br i1 [[OR_COND]], label [[Z:%.*]], label [[Y:%.*]], !prof [[PROF2]]
; CHECK: common.ret:
; CHECK-NEXT: ret void
; CHECK: Y:
Expand Down Expand Up @@ -1120,7 +1087,6 @@ exit:
!9 = !{!"branch_weights", i32 7, i32 6}
!10 = !{!"branch_weights", i32 672646, i32 21604207}
!11 = !{!"branch_weights", i32 6960, i32 21597248}
!12 = !{!"these_are_not_the_branch_weights_you_are_looking_for", i32 3, i32 5}
!13 = !{!"branch_weights", i32 2, i32 3}
!14 = !{!"branch_weights", i32 4, i32 7}
!15 = !{!"branch_weights", i32 99, i32 1}
Expand All @@ -1136,8 +1102,8 @@ exit:
; CHECK: attributes #[[ATTR2:[0-9]+]] = { noredzone nounwind ssp memory(none) }
;.
; CHECK: [[PROF0]] = !{!"branch_weights", i32 5, i32 11}
; CHECK: [[PROF1]] = !{!"branch_weights", i32 1, i32 3}
; CHECK: [[PROF2]] = !{!"branch_weights", i32 1, i32 5}
; CHECK: [[PROF1]] = !{!"branch_weights", i32 1, i32 5}
; CHECK: [[PROF2]] = !{!"branch_weights", i32 1, i32 3}
; CHECK: [[PROF3]] = !{!"branch_weights", i32 7, i32 1, i32 2}
; CHECK: [[PROF4]] = !{!"branch_weights", i32 49, i32 12, i32 24, i32 35}
; CHECK: [[PROF5]] = !{!"branch_weights", i32 11, i32 5}
Expand Down
39 changes: 39 additions & 0 deletions llvm/test/Verifier/branch-weight.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
; Test MD_prof validation

; RUN: split-file %s %t
; RUN: opt -passes=verify %t/valid.ll --disable-output
; RUN: not opt -passes=verify %t/invalid1.ll --disable-output 2>&1 | FileCheck %s
; RUN: not opt -passes=verify %t/invalid2.ll --disable-output 2>&1 | FileCheck %s
Comment on lines +5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a bit ad-hoc to share the rely on the check annotation of the source file. I'm unsure how this is done normally, so maybe this is fine(?).
I would probably pass %t/invalid1.ll and %t/invalid2.ll to FileCheck instead of %s in both cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

The usual way to do this is passing %s to FileCheck but using a different --check-prefix for each file. But given that the error message is the same here, this should be fine as-is...


;--- valid.ll
define void @test(i1 %0) {
br i1 %0, label %2, label %3, !prof !0
2:
ret void
3:
ret void
}
!0 = !{!"branch_weights", i32 1, i32 2}

;--- invalid1.ll
define void @test(i1 %0) {
br i1 %0, label %2, label %3, !prof !0
2:
ret void
3:
ret void
}
!0 = !{!"invalid", i32 1, i32 2}

;--- invalid2.ll
define void @test(i1 %0) {
br i1 %0, label %2, label %3, !prof !0
2:
ret void
3:
ret void
}

!0 = !{!"function_entry_count", i32 1}

; CHECK: expected either branch_weights or VP profile name
16 changes: 0 additions & 16 deletions mlir/test/Target/LLVMIR/Import/import-failure.ll
Original file line number Diff line number Diff line change
Expand Up @@ -258,22 +258,6 @@ end:

; // -----

; CHECK: <unknown>
; CHECK-SAME: warning: expected function_entry_count to be attached to a function
; CHECK: warning: unhandled metadata: !0 = !{!"function_entry_count", i64 42}
define void @cond_br(i1 %arg) {
entry:
br i1 %arg, label %bb1, label %bb2, !prof !0
bb1:
ret void
bb2:
ret void
}

!0 = !{!"function_entry_count", i64 42}

; // -----

; CHECK: <unknown>
; CHECK-SAME: warning: dropped instruction: call void @llvm.experimental.noalias.scope.decl(metadata !0)
define void @unused_scope() {
Expand Down