diff --git a/clang-tools-extra/CMakeLists.txt b/clang-tools-extra/CMakeLists.txt --- a/clang-tools-extra/CMakeLists.txt +++ b/clang-tools-extra/CMakeLists.txt @@ -5,6 +5,7 @@ add_subdirectory(modularize) add_subdirectory(clang-tidy) add_subdirectory(clang-tidy-vs) +add_subdirectory(clang-misexpect) add_subdirectory(clang-change-namespace) add_subdirectory(clang-doc) diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/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) diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -275,6 +275,10 @@ def warn_profile_data_unprofiled : Warning< "no profile data available for file \"%0\"">, InGroup; +def warn_profile_data_misexpect : Warning< + "Potential performance regression from use of __builtin_expect(): " + "Annotation was correct on %0 of profiled executions.">, + InGroup; } // end of instrumentation issue category diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -1031,6 +1031,7 @@ def ProfileInstrMissing : DiagGroup<"profile-instr-missing">; def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">; def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">; +def MisExpect : DiagGroup<"misexpect">; // AddressSanitizer frontend instrumentation remarks. def SanitizeAddressRemarks : DiagGroup<"sanitize-address">; diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -716,6 +716,11 @@ Group, Alias; def fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">, Group, Alias; +def fmisexpect : Flag<["-"], "fmisexpect">, + Group, Flags<[CC1Option]>, + HelpText<"Validate use of __builtin_expect with instrumentation data">; +def fno_misexpect : Flag<["-"], "fno-misexpect">, + Group, Flags<[CC1Option]>; def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">, Group, Flags<[CC1Option, CC1AsOption, CoreOption]>, HelpText<"The compilation directory to embed in the debug info.">; diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/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(S.getCond()); if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) { auto *FD = dyn_cast_or_null(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); + } } } diff --git a/clang/lib/CodeGen/CMakeLists.txt b/clang/lib/CodeGen/CMakeLists.txt --- a/clang/lib/CodeGen/CMakeLists.txt +++ b/clang/lib/CodeGen/CMakeLists.txt @@ -87,6 +87,7 @@ ItaniumCXXABI.cpp MacroPPCallbacks.cpp MicrosoftCXXABI.cpp + MisExpect.cpp ModuleBuilder.cpp ObjectFilePCHContainerOperations.cpp PatternInit.cpp diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp --- 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" @@ -363,6 +364,7 @@ bool StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D); /// Specialized handler for unsupported backend feature diagnostic. void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported &D); + void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect &D); /// Specialized handlers for optimization remarks. /// Note that these handlers only accept remarks and they always handle /// them. @@ -615,6 +617,25 @@ << 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. @@ -785,6 +806,9 @@ 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); diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/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" @@ -1360,8 +1362,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. @@ -1558,7 +1558,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 diff --git a/clang/lib/CodeGen/MisExpect.h b/clang/lib/CodeGen/MisExpect.h new file mode 100644 --- /dev/null +++ b/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 *SwitchWeights, + CodeGenModule &CGM); + +} // namespace MisExpect +} // namespace CodeGen +} // namespace clang + +#endif // LLVM_CLANG_LIB_CODEGEN_MISEXPECT_H diff --git a/clang/lib/CodeGen/MisExpect.cpp b/clang/lib/CodeGen/MisExpect.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/CodeGen/MisExpect.cpp @@ -0,0 +1,234 @@ +//===--- 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 "clang/Basic/DiagnosticFrontend.h" +#include "llvm/ADT/Optional.h" +#include "llvm/IR/Constants.h" +#include "llvm/IR/Instructions.h" +#include "llvm/Support/BranchProbability.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Transforms/Scalar/LowerExpectIntrinsic.h" + +#include +#include +#include +#include + +#define DEBUG_TYPE "misexpect" +namespace { + +using namespace clang; +using namespace clang::CodeGen; + +struct SwitchDebugInfo { + SmallVector *SwitchWeights; + uint64_t ExpectedVal; + uint64_t Index; + uint64_t TakenCount; + uint64_t ScaledThreshold; + double 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, + double PercentageCorrect) { + SourceLocation ExprLoc = Call->getBeginLoc(); + auto PercentStr = llvm::formatv("{0:P}", PercentageCorrect).str(); + CGM.getDiags().Report(ExprLoc, diag::warn_profile_data_misexpect) + << PercentStr; +} + +// 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) { + assert(SDI.SwitchWeights && "Null pointer in DebugPrintMisExpectSwitchInfo"); + LLVM_DEBUG(auto &OS = llvm::dbgs(); + uint64_t Max = *std::max_element(SDI.SwitchWeights->begin(), + SDI.SwitchWeights->end()); + auto size = SDI.SwitchWeights->size(); + OS << "------------------\n"; + for (size_t i = 0; i < size; ++i) { + OS << "Index: " << i << "\tProfile Value:\t" + << (*SDI.SwitchWeights)[i] << "\n"; + } + + uint64_t CaseTotal = std::accumulate(SDI.SwitchWeights->begin(), + SDI.SwitchWeights->end(), 0); + OS << "Profile Count:\t" << CaseTotal << "\n"; + OS << "Expected Value:\t" << SDI.ExpectedVal << "\n"; + OS << "Expected Index:\t" << SDI.Index << "\n"; + OS << "Taken Count:\t" << SDI.TakenCount << "\n"; + OS << "Max Count:\t" << Max << "\n"; + OS << "Threshold:\t" << SDI.ScaledThreshold << "\n"; + OS << llvm::formatv("Ratio: {0}/{1} = {2:P}\n", SDI.TakenCount, + CaseTotal, SDI.Percentage); + OS << "------------------\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 +llvm::Optional getExpectedValue(const BrSwtchInst *BI) { + // TODO: consider moving funciton into a support lib to improve code reuse + if (!BI) + return llvm::None; + + const llvm::CallInst *CI; + if (const llvm::ICmpInst *CmpI = + dyn_cast(BI->getCondition())) { + auto CmpConstOperand = dyn_cast(CmpI->getOperand(1)); + if (!CmpConstOperand) + return llvm::None; + CI = dyn_cast(CmpI->getOperand(0)); + } else { + CI = dyn_cast(BI->getCondition()); + } + + if (!CI) + return llvm::None; + + llvm::Function *Fn = CI->getCalledFunction(); + if (!Fn || Fn->getIntrinsicID() != llvm::Intrinsic::expect) + return llvm::None; + + llvm::ConstantInt *ExpectedValue = + dyn_cast(CI->getArgOperand(1)); + + if (!ExpectedValue) + return 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; + +void CheckMisExpectBranch(const Expr *Cond, const llvm::BranchInst *BI, + uint64_t TrueCount, uint64_t CurrProfCount, + CodeGenModule &CGM) { + auto CGOpt = CGM.getCodeGenOpts(); + if (!CGOpt.MisExpect || + (CGOpt.getProfileUse() == CodeGenOptions::ProfileNone)) + return; + + auto *Call = dyn_cast(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; + double Percentage; + const uint64_t TotalBranchWeight = LikelyBranchWeight + UnlikelyBranchWeight; + + // LowerExpectIntrinsics.cpp:49 LikelyBranchWeight = 2000 + // LowerExpectIntrinsics.cpp:52 UnlikelyBranchWeight = 1 + if (ExpectedTrueBranch) { + const llvm::BranchProbability LikelyThreshold(LikelyBranchWeight, + TotalBranchWeight); + Scaled = LikelyThreshold.scale(CurrProfCount); + Percentage = (TrueCount / (double)CurrProfCount); + if (TrueCount < Scaled) + IncorrectPerfCounters = true; + } else { + const llvm::BranchProbability UnlikelyThreshold(UnlikelyBranchWeight, + LikelyBranchWeight); + Scaled = UnlikelyThreshold.scale(CurrProfCount); + Percentage = ((CurrProfCount - TrueCount) / (double)CurrProfCount); + if (TrueCount > Scaled) + IncorrectPerfCounters = true; + } + + LLVM_DEBUG(llvm::dbgs() << "------------------\n"); + LLVM_DEBUG(llvm::dbgs() << "Function:" << BI->getFunction()->getName() + << "\n"); + LLVM_DEBUG(llvm::dbgs() << "Instruction:" << *BI << "\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 *SwitchWeights, + CodeGenModule &CGM) { + if (!SwitchWeights) + return; + + Optional 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. + + // 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(), (uint64_t)0, + [](uint64_t w1, uint64_t w2) { return w1 + w2; }); + const uint64_t TotalBranchWeight = + LikelyBranchWeight + (UnlikelyBranchWeight * n); + double Percentage = ((double)TakenCount / (double)CaseTotal); + const llvm::BranchProbability LikelyThreshold(LikelyBranchWeight, + TotalBranchWeight); + auto ScaledThreshold = LikelyThreshold.scale(CaseTotal); + + LLVM_DEBUG(DebugPrintMisExpectSwitchInfo( + {SwitchWeights, ExpectedValue->getZExtValue(), Index, TakenCount, + ScaledThreshold, Percentage})); + + if (TakenCount < ScaledThreshold) + EmitMisExpectWarning(Call, CGM, Percentage); +} +#undef DEBUG_TYPE + +} // namespace MisExpect +} // namespace CodeGen +} // namespace clang diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4051,6 +4051,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"); diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/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); diff --git a/clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext b/clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext new file mode 100644 --- /dev/null +++ b/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 + diff --git a/clang/test/Profile/Inputs/misexpect-branch.proftext b/clang/test/Profile/Inputs/misexpect-branch.proftext new file mode 100644 --- /dev/null +++ b/clang/test/Profile/Inputs/misexpect-branch.proftext @@ -0,0 +1,9 @@ +bar +# Func Hash: +45795613684824 +# Num Counters: +2 +# Counter Values: +200000 +0 + diff --git a/clang/test/Profile/Inputs/misexpect-switch-default-only.proftext b/clang/test/Profile/Inputs/misexpect-switch-default-only.proftext new file mode 100644 --- /dev/null +++ b/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 + diff --git a/clang/test/Profile/Inputs/misexpect-switch.proftext b/clang/test/Profile/Inputs/misexpect-switch.proftext new file mode 100644 --- /dev/null +++ b/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 + diff --git a/clang/test/Profile/misexpect-branch-cold.c b/clang/test/Profile/misexpect-branch-cold.c new file mode 100644 --- /dev/null +++ b/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; +} + diff --git a/clang/test/Profile/misexpect-branch-nonconst-expected-val.c b/clang/test/Profile/misexpect-branch-nonconst-expected-val.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/test/Profile/misexpect-branch.c b/clang/test/Profile/misexpect-branch.c new file mode 100644 --- /dev/null +++ b/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; +} + diff --git a/clang/test/Profile/misexpect-no-warning-without-flag.c b/clang/test/Profile/misexpect-no-warning-without-flag.c new file mode 100644 --- /dev/null +++ b/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; +} + diff --git a/clang/test/Profile/misexpect-switch-default.c b/clang/test/Profile/misexpect-switch-default.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/test/Profile/misexpect-switch-nonconst.c b/clang/test/Profile/misexpect-switch-nonconst.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/test/Profile/misexpect-switch-only-default-case.c b/clang/test/Profile/misexpect-switch-only-default-case.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/clang/test/Profile/misexpect-switch.c b/clang/test/Profile/misexpect-switch.c new file mode 100644 --- /dev/null +++ b/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; +} diff --git a/llvm/include/llvm/IR/DiagnosticInfo.h b/llvm/include/llvm/IR/DiagnosticInfo.h --- a/llvm/include/llvm/IR/DiagnosticInfo.h +++ b/llvm/include/llvm/IR/DiagnosticInfo.h @@ -75,7 +75,8 @@ DK_MIRParser, DK_PGOProfile, DK_Unsupported, - DK_FirstPluginKind + DK_FirstPluginKind, + DK_MisExpect }; /// Get the next available kind ID for a plugin diagnostic. @@ -1002,6 +1003,31 @@ void print(DiagnosticPrinter &DP) const override; }; +/// Diagnostic information for MisExpect analysis. +class DiagnosticInfoMisExpect : public DiagnosticInfoWithLocationBase { +public: + DiagnosticInfoMisExpect(const Function &Fn, const Twine &Msg, + const DiagnosticLocation &Loc = DiagnosticLocation(), + DiagnosticSeverity Severity = DS_Error) + : DiagnosticInfoWithLocationBase(DK_MisExpect, Severity, Fn, Loc), + Msg(Msg) {} + + DiagnosticInfoMisExpect(const Instruction *Inst, Twine &Msg); + + /// \see DiagnosticInfo::print. + void print(DiagnosticPrinter &DP) const override; + + static bool classof(const DiagnosticInfo *DI) { + return DI->getKind() == DK_MisExpect; + } + + const Twine &getMsg() const { return Msg; } + +private: + /// Message to report. + const Twine &Msg; +}; + } // end namespace llvm #endif // LLVM_IR_DIAGNOSTICINFO_H diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def b/llvm/include/llvm/IR/FixedMetadataKinds.def --- a/llvm/include/llvm/IR/FixedMetadataKinds.def +++ b/llvm/include/llvm/IR/FixedMetadataKinds.def @@ -39,3 +39,4 @@ LLVM_FIXED_MD_KIND(MD_access_group, "llvm.access.group", 25) LLVM_FIXED_MD_KIND(MD_callback, "callback", 26) LLVM_FIXED_MD_KIND(MD_preserve_access_index, "llvm.preserve.access.index", 27) +LLVM_FIXED_MD_KIND(MD_misexpect, "misexpect", 28) diff --git a/llvm/include/llvm/IR/MDBuilder.h b/llvm/include/llvm/IR/MDBuilder.h --- a/llvm/include/llvm/IR/MDBuilder.h +++ b/llvm/include/llvm/IR/MDBuilder.h @@ -16,6 +16,7 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/StringRef.h" +#include "llvm/IR/Constants.h" #include "llvm/IR/GlobalValue.h" #include "llvm/Support/DataTypes.h" #include @@ -75,6 +76,10 @@ /// Return metadata containing the section prefix for a function. MDNode *createFunctionSectionPrefix(StringRef Prefix); + /// return metadata containing expected value + MDNode *createMisExpect(uint64_t Index, uint64_t LikelyWeight, + uint64_t UnlikelyWeight); + //===------------------------------------------------------------------===// // Range metadata. //===------------------------------------------------------------------===// diff --git a/llvm/include/llvm/Transforms/Utils/MisExpect.h b/llvm/include/llvm/Transforms/Utils/MisExpect.h new file mode 100644 --- /dev/null +++ b/llvm/include/llvm/Transforms/Utils/MisExpect.h @@ -0,0 +1,35 @@ +//===--- 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(). This utility extracts the threshold values from metadata +// associated with the instrumented Branch or Switch. The threshold values are +// then used to determin if a warning would be emmited. +// +//===----------------------------------------------------------------------===// + +#include "llvm/ADT/SmallVector.h" +#include "llvm/IR/Function.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/LLVMContext.h" + +namespace llvm { +namespace misexpect { + +/// verifyMisExpect - compares PGO counters to the thresholds used for +/// __builtin_expect and warns if the PGO counters are outside of the expected +/// range. +/// \param I the Instruction being checked +/// \param Weights A vector of profile weights for each target block +/// \param Ctx The current LLVM context +void verifyMisExpect(llvm::Instruction *I, + const llvm::SmallVector &Weights, + llvm::LLVMContext &Ctx); + +} // namespace misexpect +} // namespace llvm diff --git a/llvm/lib/IR/DiagnosticInfo.cpp b/llvm/lib/IR/DiagnosticInfo.cpp --- a/llvm/lib/IR/DiagnosticInfo.cpp +++ b/llvm/lib/IR/DiagnosticInfo.cpp @@ -370,5 +370,16 @@ return OS.str(); } +DiagnosticInfoMisExpect::DiagnosticInfoMisExpect(const Instruction *Inst, + Twine &Msg) + : DiagnosticInfoWithLocationBase(DK_MisExpect, DS_Warning, + *Inst->getParent()->getParent(), + Inst->getDebugLoc()), + Msg(Msg) {} + +void DiagnosticInfoMisExpect::print(DiagnosticPrinter &DP) const { + DP << getLocationStr() << ": " << getMsg(); +} + void OptimizationRemarkAnalysisFPCommute::anchor() {} void OptimizationRemarkAnalysisAliasing::anchor() {} diff --git a/llvm/lib/IR/MDBuilder.cpp b/llvm/lib/IR/MDBuilder.cpp --- a/llvm/lib/IR/MDBuilder.cpp +++ b/llvm/lib/IR/MDBuilder.cpp @@ -309,3 +309,15 @@ }; return MDNode::get(Context, Vals); } + +MDNode *MDBuilder::createMisExpect(uint64_t Index, uint64_t LikleyWeight, + uint64_t UnlikleyWeight) { + auto IntType = Type::getInt64Ty(Context); + Metadata *Vals[] = { + createString("misexpect"), + createConstant(ConstantInt::get(IntType, Index)), + createConstant(ConstantInt::get(IntType, LikleyWeight)), + createConstant(ConstantInt::get(IntType, UnlikleyWeight)), + }; + return MDNode::get(Context, Vals); +} diff --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp --- a/llvm/lib/Transforms/IPO/SampleProfile.cpp +++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp @@ -72,6 +72,7 @@ #include "llvm/Transforms/Instrumentation.h" #include "llvm/Transforms/Utils/CallPromotionUtils.h" #include "llvm/Transforms/Utils/Cloning.h" +#include "llvm/Transforms/Utils/MisExpect.h" #include #include #include @@ -1447,6 +1448,8 @@ } } + misexpect::verifyMisExpect(TI, Weights, TI->getContext()); + uint64_t TempWeight; // Only set weights if there is at least one non-zero weight. // In any other case, let the analyzer set weights. diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -108,6 +108,7 @@ #include "llvm/Transforms/Instrumentation.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" #include "llvm/Transforms/Utils/BasicBlockUtils.h" +#include "llvm/Transforms/Utils/MisExpect.h" #include #include #include @@ -1776,6 +1777,9 @@ : Weights) { dbgs() << W << " "; } dbgs() << "\n";); + + misexpect::verifyMisExpect(TI, Weights, TI->getContext()); + TI->setMetadata(LLVMContext::MD_prof, MDB.createBranchWeights(Weights)); if (EmitBranchProbability) { std::string BrCondStr = getBranchCondString(TI); diff --git a/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp b/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp --- a/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp +++ b/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp @@ -71,13 +71,15 @@ unsigned n = SI.getNumCases(); // +1 for default case. SmallVector Weights(n + 1, UnlikelyBranchWeight); - if (Case == *SI.case_default()) - Weights[0] = LikelyBranchWeight; - else - Weights[Case.getCaseIndex() + 1] = LikelyBranchWeight; + uint64_t Index = (Case == *SI.case_default()) ? 0 : Case.getCaseIndex() + 1; + Weights[Index] = LikelyBranchWeight; SI.setMetadata(LLVMContext::MD_prof, MDBuilder(CI->getContext()).createBranchWeights(Weights)); + SI.setMetadata( + LLVMContext::MD_misexpect, + MDBuilder(CI->getContext()) + .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight)); SI.setCondition(ArgValue); return true; @@ -280,14 +282,19 @@ MDBuilder MDB(CI->getContext()); MDNode *Node; + MDNode *ExpNode; if ((ExpectedValue->getZExtValue() == ValueComparedTo) == - (Predicate == CmpInst::ICMP_EQ)) + (Predicate == CmpInst::ICMP_EQ)) { Node = MDB.createBranchWeights(LikelyBranchWeight, UnlikelyBranchWeight); - else + ExpNode = MDB.createMisExpect(0, LikelyBranchWeight, UnlikelyBranchWeight); + } else { Node = MDB.createBranchWeights(UnlikelyBranchWeight, LikelyBranchWeight); + ExpNode = MDB.createMisExpect(1, LikelyBranchWeight, UnlikelyBranchWeight); + } BSI.setMetadata(LLVMContext::MD_prof, Node); + BSI.setMetadata(LLVMContext::MD_misexpect, ExpNode); if (CmpI) CmpI->setOperand(0, ArgValue); diff --git a/llvm/lib/Transforms/Utils/CMakeLists.txt b/llvm/lib/Transforms/Utils/CMakeLists.txt --- a/llvm/lib/Transforms/Utils/CMakeLists.txt +++ b/llvm/lib/Transforms/Utils/CMakeLists.txt @@ -40,6 +40,7 @@ LowerSwitch.cpp Mem2Reg.cpp MetaRenamer.cpp + MisExpect.cpp ModuleUtils.cpp NameAnonGlobals.cpp PredicateInfo.cpp diff --git a/llvm/lib/Transforms/Utils/MisExpect.cpp b/llvm/lib/Transforms/Utils/MisExpect.cpp new file mode 100644 --- /dev/null +++ b/llvm/lib/Transforms/Utils/MisExpect.cpp @@ -0,0 +1,105 @@ +//===--- 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(). This utility extracts the threshold values from metadata +// associated with the instrumented Branch or Switch. The threshold values are +// then used to determin if a warning would be emmited. +// +// MisExpect metadata is generated when llvm.expect intrinsics are lowered see +// LowerExpectIntrinsic.cpp +// +//===----------------------------------------------------------------------===// + +#include "llvm/Transforms/Utils/MisExpect.h" +#include "llvm/ADT/Twine.h" +#include "llvm/IR/DiagnosticInfo.h" +#include "llvm/IR/Instruction.h" +#include "llvm/IR/Instructions.h" +#include "llvm/IR/LLVMContext.h" +#include "llvm/Support/BranchProbability.h" +#include "llvm/Support/Debug.h" +#include "llvm/Support/FormatVariadic.h" +#include +#include + +#define DEBUG_TYPE "misexpect" + +namespace { + +llvm::Instruction *getOprndOrInst(llvm::Instruction *I) { + llvm::Instruction *Ret = nullptr; + if (auto B = llvm::dyn_cast(I)) { + Ret = llvm::dyn_cast(B->getCondition()); + } else if (auto S = llvm::dyn_cast(I)) { + Ret = llvm::dyn_cast(S->getCondition()); + } + return Ret ? Ret : I; +} + +void emitMisexpectWarning(llvm::Instruction *I, llvm::LLVMContext &Ctx, + double PercentageCorrect) { + auto PerString = llvm::formatv("{0:P}", PercentageCorrect); + llvm::Twine Msg(PerString); + llvm::Instruction *Cond = getOprndOrInst(I); + Ctx.diagnose(llvm::DiagnosticInfoMisExpect(Cond, Msg)); +} + +} // namespace + +namespace llvm { +namespace misexpect { + +void verifyMisExpect(Instruction *I, const SmallVector &Weights, + LLVMContext &Ctx) { + if (auto *MisExpectData = I->getMetadata(LLVMContext::MD_misexpect)) { + auto *MisExpectDataName = dyn_cast(MisExpectData->getOperand(0)); + if (MisExpectDataName && + MisExpectDataName->getString().equals("misexpect")) { + LLVM_DEBUG(llvm::dbgs() << "------------------\n"); + LLVM_DEBUG(llvm::dbgs() + << "Function: " << I->getFunction()->getName() << "\n"); + LLVM_DEBUG(llvm::dbgs() << "Instruction: " << *I << ":\n"); + LLVM_DEBUG(for (int Idx = 0, Size = Weights.size(); Idx < Size; ++Idx) { + llvm::dbgs() << "Weights[" << Idx << "] = " << Weights[Idx] << "\n"; + }); + LLVM_DEBUG(llvm::dbgs() << "------------------\n"); + + // extract values from misexpect metadata + auto *C = mdconst::dyn_extract(MisExpectData->getOperand(1)); + auto *L = mdconst::dyn_extract(MisExpectData->getOperand(2)); + auto *U = mdconst::dyn_extract(MisExpectData->getOperand(3)); + + uint64_t Index = C->getValue().getZExtValue(); + uint64_t LikelyBranchWeight = L->getValue().getZExtValue(); + uint64_t UnlikelyBranchWeight = U->getValue().getZExtValue(); + uint64_t ProfileCount = Weights[Index]; + uint64_t CaseTotal = + std::accumulate(Weights.begin(), Weights.end(), (uint64_t)0, + [](uint64_t W1, uint64_t W2) { return W1 + W2; }); + int NumUnlikelyTargets = Weights.size() - 2; + + const uint64_t TotalBranchWeight = + LikelyBranchWeight + (UnlikelyBranchWeight * NumUnlikelyTargets); + + double Percentage = ((double)ProfileCount / (double)CaseTotal); + + const llvm::BranchProbability LikelyThreshold(LikelyBranchWeight, + TotalBranchWeight); + auto ScaledThreshold = LikelyThreshold.scale(CaseTotal); + + if (ProfileCount < ScaledThreshold) + emitMisexpectWarning(I, Ctx, Percentage); + } + } +} + +} // namespace misexpect +} // namespace llvm + +#undef DEBUG_TYPE diff --git a/llvm/test/ThinLTO/X86/lazyload_metadata.ll b/llvm/test/ThinLTO/X86/lazyload_metadata.ll --- a/llvm/test/ThinLTO/X86/lazyload_metadata.ll +++ b/llvm/test/ThinLTO/X86/lazyload_metadata.ll @@ -10,13 +10,13 @@ ; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc \ ; RUN: -o /dev/null -stats \ ; RUN: 2>&1 | FileCheck %s -check-prefix=LAZY -; LAZY: 61 bitcode-reader - Number of Metadata records loaded +; LAZY: 63 bitcode-reader - Number of Metadata records loaded ; LAZY: 2 bitcode-reader - Number of MDStrings loaded ; RUN: llvm-lto -thinlto-action=import %t2.bc -thinlto-index=%t3.bc \ ; RUN: -o /dev/null -disable-ondemand-mds-loading -stats \ ; RUN: 2>&1 | FileCheck %s -check-prefix=NOTLAZY -; NOTLAZY: 70 bitcode-reader - Number of Metadata records loaded +; NOTLAZY: 72 bitcode-reader - Number of Metadata records loaded ; NOTLAZY: 7 bitcode-reader - Number of MDStrings loaded diff --git a/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll b/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll --- a/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll +++ b/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll @@ -138,7 +138,7 @@ %conv1 = sext i32 %conv to i64 %expval = call i64 @llvm.expect.i64(i64 %conv1, i64 0) %tobool = icmp ne i64 %expval, 0 -; CHECK: !prof !1 +; CHECK: !prof !2 ; CHECK-NOT: @llvm.expect br i1 %tobool, label %if.then, label %if.end @@ -165,7 +165,7 @@ %tmp = load i32, i32* %x.addr, align 4 %conv = sext i32 %tmp to i64 %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1) -; CHECK: !prof !2 +; CHECK: !prof !4 ; CHECK-NOT: @llvm.expect switch i64 %expval, label %sw.epilog [ i64 1, label %sw.bb @@ -194,7 +194,7 @@ %tmp = load i32, i32* %x.addr, align 4 %conv = sext i32 %tmp to i64 %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1) -; CHECK: !prof !3 +; CHECK: !prof !5 ; CHECK-NOT: @llvm.expect switch i64 %expval, label %sw.epilog [ i64 2, label %sw.bb @@ -278,7 +278,7 @@ %t7 = call i64 @llvm.expect.i64(i64 %t6, i64 0) %t8 = icmp ne i64 %t7, 0 %t9 = select i1 %t8, i32 1, i32 2 -; CHECK: select{{.*}}, !prof !1 +; CHECK: select{{.*}}, !prof !2 ret i32 %t9 } @@ -286,6 +286,6 @@ declare i1 @llvm.expect.i1(i1, i1) nounwind readnone ; CHECK: !0 = !{!"branch_weights", i32 2000, i32 1} -; CHECK: !1 = !{!"branch_weights", i32 1, i32 2000} -; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000, i32 1} -; CHECK: !3 = !{!"branch_weights", i32 2000, i32 1, i32 1} +; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000} +; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1} +; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}