diff --git a/clang/test/Profile/missing-annotations-branch.c b/clang/test/Profile/missing-annotations-branch.c new file mode 100644 index 0000000000000..fa764d9238c8a --- /dev/null +++ b/clang/test/Profile/missing-annotations-branch.c @@ -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 + diff --git a/clang/test/Profile/missing-annotations-switch.c b/clang/test/Profile/missing-annotations-switch.c new file mode 100644 index 0000000000000..2d7ea0865ac8a --- /dev/null +++ b/clang/test/Profile/missing-annotations-switch.c @@ -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 + diff --git a/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp b/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp index 17c5a4ee1fd0b..c49bf830ce2aa 100644 --- a/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp +++ b/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp @@ -369,9 +369,21 @@ static bool lowerExpectIntrinsic(Function &F) { if (BranchInst *BI = dyn_cast(BB.getTerminator())) { if (handleBranchExpect(*BI)) ExpectIntrinsicsHandled++; + else { + SmallVector Weights; + if (extractBranchWeights(*BI, Weights)) + misexpect::checkMissingAnnotations(*BI, Weights, + /*IsFrontendInstr=*/true); + } } else if (SwitchInst *SI = dyn_cast(BB.getTerminator())) { if (handleSwitchExpect(*SI)) ExpectIntrinsicsHandled++; + else { + SmallVector 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(&Inst)) { if (handleBrSelExpect(*SI)) ExpectIntrinsicsHandled++; + else { + SmallVector Weights; + if (extractBranchWeights(*SI, Weights)) + misexpect::checkMissingAnnotations(*SI, Weights, + /*isFrontend=*/true); + } } continue; } diff --git a/llvm/lib/Transforms/Utils/MisExpect.cpp b/llvm/lib/Transforms/Utils/MisExpect.cpp index 933d9a146533d..1d88f867971e8 100644 --- a/llvm/lib/Transforms/Utils/MisExpect.cpp +++ b/llvm/lib/Transforms/Utils/MisExpect.cpp @@ -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 ExpectedWeights; if (extractBranchWeights(I, ExpectedWeights))