From a10802fd73f937cab8cfd448d9e9ff283f75b191 Mon Sep 17 00:00:00 2001 From: Petr Hosek Date: Tue, 10 Sep 2019 03:11:39 +0000 Subject: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM This patch contains the basic functionality for reporting potentially incorrect usage of __builtin_expect() by comparing the developer's annotation against a collected PGO profile. A more detailed proposal and discussion appears on the CFE-dev mailing list (http://lists.llvm.org/pipermail/cfe-dev/2019-July/062971.html) and a prototype of the initial frontend changes appear here in D65300 We revised the work in D65300 by moving the misexpect check into the LLVM backend, and adding support for IR and sampling based profiles, in addition to frontend instrumentation. We add new misexpect metadata tags to those instructions directly influenced by the llvm.expect intrinsic (branch, switch, and select) when lowering the intrinsics. The misexpect metadata contains information about the expected target of the intrinsic so that we can check against the correct PGO counter when emitting diagnostics, and the compiler's values for the LikelyBranchWeight and UnlikelyBranchWeight. We use these branch weight values to determine when to emit the diagnostic to the user. A future patch should address the comment at the top of LowerExpectIntrisic.cpp to hoist the LikelyBranchWeight and UnlikelyBranchWeight values into a shared space that can be accessed outside of the LowerExpectIntrinsic pass. Once that is done, the misexpect metadata can be updated to be smaller. In the long term, it is possible to reconstruct portions of the misexpect metadata from the existing profile data. However, we have avoided this to keep the code simple, and because some kind of metadata tag will be required to identify which branch/switch/select instructions are influenced by the use of llvm.expect Patch By: paulkirth Differential Revision: https://reviews.llvm.org/D66324 llvm-svn: 371484 --- clang/lib/CodeGen/CodeGenAction.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'clang/lib/CodeGen/CodeGenAction.cpp') diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 2356f84..87bda4a 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -14,6 +14,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclGroup.h" +#include "clang/Basic/DiagnosticFrontend.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangStandard.h" #include "clang/Basic/SourceManager.h" @@ -365,6 +366,9 @@ namespace clang { bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D); /// Specialized handler for unsupported backend feature diagnostic. void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D); + /// Specialized handler for misexpect warnings. + /// Note that misexpect remarks are emitted through ORE + void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect &D); /// Specialized handlers for optimization remarks. /// Note that these handlers only accept remarks and they always handle /// them. @@ -617,6 +621,25 @@ void BackendConsumer::UnsupportedDiagHandler( << Filename << Line << Column; } +void BackendConsumer::MisExpectDiagHandler( + const llvm::DiagnosticInfoMisExpect &D) { + StringRef Filename; + unsigned Line, Column; + bool BadDebugInfo = false; + FullSourceLoc Loc = + getBestLocationFromDebugLoc(D, BadDebugInfo, Filename, Line, Column); + + Diags.Report(Loc, diag::warn_profile_data_misexpect) << D.getMsg().str(); + + if (BadDebugInfo) + // If we were not able to translate the file:line:col information + // back to a SourceLocation, at least emit a note stating that + // we could not translate this location. This can happen in the + // case of #line directives. + Diags.Report(Loc, diag::note_fe_backend_invalid_loc) + << Filename << Line << Column; +} + void BackendConsumer::EmitOptimizationMessage( const llvm::DiagnosticInfoOptimizationBase &D, unsigned DiagID) { // We only support warnings and remarks. @@ -787,6 +810,9 @@ void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) { case llvm::DK_Unsupported: UnsupportedDiagHandler(cast(DI)); return; + case llvm::DK_MisExpect: + MisExpectDiagHandler(cast(DI)); + return; default: // Plugin IDs are not bound to any value as they are set dynamically. ComputeDiagRemarkID(Severity, backend_plugin, DiagID); -- cgit v1.1