paulkirth updated this revision to Diff 211853.
paulkirth added a comment.
Add missing test for switch statements when the expected value is not a compile
time constant.
Make sure that when the expected value cannot be evaluated, we do not issue any
warnings or errors
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.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.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 {{Current PGO counters disagree with the use of __builtin_expect().}}
+ 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-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 {{Current PGO counters disagree with the use of __builtin_expect().}}
+ 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 {{Current PGO counters disagree with the use of __builtin_expect()}}
+ 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-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 - Emit LLVM Code from Statements ---------------------===//
+//
+// 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
+/// __builting_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 __builting_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,176 @@
+//===--- MisExpect.cpp - Emit LLVM Code from Statements -------------------===//
+//
+// 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 "llvm/Support/BranchProbability.h"
+
+#include <algorithm>
+#include <numeric>
+
+namespace clang {
+namespace CodeGen {
+namespace MisExpect {
+
+#define DEBUG_TYPE "misexpect"
+
+void DebugPrintMisExpectSwitchInfo(SmallVector<uint64_t, 16> *SwitchWeights,
+ llvm::DenseMap<int64_t, size_t> *CaseMap);
+void EmitMisExpectWarning(const CallExpr *Call, CodeGenModule &CGM);
+
+Optional<int64_t> getExpectedValue(const clang::CallExpr *Call,
+ ASTContext &Context) {
+ if (!Call)
+ return Optional<int64_t>(None);
+
+ auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl());
+ if (!FD || FD->getBuiltinID() != Builtin::BI__builtin_expect) {
+ return Optional<int64_t>(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>(None);
+
+ llvm::APSInt &Into = ExprResult.Val.getInt();
+ int64_t ExpectedVal = Into.getExtValue();
+ return Optional<int64_t>(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;
+
+ // TODO: determine better heuristics than hot/cold function thresholds
+ if (ExpectedTrueBranch) {
+ const llvm::BranchProbability HotFunctionThreshold(1, 100);
+ Scaled = HotFunctionThreshold.scale(CurrProfCount);
+ if (TrueCount < Scaled)
+ IncorrectPerfCounters = true;
+ } else {
+ const llvm::BranchProbability ColdFunctionThreshold(2, 10000);
+ Scaled = ColdFunctionThreshold.scale(CurrProfCount);
+ if (TrueCount > Scaled)
+ IncorrectPerfCounters = true;
+ }
+ LLVM_DEBUG(llvm::dbgs() << "Scaled Count:\t" << Scaled << "\n");
+
+ if (IncorrectPerfCounters) {
+ EmitMisExpectWarning(Call, CGM);
+ }
+}
+
+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;
+ if (MapIndex != CaseMap->end()) {
+ Index = MapIndex->second;
+ } else {
+ Index = 0;
+ }
+
+ uint64_t TakenCount = (*SwitchWeights)[Index];
+
+ LLVM_DEBUG(llvm::dbgs() << "Taken Count:\t" << TakenCount << "\n");
+ LLVM_DEBUG(llvm::dbgs() << "Max Count:\t" << Max << "\n");
+
+ if (TakenCount < Max) {
+ EmitMisExpectWarning(Call, CGM);
+ }
+}
+
+// Emit Warning notifiying user that the current PGO counter is a mismatch with
+// the use of __builtin_expect()
+void EmitMisExpectWarning(const CallExpr *Call, CodeGenModule &CGM) {
+ SourceLocation ExprLoc = Call->getBeginLoc();
+ unsigned DiagID = CGM.getDiags().getCustomDiagID(
+ DiagnosticsEngine::Warning, "Current PGO counters disagree with "
+ "the use of __builtin_expect().");
+ CGM.getDiags().Report(ExprLoc, DiagID);
+}
+
+// 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";
+}
+
+#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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
