paulkirth updated this revision to Diff 213164.
paulkirth added a comment.
Address feedback from review
1. Fixed comments in License headers
2. removed excess {}
3. Changed conditional assignment to ternary operation
4. Moved local functions into anonymous namespace
5. Added reference to the origin of the threshold values in the clang source
code
6. Updated warning to have more useful text
7. Fixed whitespace
8. Updated unqualified use of llvm::None
9. Updated tests with new warning text
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65300/new/
https://reviews.llvm.org/D65300
Files:
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CMakeLists.txt
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/MisExpect.cpp
clang/lib/CodeGen/MisExpect.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
clang/test/Profile/Inputs/misexpect-branch.proftext
clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
clang/test/Profile/Inputs/misexpect-switch.proftext
clang/test/Profile/misexpect-branch-cold.c
clang/test/Profile/misexpect-branch-nonconst-expected-val.c
clang/test/Profile/misexpect-branch.c
clang/test/Profile/misexpect-no-warning-without-flag.c
clang/test/Profile/misexpect-switch-default.c
clang/test/Profile/misexpect-switch-nonconst.c
clang/test/Profile/misexpect-switch-only-default-case.c
clang/test/Profile/misexpect-switch.c
Index: clang/test/Profile/misexpect-switch.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch.c
@@ -0,0 +1,68 @@
+// Test that misexpect detects mis-annotated switch statements
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+void init_arry() {
+ int i;
+ for (i = 0; i < arry_size; ++i) {
+ arry[i] = rand() % 10;
+ }
+}
+
+int main() {
+ init_arry();
+ 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 (__builtin_expect(condition, 0)) { // expected-warning-re {{Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.+}}% of profiled executions.}}
+ case 0:
+ val += sum(arry, arry_size);
+ break;
+ case 1:
+ case 2:
+ case 3:
+ case 4:
+ val += random_sample(arry, arry_size);
+ break;
+ default:
+ __builtin_unreachable();
+ } // end switch
+ } // end inner_loop
+ } // end outer_loop
+
+ return 0;
+}
+
+int sum(int *buff, int size) {
+ int total = 0;
+ int i = 0;
+ for (i = 0; i < size; ++i) {
+ total += buff[i];
+ }
+ return total;
+}
+
+int random_sample(int *buff, int size) {
+ int total = 0;
+ int i;
+ for (i = 0; i < size; ++i) {
+ if (rand() % 5 == 0)
+ total += buff[i];
+ }
+
+ return total;
+}
Index: clang/test/Profile/misexpect-switch-only-default-case.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch-only-default-case.c
@@ -0,0 +1,62 @@
+// Test that misexpect detects mis-annotated switch statements
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch-default-only.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+// expected-no-diagnostics
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+void init_arry() {
+ int i;
+ for (i = 0; i < arry_size; ++i) {
+ arry[i] = rand() % 10;
+ }
+}
+
+int main()
+{
+ init_arry();
+ 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 (__builtin_expect(condition, 0)) {
+ default:
+ val += random_sample(arry, arry_size);
+ break;
+ }; // end switch
+ } // end inner_loop
+ } // end outer_loop
+
+ return 0;
+}
+
+int sum(int *buff, int size) {
+ int total = 0;
+ int i = 0;
+ for (i = 0; i < size; ++i) {
+ total += buff[i];
+ }
+ return total;
+}
+
+int random_sample(int *buff, int size) {
+ int total = 0;
+ int i;
+ for (i = 0; i < size; ++i) {
+ if (rand() % 5 == 0)
+ total += buff[i];
+ }
+
+ return total;
+}
Index: clang/test/Profile/misexpect-switch-nonconst.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch-nonconst.c
@@ -0,0 +1,69 @@
+// Test that misexpect detects mis-annotated switch statements
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+// expected-no-diagnostics
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+void init_arry() {
+ int i;
+ for (i = 0; i < arry_size; ++i) {
+ arry[i] = rand() % 10;
+ }
+}
+
+int main() {
+ init_arry();
+ 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 (__builtin_expect(condition, rand())) {
+ case 0:
+ val += sum(arry, arry_size);
+ break;
+ case 1:
+ case 2:
+ case 3:
+ case 4:
+ val += random_sample(arry, arry_size);
+ break;
+ default:
+ __builtin_unreachable();
+ } // end switch
+ } // end inner_loop
+ } // end outer_loop
+
+ return 0;
+}
+
+int sum(int *buff, int size) {
+ int total = 0;
+ int i = 0;
+ for (i = 0; i < size; ++i) {
+ total += buff[i];
+ }
+ return total;
+}
+
+int random_sample(int *buff, int size) {
+ int total = 0;
+ int i;
+ for (i = 0; i < size; ++i) {
+ if (rand() % 5 == 0)
+ total += buff[i];
+ }
+
+ return total;
+}
Index: clang/test/Profile/misexpect-switch-default.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch-default.c
@@ -0,0 +1,68 @@
+// Test that misexpect detects mis-annotated switch statements
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+int sum(int *buff, int size);
+int random_sample(int *buff, int size);
+int rand();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+void init_arry() {
+ int i;
+ for (i = 0; i < arry_size; ++i) {
+ arry[i] = rand() % 10;
+ }
+}
+
+int main() {
+ init_arry();
+ int val = 0;
+
+ int j, k;
+ for (j = 0; j < outer_loop; ++j) {
+ for (k = 0; k < inner_loop; ++k) {
+ unsigned condition = rand() % 5;
+ switch (__builtin_expect(condition, 6)) { // expected-warning-re {{Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.+}}% of profiled executions.}}
+ case 0:
+ val += sum(arry, arry_size);
+ break;
+ case 1:
+ case 2:
+ case 3:
+ case 4:
+ val += random_sample(arry, arry_size);
+ break;
+ default:
+ __builtin_unreachable();
+ } // end switch
+ } // end inner_loop
+ } // end outer_loop
+
+ return 0;
+}
+
+int sum(int *buff, int size) {
+ int total = 0;
+ int i = 0;
+ for (i = 0; i < size; ++i) {
+ total += buff[i];
+ }
+ return total;
+}
+
+int random_sample(int *buff, int size) {
+ int total = 0;
+ int i;
+ for (i = 0; i < size; ++i) {
+ if (rand() % 5 == 0)
+ total += buff[i];
+ }
+
+ return total;
+}
Index: clang/test/Profile/misexpect-no-warning-without-flag.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-no-warning-without-flag.c
@@ -0,0 +1,51 @@
+// Test that misexpect detects mis-annotated branches
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify
+
+// expected-no-diagnostics
+#define likely(x) __builtin_expect(!!(x), 1)
+#define unlikely(x) __builtin_expect(!!(x), 0)
+
+int foo(int);
+int bar();
+int baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+int main() {
+ int val = 0;
+ for (int i = 0; i < outer_loop; ++i) {
+ for (int i = 0; i < inner_loop; ++i) {
+ val = bar();
+ }
+ }
+ return 0;
+}
+
+int bar() {
+ int rando = buzz();
+ int x = 0;
+ if (likely(rando % (outer_loop * inner_loop) == 0)) {
+ x = baz(rando);
+ } else {
+ x = foo(50);
+ }
+ return x;
+}
+
+int foo(int i) {
+ int j = 0;
+ while (i < 100) {
+ i += buzz() % 5;
+ j++;
+ }
+ return j;
+}
+
+int baz(int rando) {
+ int x = rando;
+ return x;
+}
Index: clang/test/Profile/misexpect-branch.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-branch.c
@@ -0,0 +1,50 @@
+// Test that misexpect detects mis-annotated branches
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+#define likely(x) __builtin_expect(!!(x), 1)
+#define unlikely(x) __builtin_expect(!!(x), 0)
+
+int foo(int);
+int bar();
+int baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+int main() {
+ int val = 0;
+ for (int i = 0; i < outer_loop; ++i) {
+ for (int i = 0; i < inner_loop; ++i) {
+ val = bar();
+ }
+ }
+ return 0;
+}
+
+int bar() {
+ int rando = buzz();
+ int x = 0;
+ if (likely(rando % (outer_loop * inner_loop) == 0)) { // expected-warning-re {{Potential performance regression from use of __builtin_expect(): Annotation was correct on {{.+}}% of profiled executions.}}
+ x = baz(rando);
+ } else {
+ x = foo(50);
+ }
+ return x;
+}
+
+int foo(int i) {
+ int j = 0;
+ while (i < 100) {
+ i += buzz() % 5;
+ j++;
+ }
+ return j;
+}
+
+int baz(int rando) {
+ int x = rando;
+ return x;
+}
Index: clang/test/Profile/misexpect-branch-nonconst-expected-val.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-branch-nonconst-expected-val.c
@@ -0,0 +1,51 @@
+// Test that misexpect detects mis-annotated branches
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch-nonconst-expect-arg.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+// expected-no-diagnostics
+#define likely(x) __builtin_expect(!!(x), 1)
+#define unlikely(x) __builtin_expect(!!(x), 0)
+
+int foo(int);
+int bar();
+int baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+int main() {
+ int val = 0;
+ for (int i = 0; i < outer_loop; ++i) {
+ for (int i = 0; i < inner_loop; ++i) {
+ val = bar();
+ }
+ }
+ return 0;
+}
+
+int bar() {
+ int rando = buzz();
+ int x = 0;
+ if (__builtin_expect(rando % (outer_loop * inner_loop) == 0, buzz())) {
+ x = baz(rando);
+ } else {
+ x = foo(50);
+ }
+ return x;
+}
+
+int foo(int i) {
+ int j = 0;
+ while (i < 100) {
+ i += buzz() % 5;
+ j++;
+ }
+ return j;
+}
+
+int baz(int rando) {
+ int x = rando;
+ return x;
+}
Index: clang/test/Profile/misexpect-branch-cold.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-branch-cold.c
@@ -0,0 +1,51 @@
+// Test that misexpect detects mis-annotated branches
+
+// RUN: llvm-profdata merge %S/Inputs/misexpect-branch.proftext -o %t.profdata
+// RUN: %clang_cc1 %s -O2 -o - -disable-llvm-passes -emit-llvm -fprofile-instrument-use-path=%t.profdata -verify -fmisexpect
+
+// expected-no-diagnostics
+#define likely(x) __builtin_expect(!!(x), 1)
+#define unlikely(x) __builtin_expect(!!(x), 0)
+
+int foo(int);
+int bar();
+int baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+int main() {
+ int val = 0;
+ for (int i = 0; i < outer_loop; ++i) {
+ for (int i = 0; i < inner_loop; ++i) {
+ val = bar();
+ }
+ }
+ return 0;
+}
+
+int bar() {
+ int rando = buzz();
+ int x = 0;
+ if (unlikely(rando % (outer_loop * inner_loop) == 0)) {
+ x = baz(rando);
+ } else {
+ x = foo(50);
+ }
+ return x;
+}
+
+int foo(int i) {
+ int j = 0;
+ while (i < 100) {
+ i += buzz() % 5;
+ j++;
+ }
+ return j;
+}
+
+int baz(int rando) {
+ int x = rando;
+ return x;
+}
Index: clang/test/Profile/Inputs/misexpect-switch.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-switch.proftext
@@ -0,0 +1,45 @@
+init_arry
+# Func Hash:
+18129
+# Num Counters:
+2
+# Counter Values:
+1
+25
+
+main
+# Func Hash:
+1965403898329309329
+# Num Counters:
+10
+# Counter Values:
+1
+20
+20000
+20000
+5
+19995
+0
+0
+0
+0
+
+random_sample
+# Func Hash:
+19458874217560
+# Num Counters:
+3
+# Counter Values:
+19995
+499875
+100042
+
+sum
+# Func Hash:
+1160280
+# Num Counters:
+2
+# Counter Values:
+5
+125
+
Index: clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
@@ -0,0 +1,40 @@
+init_arry
+# Func Hash:
+18129
+# Num Counters:
+2
+# Counter Values:
+1
+25
+
+main
+# Func Hash:
+79676873694057560
+# Num Counters:
+5
+# Counter Values:
+1
+20
+20000
+20000
+20000
+
+random_sample
+# Func Hash:
+19458874217560
+# Num Counters:
+3
+# Counter Values:
+20000
+500000
+100006
+
+sum
+# Func Hash:
+1160280
+# Num Counters:
+2
+# Counter Values:
+0
+0
+
Index: clang/test/Profile/Inputs/misexpect-branch.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-branch.proftext
@@ -0,0 +1,36 @@
+bar
+# Func Hash:
+45795613684824
+# Num Counters:
+2
+# Counter Values:
+200000
+0
+
+baz
+# Func Hash:
+24
+# Num Counters:
+1
+# Counter Values:
+0
+
+foo
+# Func Hash:
+635992
+# Num Counters:
+2
+# Counter Values:
+200000
+5095099
+
+main
+# Func Hash:
+303943193688
+# Num Counters:
+3
+# Counter Values:
+1
+2000
+200000
+
Index: clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
@@ -0,0 +1,36 @@
+bar
+# Func Hash:
+11262309464
+# Num Counters:
+2
+# Counter Values:
+200000
+2
+
+baz
+# Func Hash:
+24
+# Num Counters:
+1
+# Counter Values:
+2
+
+foo
+# Func Hash:
+635992
+# Num Counters:
+2
+# Counter Values:
+199998
+5096876
+
+main
+# Func Hash:
+303943193688
+# Num Counters:
+3
+# Counter Values:
+1
+2000
+200000
+
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -809,6 +809,7 @@
<< Args.getLastArg(OPT_fprofile_remapping_file_EQ)->getAsString(Args)
<< "-fexperimental-new-pass-manager";
}
+ Opts.MisExpect = Args.hasFlag(OPT_fmisexpect, OPT_fno_misexpect, false);
Opts.CoverageMapping =
Args.hasFlag(OPT_fcoverage_mapping, OPT_fno_coverage_mapping, false);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3998,6 +3998,11 @@
Args.AddLastArg(CmdArgs, options::OPT_ffine_grained_bitfield_accesses,
options::OPT_fno_fine_grained_bitfield_accesses);
+ if (Args.hasFlag(options::OPT_fmisexpect, options::OPT_fno_misexpect,
+ false)) {
+ CmdArgs.push_back("-fmisexpect");
+ }
+
// Handle segmented stacks.
if (Args.hasArg(options::OPT_fsplit_stack))
CmdArgs.push_back("-split-stacks");
Index: clang/lib/CodeGen/MisExpect.h
===================================================================
--- /dev/null
+++ clang/lib/CodeGen/MisExpect.h
@@ -0,0 +1,52 @@
+//===--- MisExpect.h - Check Use of __builtin_expect() with PGO data ------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This contains code to emit warnings for potentially incorrect usage of
+// __builtin_expect(). It uses PGO profiles to validation.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_MISEXPECT_H
+#define LLVM_CLANG_LIB_CODEGEN_MISEXPECT_H
+
+#include "CodeGenModule.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/Optional.h"
+
+namespace clang {
+namespace CodeGen {
+namespace MisExpect {
+
+/// getExpectedValue - Returns the value that __builtin_expect() is expecting.
+/// If the second parameter cannot be evaluated at compile-time, returns an
+/// empty Optional. Returns None when Call is not __builtin_expect()
+Optional<int64_t> getExpectedValue(const clang::CallExpr *Call,
+ ASTContext &Context);
+
+/// CheckMisExpectBranch - check if a branch is annotated with
+/// __builtin_expect and when using profiling data, verify that the profile
+/// agrees with the use of the annotation
+
+void CheckMisExpectBranch(const Expr *Cond, uint64_t TrueCount,
+ uint64_t CurrProfCount, CodeGenModule &CGM);
+
+/// CheckMisExpect - check if a branch is annotated with __builtin_expect and
+/// when using profiling data, verify that the profile agrees with the use of
+/// the annotation
+void CheckMisExpectSwitch(const CallExpr *Call,
+ llvm::SmallVector<uint64_t, 16> *SwitchWeights,
+ llvm::DenseMap<int64_t, size_t> *CaseMap,
+ CodeGenModule &CGM);
+
+} // namespace MisExpect
+} // namespace CodeGen
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_CODEGEN_MISEXPECT_H
Index: clang/lib/CodeGen/MisExpect.cpp
===================================================================
--- /dev/null
+++ clang/lib/CodeGen/MisExpect.cpp
@@ -0,0 +1,183 @@
+//===--- MisExpect.cpp - Check Use of __builtin_expect() with PGO data ----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This contains code to emit warnings for potentially incorrect usage of
+// __builtin_expect(). It uses PGO profiles to validation.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MisExpect.h"
+#include "CodeGenModule.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "llvm/Support/BranchProbability.h"
+
+#include <algorithm>
+#include <numeric>
+
+namespace {
+
+using namespace clang;
+using namespace clang::CodeGen;
+// Emit Warning notifiying user that the current PGO counter is a mismatch with
+// the use of __builtin_expect()
+void EmitMisExpectWarning(const clang::CallExpr *Call, CodeGenModule &CGM,
+ unsigned PercentageCorrect) {
+ SourceLocation ExprLoc = Call->getBeginLoc();
+ unsigned DiagID = CGM.getDiags().getCustomDiagID(
+ DiagnosticsEngine::Warning,
+ "Potential performance regression from use of __builtin_expect(): "
+ "Annotation was correct on %0%% of profiled executions.");
+
+ CGM.getDiags()
+ .Report(ExprLoc, DiagID)
+ .AddTaggedVal(PercentageCorrect,
+ clang::DiagnosticsEngine::ArgumentKind::ak_uint);
+}
+
+// Prints some debug diagnostics useful when checking SwitchStmts.
+// Allows for simple comparison of the Case Value mappings to their index in the
+// SwitchWeights datastructure in CGStmts.cpp
+void DebugPrintMisExpectSwitchInfo(SmallVector<uint64_t, 16> *SwitchWeights,
+ llvm::DenseMap<int64_t, size_t> *CaseMap) {
+ auto size = SwitchWeights->size();
+ for (size_t i = 0; i < size; ++i) {
+ llvm::dbgs() << "Index: " << i << "\tProfile Value:\t"
+ << (*SwitchWeights)[i] << "\n";
+ }
+
+ llvm::dbgs() << "------------------\n";
+
+ for (auto &item : *CaseMap) {
+ llvm::dbgs() << "Case Key: " << item.first << "\tRelated Index:\t"
+ << item.second << "\n";
+ }
+
+ llvm::dbgs() << "------------------\n";
+ uint64_t CaseTotal =
+ std::accumulate(SwitchWeights->begin(), SwitchWeights->end(), 0);
+ llvm::dbgs() << "Switch Profile Count:\t" << CaseTotal << "\n";
+}
+
+} // namespace
+
+namespace clang {
+namespace CodeGen {
+namespace MisExpect {
+
+#define DEBUG_TYPE "misexpect"
+Optional<int64_t> getExpectedValue(const clang::CallExpr *Call,
+ ASTContext &Context) {
+ if (!Call)
+ return Optional<int64_t>(llvm::None);
+
+ auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
+ if (!FD || FD->getBuiltinID() != Builtin::BI__builtin_expect) {
+ return Optional<int64_t>(llvm::None);
+ }
+
+ // Check if we can evaluate the 2nd parameter to __builtin_expect(expr,long)
+ // since it may not be able to be evaluated at compile-time
+ Expr::EvalResult ExprResult;
+ auto Arg = Call->getArg(1);
+ Arg->EvaluateAsInt(ExprResult, Context, Expr::SE_AllowSideEffects);
+
+ if (!ExprResult.Val.hasValue())
+ return Optional<int64_t>(llvm::None);
+
+ llvm::APSInt &Into = ExprResult.Val.getInt();
+ int64_t ExpectedVal = Into.getExtValue();
+ return ExpectedVal;
+}
+
+void CheckMisExpectBranch(const Expr *Cond, uint64_t TrueCount,
+ uint64_t CurrProfCount, CodeGenModule &CGM) {
+ if (!CGM.getCodeGenOpts().MisExpect ||
+ CGM.getCodeGenOpts().getProfileUse() == CodeGenOptions::ProfileNone)
+ return;
+
+ auto *Call = dyn_cast<CallExpr>(Cond->IgnoreImpCasts());
+
+ Optional<int64_t> ExpectedValOpt = getExpectedValue(Call, CGM.getContext());
+
+ if (!ExpectedValOpt.hasValue())
+ return;
+
+ const long ExpectedVal = ExpectedValOpt.getValue();
+ const bool ExpectedTrueBranch = (ExpectedVal != 0);
+
+ LLVM_DEBUG(llvm::dbgs() << "Expected Value:\t" << ExpectedVal << "\n");
+ LLVM_DEBUG(llvm::dbgs() << "Current Count:\t" << CurrProfCount << "\n");
+ LLVM_DEBUG(llvm::dbgs() << "True Count:\t" << TrueCount << "\n");
+
+ bool IncorrectPerfCounters = false;
+ uint64_t Scaled;
+ unsigned Percentage;
+
+ // TODO: determine better heuristics than hot/cold function thresholds
+ // Currenlty {Hot|Cold}FunctionThreshold is taken from
+ // PGOInstrumentation.cpp:1084
+ if (ExpectedTrueBranch) {
+ const llvm::BranchProbability HotFunctionThreshold(1, 100);
+ Scaled = HotFunctionThreshold.scale(CurrProfCount);
+ Percentage = (TrueCount / (float)CurrProfCount) * 100;
+ if (TrueCount < Scaled)
+ IncorrectPerfCounters = true;
+ } else {
+ const llvm::BranchProbability ColdFunctionThreshold(2, 10000);
+ Scaled = ColdFunctionThreshold.scale(CurrProfCount);
+ Percentage = ((CurrProfCount - TrueCount) / (float)CurrProfCount) * 100;
+ if (TrueCount > Scaled)
+ IncorrectPerfCounters = true;
+ }
+ LLVM_DEBUG(llvm::dbgs() << "Scaled Count:\t" << Scaled << "\n");
+
+ if (IncorrectPerfCounters)
+ EmitMisExpectWarning(Call, CGM, Percentage);
+}
+
+void CheckMisExpectSwitch(const CallExpr *Call,
+ SmallVector<uint64_t, 16> *SwitchWeights,
+ llvm::DenseMap<int64_t, size_t> *CaseMap,
+ CodeGenModule &CGM) {
+ if (!SwitchWeights || !CaseMap)
+ return;
+
+ Optional<int64_t> ExpectedValOpt = getExpectedValue(Call, CGM.getContext());
+
+ if (!ExpectedValOpt.hasValue())
+ return;
+
+ LLVM_DEBUG(DebugPrintMisExpectSwitchInfo(SwitchWeights, CaseMap));
+ const long ExpectedVal = ExpectedValOpt.getValue();
+
+ LLVM_DEBUG(llvm::dbgs() << "Expected Value:\t" << ExpectedVal << "\n");
+
+ uint64_t Max =
+ *std::max_element(SwitchWeights->begin(), SwitchWeights->end());
+
+ auto MapIndex = CaseMap->find(ExpectedVal);
+
+ uint64_t Index = (MapIndex != CaseMap->end()) ? MapIndex->second : 0;
+ uint64_t TakenCount = (*SwitchWeights)[Index];
+
+ LLVM_DEBUG(llvm::dbgs() << "Taken Count:\t" << TakenCount << "\n");
+ LLVM_DEBUG(llvm::dbgs() << "Max Count:\t" << Max << "\n");
+ uint64_t CaseTotal =
+ std::accumulate(SwitchWeights->begin(), SwitchWeights->end(), 0);
+ unsigned Percentage = ((float)TakenCount / (float)CaseTotal) * 100;
+
+ if (TakenCount < Max)
+ EmitMisExpectWarning(Call, CGM, Percentage);
+}
+#undef DEBUG_TYPE
+
+} // namespace MisExpect
+} // namespace CodeGen
+} // namespace clang
Index: clang/lib/CodeGen/CodeGenFunction.h
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -1370,6 +1370,8 @@
llvm::SwitchInst *SwitchInsn = nullptr;
/// The branch weights of SwitchInsn when doing instrumentation based PGO.
SmallVector<uint64_t, 16> *SwitchWeights = nullptr;
+ llvm::DenseMap<int64_t, size_t> *CaseMap = nullptr;
+
/// CaseRangeBlock - This block holds if condition check for last case
/// statement range in current switch instruction.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -20,6 +20,7 @@
#include "CodeGenModule.h"
#include "CodeGenPGO.h"
#include "TargetInfo.h"
+#include "MisExpect.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/Decl.h"
@@ -1346,8 +1347,6 @@
return true;
}
-
-
/// EmitBranchOnBoolExpr - Emit a branch on a boolean condition (e.g. for an if
/// statement) to the specified blocks. Based on the condition, this might try
/// to simplify the codegen of the conditional based on the branch.
@@ -1358,6 +1357,8 @@
uint64_t TrueCount) {
Cond = Cond->IgnoreParens();
+ MisExpect::CheckMisExpectBranch(Cond, TrueCount, getCurrentProfileCount(), CGM);
+
if (const BinaryOperator *CondBOp = dyn_cast<BinaryOperator>(Cond)) {
// Handle X && Y in a condition.
Index: clang/lib/CodeGen/CMakeLists.txt
===================================================================
--- clang/lib/CodeGen/CMakeLists.txt
+++ clang/lib/CodeGen/CMakeLists.txt
@@ -87,6 +87,7 @@
ItaniumCXXABI.cpp
MacroPPCallbacks.cpp
MicrosoftCXXABI.cpp
+ MisExpect.cpp
ModuleBuilder.cpp
ObjectFilePCHContainerOperations.cpp
PatternInit.cpp
Index: clang/lib/CodeGen/CGStmt.cpp
===================================================================
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -14,6 +14,7 @@
#include "CGDebugInfo.h"
#include "CodeGenModule.h"
#include "TargetInfo.h"
+#include "MisExpect.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/PrettyStackTrace.h"
@@ -1284,8 +1285,16 @@
llvm::BasicBlock *CaseDest = createBasicBlock("sw.bb");
EmitBlockWithFallThrough(CaseDest, &S);
- if (SwitchWeights)
+ size_t MapIndex = 0;
+ if (SwitchWeights) {
SwitchWeights->push_back(getProfileCount(&S));
+ if (CaseMap) {
+ MapIndex = SwitchWeights->size() - 1;
+ llvm::ConstantInt *CaseVal =
+ Builder.getInt(S.getLHS()->EvaluateKnownConstInt(getContext()));
+ (*CaseMap)[CaseVal->getSExtValue()] = MapIndex;
+ }
+ }
SwitchInsn->addCase(CaseVal, CaseDest);
// Recursively emitting the statement is acceptable, but is not wonderful for
@@ -1306,8 +1315,15 @@
llvm::ConstantInt *CaseVal =
Builder.getInt(CurCase->getLHS()->EvaluateKnownConstInt(getContext()));
- if (SwitchWeights)
+ if (SwitchWeights) {
SwitchWeights->push_back(getProfileCount(NextCase));
+ if (CaseMap) {
+ // only the original statement gets a counter, so map back to it's index
+ // in SwitchWeights
+ (*CaseMap)[CaseVal->getSExtValue()] = MapIndex;
+ }
+ }
+
if (CGM.getCodeGenOpts().hasProfileClangInstr()) {
CaseDest = createBasicBlock("sw.bb");
EmitBlockWithFallThrough(CaseDest, &S);
@@ -1575,6 +1591,7 @@
// Handle nested switch statements.
llvm::SwitchInst *SavedSwitchInsn = SwitchInsn;
SmallVector<uint64_t, 16> *SavedSwitchWeights = SwitchWeights;
+ llvm::DenseMap<int64_t, size_t> *SavedCaseMap = CaseMap;
llvm::BasicBlock *SavedCRBlock = CaseRangeBlock;
// See if we can constant fold the condition of the switch and therefore only
@@ -1646,6 +1663,8 @@
}
SwitchWeights = new SmallVector<uint64_t, 16>();
SwitchWeights->reserve(NumCases);
+ CaseMap = new llvm::DenseMap<int64_t, size_t>();
+ CaseMap->reserve(NumCases);
// The default needs to be first. We store the edge count, so we already
// know the right weight.
SwitchWeights->push_back(DefaultCount);
@@ -1698,10 +1717,15 @@
auto *Call = dyn_cast<CallExpr>(S.getCond());
if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) {
auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
- if (FD && FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) {
- llvm::MDBuilder MDHelper(getLLVMContext());
- SwitchInsn->setMetadata(llvm::LLVMContext::MD_unpredictable,
- MDHelper.createUnpredictable());
+ if (FD) {
+ if (FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) {
+ llvm::MDBuilder MDHelper(getLLVMContext());
+ SwitchInsn->setMetadata(llvm::LLVMContext::MD_unpredictable,
+ MDHelper.createUnpredictable());
+ } else if (CGM.getCodeGenOpts().MisExpect &&
+ FD->getBuiltinID() == Builtin::BI__builtin_expect) {
+ MisExpect::CheckMisExpectSwitch(Call, SwitchWeights, CaseMap, CGM);
+ }
}
}
@@ -1714,8 +1738,14 @@
createProfileWeights(*SwitchWeights));
delete SwitchWeights;
}
+
+ if (CaseMap) {
+ delete CaseMap;
+ }
+
SwitchInsn = SavedSwitchInsn;
SwitchWeights = SavedSwitchWeights;
+ CaseMap = SavedCaseMap;
CaseRangeBlock = SavedCRBlock;
}
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -716,6 +716,11 @@
Group<f_Group>, Alias<fprofile_sample_accurate>;
def fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">,
Group<f_Group>, Alias<fno_profile_sample_accurate>;
+def fmisexpect : Flag<["-"], "fmisexpect">,
+ Group<f_Group>, Flags<[CC1Option]>,
+ HelpText<"Validate use of __builtin_expect with instrumentation data">;
+def fno_misexpect : Flag<["-"], "fno-misexpect">,
+ Group<f_Group>, Flags<[CC1Option]>;
def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
Group<f_Group>, Flags<[CC1Option, CC1AsOption, CoreOption]>,
HelpText<"The compilation directory to embed in the debug info.">;
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -169,6 +169,7 @@
///< enable code coverage analysis.
CODEGENOPT(DumpCoverageMapping , 1, 0) ///< Dump the generated coverage mapping
///< regions.
+CODEGENOPT(MisExpect , 1, 0) ///< Validate __builtin_expect with PGO counters
/// If -fpcc-struct-return or -freg-struct-return is specified.
ENUM_CODEGENOPT(StructReturnConvention, StructReturnConventionKind, 2, SRCK_Default)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
