Skip to content

[misexpect] Support missing-annotations diagnostics from frontend profile data #96524

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

Open
wants to merge 3 commits into
base: users/ilovepi/spr/main.misexpect-support-diagnostics-from-frontend-profile-data
Choose a base branch
from
Open
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
62 changes: 62 additions & 0 deletions clang/test/Profile/missing-annotations-branch.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// RUN: rm -rf %t && mkdir -p %t
// RUN: split-file %s %t

/// Test that missing-annotations detects branches that are hot, but not annotated.
// RUN: llvm-profdata merge %t/a.proftext -o %t/profdata
// RUN: %clang_cc1 %t/a.c -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t/profdata -verify -mllvm -pgo-missing-annotations -Rpass=missing-annotations -fdiagnostics-misexpect-tolerance=10

/// Test that we don't report any diagnostics, if the threshold isn't exceeded.
// RUN: %clang_cc1 %t/a.c -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t/profdata -mllvm -pgo-missing-annotations -Rpass=missing-annotations 2>&1 | FileCheck -implicit-check-not=remark %s

//--- a.c
// foo-no-diagnostics
#define UNLIKELY(x) __builtin_expect(!!(x), 0)

int foo(int);
int baz(int);
int buzz(void);

const int inner_loop = 100;
const int outer_loop = 2000;

int bar(void) { // imprecise-remark-re {{Extremely hot condition. Consider adding llvm.expect intrinsic{{.*}}}}
int a = buzz();
int x = 0;
if (a % (outer_loop * inner_loop) == 0) { // expected-remark {{Extremely hot condition. Consider adding llvm.expect intrinsic}}
x = baz(a);
} else {
x = foo(50);
}
return x;
}

int fizz(void) {
int a = buzz();
int x = 0;
if ((a % (outer_loop * inner_loop) == 0)) { // expected-remark-re {{Extremely hot condition. Consider adding llvm.expect intrinsic{{.*}}}}}
x = baz(a);
} else {
x = foo(50);
}
return x;
}

//--- a.proftext
bar
# Func Hash:
11262309464
# Num Counters:
2
# Counter Values:
1901
99

fizz
# Func Hash:
11262309464
# Num Counters:
2
# Counter Values:
1901
99

64 changes: 64 additions & 0 deletions clang/test/Profile/missing-annotations-switch.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// RUN: rm -rf %t && mkdir -p %t
// RUN: split-file %s %t

/// Test that missing-annotations detects switch conditions that are hot, but not annotated.
// RUN: llvm-profdata merge %t/a.proftext -o %t/profdata
// RUN: %clang_cc1 %t/a.c -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t/profdata -verify -mllvm -pgo-missing-annotations -Rpass=missing-annotations -fdiagnostics-misexpect-tolerance=10

/// Test that we don't report any diagnostics, if the threshold isn't exceeded.
// RUN: %clang_cc1 %t/a.c -O2 -o - -emit-llvm -fprofile-instrument-use-path=%t/profdata -mllvm -pgo-missing-annotations -Rpass=missing-annotations 2>&1 | FileCheck -implicit-check-not=remark %s

//--- a.c
#define inner_loop 1000
#define outer_loop 20
#define arry_size 25

int arry[arry_size] = {0};

int rand(void);
int sum(int *buff, int size);
int random_sample(int *buff, int size);

int main(void) {
int val = 0;

int j, k;
for (j = 0; j < outer_loop; ++j) {
for (k = 0; k < inner_loop; ++k) {
unsigned condition = rand() % 10000;
switch (condition) { // expected-remark {{Extremely hot condition. Consider adding llvm.expect intrinsic}}

case 0:
val += sum(arry, arry_size);
break;
case 1:
case 2:
case 3:
break;
default:
val += random_sample(arry, arry_size);
break;
} // end switch
} // end inner_loop
} // end outer_loop

return val;
}

//--- a.proftext
main
# Func Hash:
872687477373597607
# Num Counters:
9
# Counter Values:
2
9
2
2
3
3
1
999
18001

18 changes: 18 additions & 0 deletions llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
Original file line number Diff line number Diff line change
@@ -369,9 +369,21 @@ static bool lowerExpectIntrinsic(Function &F) {
if (BranchInst *BI = dyn_cast<BranchInst>(BB.getTerminator())) {
if (handleBranchExpect(*BI))
ExpectIntrinsicsHandled++;
else {
SmallVector<uint32_t> Weights;
if (extractBranchWeights(*BI, Weights))
misexpect::checkMissingAnnotations(*BI, Weights,
/*IsFrontendInstr=*/true);
}
} else if (SwitchInst *SI = dyn_cast<SwitchInst>(BB.getTerminator())) {
if (handleSwitchExpect(*SI))
ExpectIntrinsicsHandled++;
else {
SmallVector<uint32_t> Weights;
if (extractBranchWeights(*SI, Weights))
misexpect::checkMissingAnnotations(*SI, Weights,
/*isFrontend=*/true);
}
}

// Remove llvm.expect intrinsics. Iterate backwards in order
@@ -383,6 +395,12 @@ static bool lowerExpectIntrinsic(Function &F) {
if (SelectInst *SI = dyn_cast<SelectInst>(&Inst)) {
if (handleBrSelExpect(*SI))
ExpectIntrinsicsHandled++;
else {
SmallVector<uint32_t> Weights;
if (extractBranchWeights(*SI, Weights))
misexpect::checkMissingAnnotations(*SI, Weights,
/*isFrontend=*/true);
}
}
continue;
}
3 changes: 1 addition & 2 deletions llvm/lib/Transforms/Utils/MisExpect.cpp
Original file line number Diff line number Diff line change
@@ -302,8 +302,7 @@ void checkMissingAnnotations(Instruction &I,
return;

if (IsFrontendInstr) {
// TODO: Frontend checking will have to be thought through, since we need
// to do the check on branches that don't have expect intrinsics
verifyMissingAnnotations(I, ExistingWeights);
} else {
SmallVector<uint32_t> ExpectedWeights;
if (extractBranchWeights(I, ExpectedWeights))