paulkirth updated this revision to Diff 213725.
paulkirth added a comment.
Use existing LLVM code for mapping case literals to their case arms.
This update refactors a great deal of the implementation and test code.
1. Removes the CaseMap data structure completely. LLVM already maintains a
mapping of the constants to their case arm, so there is no reason to duplicate
that logic. This also minimizes the changes impacting existing Clang/LLVM
components.
2. Cleans up debug printing. Without the CaseMap printing debug output can be
simplified.
3. Minimizes the code in the end-to-end tests & test profiles.
4. Improves formatting, white space, and comments.
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/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,41 @@
+// 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();
+void init_arry();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+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:
+ break;
+ default:
+ val += random_sample(arry, arry_size);
+ break;
+ } // end switch
+ } // end inner_loop
+ } // end outer_loop
+
+ return 0;
+}
Index: clang/test/Profile/misexpect-switch-only-default-case.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch-only-default-case.c
@@ -0,0 +1,35 @@
+// Test that misexpect emits no warning when there is only one switch case
+
+// 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();
+void init_arry();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+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;
+}
Index: clang/test/Profile/misexpect-switch-nonconst.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch-nonconst.c
@@ -0,0 +1,43 @@
+// Test that misexpect emits no warning when switch condition is non-const
+
+// 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();
+void init_arry();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+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;
+}
Index: clang/test/Profile/misexpect-switch-default.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-switch-default.c
@@ -0,0 +1,42 @@
+// Test that misexpect detects mis-annotated switch statements for default case
+
+// 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();
+void init_arry();
+
+const int inner_loop = 1000;
+const int outer_loop = 20;
+const int arry_size = 25;
+
+int arry[arry_size] = {0};
+
+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;
+}
Index: clang/test/Profile/misexpect-no-warning-without-flag.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-no-warning-without-flag.c
@@ -0,0 +1,27 @@
+// Test that misexpect emits no warning without -fmisexpect flag
+
+// 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 baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+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;
+}
+
Index: clang/test/Profile/misexpect-branch.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-branch.c
@@ -0,0 +1,26 @@
+// 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 baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+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;
+}
+
Index: clang/test/Profile/misexpect-branch-nonconst-expected-val.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-branch-nonconst-expected-val.c
@@ -0,0 +1,26 @@
+// Test that misexpect emits no warning when condition is not a compile-time constant
+
+// 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 baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+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;
+}
Index: clang/test/Profile/misexpect-branch-cold.c
===================================================================
--- /dev/null
+++ clang/test/Profile/misexpect-branch-cold.c
@@ -0,0 +1,27 @@
+// Test that misexpect emits no warning when prediction is correct
+
+// 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 baz(int);
+int buzz();
+
+const int inner_loop = 100;
+const int outer_loop = 2000;
+
+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;
+}
+
Index: clang/test/Profile/Inputs/misexpect-switch.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-switch.proftext
@@ -0,0 +1,16 @@
+main
+# Func Hash:
+1965403898329309329
+# Num Counters:
+9
+# Counter Values:
+1
+20
+20000
+20000
+12
+26
+0
+0
+19962
+
Index: clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
@@ -0,0 +1,12 @@
+main
+# Func Hash:
+79676873694057560
+# Num Counters:
+5
+# Counter Values:
+1
+20
+20000
+20000
+20000
+
Index: clang/test/Profile/Inputs/misexpect-branch.proftext
===================================================================
--- /dev/null
+++ clang/test/Profile/Inputs/misexpect-branch.proftext
@@ -0,0 +1,9 @@
+bar
+# Func Hash:
+45795613684824
+# Num Counters:
+2
+# Counter Values:
+200000
+0
+
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,9 @@
+bar
+# Func Hash:
+11262309464
+# Num Counters:
+2
+# Counter Values:
+200000
+2
+
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,9 @@
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,55 @@
+//===--- 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 for 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 {
+
+/// 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
+/// \param Cond the conditional expression being checked
+/// \param TrueCount the profile counter for this block
+/// \param CurrProfCount the current total profile count
+/// \param CGM a reference to the current CodeGenModule
+void CheckMisExpectBranch(const Expr *Cond, const llvm::BranchInst *BI,
+ 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
+/// \param Call the call expression to __builtin_expect()
+/// \param SwitchWeights pointer to a vector of profile counts for each case arm
+/// \param CaseMap a table mapping the constant value of a case target to its
+/// index in the SwitchWeights vector
+/// \param CGM a reference to the current CodeGenModule
+void CheckMisExpectSwitch(const CallExpr *Call,
+ llvm::SwitchInst *SwitchInstruction,
+ llvm::SmallVector<uint64_t, 16> *SwitchWeights,
+ 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,236 @@
+//===--- 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 for validation.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MisExpect.h"
+#include "CodeGenModule.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/Support/BranchProbability.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
+
+#include <algorithm>
+#include <bits/stdint-intn.h>
+#include <bits/stdint-uintn.h>
+#include <numeric>
+
+namespace {
+
+using namespace clang;
+using namespace clang::CodeGen;
+
+struct SwitchDebugInfo {
+ SmallVector<uint64_t, 16> *SwitchWeights;
+ long ExpectedVal;
+ uint64_t Index;
+ uint64_t TakenCount;
+ uint64_t Max;
+ uint64_t ScaledThreshold;
+ float Percentage;
+};
+
+// Emit Warning notifying user that the current PGO counter is a mismatch with
+// the use of __builtin_expect()
+// \param PercentageCorrect the percentage the expected target of
+// __builtin_expect() was taken during profiling as an integer
+void EmitMisExpectWarning(const clang::CallExpr *Call, CodeGenModule &CGM,
+ float 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.");
+
+ auto PercentStr = llvm::formatv("{0:P}", PercentageCorrect);
+ CGM.getDiags().Report(ExprLoc, DiagID).AddString(PercentStr.str());
+}
+
+// Prints some debug diagnostics useful when checking SwitchStmts.
+// Allows for simple comparison of the Case Value mappings to their index in the
+// SwitchWeights data structure in CGStmts.cpp
+void DebugPrintMisExpectSwitchInfo(SwitchDebugInfo SDI) {
+ auto size = SDI.SwitchWeights->size();
+ llvm::dbgs() << "------------------\n";
+ for (size_t i = 0; i < size; ++i) {
+ llvm::dbgs() << "Index: " << i << "\tProfile Value:\t"
+ << (*SDI.SwitchWeights)[i] << "\n";
+ }
+
+ uint64_t CaseTotal =
+ std::accumulate(SDI.SwitchWeights->begin(), SDI.SwitchWeights->end(), 0);
+ llvm::dbgs() << "Profile Count:\t" << CaseTotal << "\n";
+ llvm::dbgs() << "Expected Value:\t" << SDI.ExpectedVal << "\n";
+ llvm::dbgs() << "Expected Index:\t" << SDI.Index << "\n";
+ llvm::dbgs() << "Taken Count:\t" << SDI.TakenCount << "\n";
+ llvm::dbgs() << "Max Count:\t" << SDI.Max << "\n";
+ llvm::dbgs() << "Threshold:\t" << SDI.ScaledThreshold << "\n";
+ llvm::dbgs() << llvm::formatv("Ratio: {0}/{1} = {2:P}\n", SDI.TakenCount,
+ CaseTotal, SDI.Percentage);
+ llvm::dbgs() << "------------------\n";
+}
+
+// getExpectedValue - Returns the value that __builtin_expect() is expecting.
+// \param BI the branch instruction being checked
+// \return None if second parameter to __builtin_expect() cannot be evaluated at
+// compile-time, else returns an empty Optional.
+template <class BrSwtchInst>
+llvm::Optional<llvm::ConstantInt *> getExpectedValue(const BrSwtchInst *BI) {
+ // TODO: consider moving funciton into a support lib to improve code reuse
+ if (!BI)
+ return llvm::Optional<llvm::ConstantInt *>(llvm::None);
+
+ llvm::CallInst *CI = nullptr;
+ llvm::ICmpInst *CmpI = dyn_cast<llvm::ICmpInst>(BI->getCondition());
+ llvm::ConstantInt *CmpConstOperand = nullptr;
+
+ if (!CmpI) {
+ CI = dyn_cast<llvm::CallInst>(BI->getCondition());
+ } else {
+ CmpConstOperand = dyn_cast<llvm::ConstantInt>(CmpI->getOperand(1));
+ if (!CmpConstOperand)
+ return llvm::Optional<llvm::ConstantInt *>(llvm::None);
+ CI = dyn_cast<llvm::CallInst>(CmpI->getOperand(0));
+ }
+
+ if (!CI)
+ return llvm::Optional<llvm::ConstantInt *>(llvm::None);
+
+ llvm::Function *Fn = CI->getCalledFunction();
+ if (!Fn || Fn->getIntrinsicID() != llvm::Intrinsic::expect)
+ return Optional<llvm::ConstantInt *>(llvm::None);
+
+ llvm::ConstantInt *ExpectedValue =
+ dyn_cast<llvm::ConstantInt>(CI->getArgOperand(1));
+
+ if (!ExpectedValue)
+ return Optional<llvm::ConstantInt *>(llvm::None);
+
+ return ExpectedValue;
+}
+
+} // namespace
+
+namespace clang {
+namespace CodeGen {
+namespace MisExpect {
+
+// TODO: see LowerExpectIntrinsic.cpp for notes on sharing these constants
+const uint32_t LikelyBranchWeight = 2000;
+const uint32_t UnlikelyBranchWeight = 1;
+
+#define DEBUG_TYPE "misexpect"
+
+void CheckMisExpectBranch(const Expr *Cond, const llvm::BranchInst *BI,
+ 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());
+ auto Exp = getExpectedValue(BI);
+
+ if (!Call || !Exp.hasValue())
+ return;
+
+ const long ExpectedVal = Exp.getValue()->getZExtValue();
+ const bool ExpectedTrueBranch = (ExpectedVal != 0);
+ bool IncorrectPerfCounters = false;
+ uint64_t Scaled;
+ float Percentage;
+ const uint64_t TotalBranchWeight = LikelyBranchWeight + UnlikelyBranchWeight;
+
+ // TODO: determine better heuristics than hot/cold function thresholds
+ // LowerExpectIntrinsics.cpp:49 LikelyBranchWeight = 2000
+ // LowerExpectIntrinsics.cpp:52 UnlikelyBranchWeight = 1
+ if (ExpectedTrueBranch) {
+ const llvm::BranchProbability LikelyThreshold(LikelyBranchWeight,
+ TotalBranchWeight);
+ Scaled = LikelyThreshold.scale(CurrProfCount);
+ Percentage = (TrueCount / (float)CurrProfCount) * 100;
+ if (TrueCount < Scaled)
+ IncorrectPerfCounters = true;
+ } else {
+ const llvm::BranchProbability UnlikelyThreshold(UnlikelyBranchWeight,
+ LikelyBranchWeight);
+ Scaled = UnlikelyThreshold.scale(CurrProfCount);
+ Percentage = ((CurrProfCount - TrueCount) / (float)CurrProfCount);
+ if (TrueCount > Scaled)
+ IncorrectPerfCounters = true;
+ }
+
+ LLVM_DEBUG(llvm::dbgs() << "------------------\n");
+ 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");
+ LLVM_DEBUG(llvm::dbgs() << "Scaled Count:\t" << Scaled << "\n");
+ LLVM_DEBUG(llvm::dbgs() << "------------------\n");
+
+ if (IncorrectPerfCounters)
+ EmitMisExpectWarning(Call, CGM, Percentage);
+}
+
+void CheckMisExpectSwitch(const CallExpr *Call, llvm::SwitchInst *SI,
+ SmallVector<uint64_t, 16> *SwitchWeights,
+ CodeGenModule &CGM) {
+ if (!SwitchWeights)
+ return;
+
+ Optional<llvm::ConstantInt *> ExpectedValOpt = getExpectedValue(SI);
+
+ if (!ExpectedValOpt.hasValue())
+ return;
+
+ llvm::ConstantInt *ExpectedValue = ExpectedValOpt.getValue();
+
+ llvm::SwitchInst::CaseHandle Case = *SI->findCaseValue(ExpectedValue);
+ unsigned n = SI->getNumCases(); // +1 for default case.
+
+ const long ExpectedVal = ExpectedValOpt.getValue()->getZExtValue();
+
+ uint64_t Max =
+ *std::max_element(SwitchWeights->begin(), SwitchWeights->end());
+
+ // The default case is always mapped to index 0 of the SwitchWeights vector.
+ // This relies on internal details of another component, so ideally we can
+ // expose an interface that we can use instead of relying on implementaion
+ // details in another module.
+ // TODO: create interface to switchweights default index
+ uint64_t Index = (Case == *SI->case_default()) ? 0 : Case.getCaseIndex() + 1;
+ uint64_t TakenCount = (*SwitchWeights)[Index];
+
+ uint64_t CaseTotal =
+ std::accumulate(SwitchWeights->begin(), SwitchWeights->end(), 0);
+ const uint64_t TotalBranchWeight =
+ LikelyBranchWeight + (UnlikelyBranchWeight * n);
+ float Percentage = ((float)TakenCount / (float)CaseTotal);
+ const llvm::BranchProbability LikelyThreshold(LikelyBranchWeight,
+ TotalBranchWeight);
+ auto ScaledThreshold = LikelyThreshold.scale(CaseTotal);
+
+ LLVM_DEBUG(DebugPrintMisExpectSwitchInfo({SwitchWeights, ExpectedVal, Index,
+ TakenCount, Max, ScaledThreshold,
+ Percentage}));
+
+ if (TakenCount < ScaledThreshold)
+ EmitMisExpectWarning(Call, CGM, Percentage);
+}
+#undef DEBUG_TYPE
+
+} // namespace MisExpect
+} // namespace CodeGen
+} // namespace clang
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"
@@ -33,6 +34,7 @@
#include "clang/Frontend/FrontendDiagnostic.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Dominators.h"
+#include "llvm/IR/Instructions.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/Operator.h"
@@ -1346,8 +1348,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.
@@ -1544,7 +1544,9 @@
ApplyDebugLocation DL(*this, Cond);
CondV = EvaluateExprAsBool(Cond);
}
- Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights, Unpredictable);
+ llvm::BranchInst *BI = Builder.CreateCondBr(CondV, TrueBlock, FalseBlock,
+ Weights, Unpredictable);
+ MisExpect::CheckMisExpectBranch(Cond, BI, TrueCount, CurrentCount, CGM);
}
/// ErrorUnsupported - Print out an error that codegen doesn't support the
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"
@@ -1698,10 +1699,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, SwitchInsn, SwitchWeights, CGM);
+ }
}
}
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
