Skip to content
This repository has been archived by the owner on Jan 1, 2023. It is now read-only.

Commit

Permalink
Reland "clang-misexpect: Profile Guided Validation of Performance Ann…
Browse files Browse the repository at this point in the history
…otations 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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371635 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
petrhosek committed Sep 11, 2019
1 parent 63f6564 commit 0c36de7
Show file tree
Hide file tree
Showing 23 changed files with 1,345 additions and 25 deletions.
22 changes: 21 additions & 1 deletion include/llvm/IR/DiagnosticInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ enum DiagnosticKind {
DK_MIRParser,
DK_PGOProfile,
DK_Unsupported,
DK_FirstPluginKind
DK_FirstPluginKind,
DK_MisExpect
};

/// Get the next available kind ID for a plugin diagnostic.
Expand Down Expand Up @@ -1002,6 +1003,25 @@ class DiagnosticInfoUnsupported : public DiagnosticInfoWithLocationBase {
void print(DiagnosticPrinter &DP) const override;
};

/// Diagnostic information for MisExpect analysis.
class DiagnosticInfoMisExpect : public DiagnosticInfoWithLocationBase {
public:
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
1 change: 1 addition & 0 deletions include/llvm/IR/FixedMetadataKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,4 @@ LLVM_FIXED_MD_KIND(MD_irr_loop, "irr_loop", 24)
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)
5 changes: 5 additions & 0 deletions include/llvm/IR/MDBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 <utility>
Expand Down Expand Up @@ -75,6 +76,10 @@ class MDBuilder {
/// 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.
//===------------------------------------------------------------------===//
Expand Down
43 changes: 43 additions & 0 deletions include/llvm/Transforms/Utils/MisExpect.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//===--- MisExpect.h - Check the use of llvm.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 the
// llvm.expect intrinsic. This utility extracts the threshold values from
// metadata associated with the instrumented Branch or Switch instruction. The
// threshold values are then used to determine if a warning should 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
/// llvm.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<uint32_t, 4> &Weights,
llvm::LLVMContext &Ctx);

/// checkClangInstrumentation - verify if llvm.expect matches PGO profile
/// This function checks the frontend instrumentation in the backend when
/// lowering llvm.expect intrinsics. It checks for existing metadata, and
/// then validates the use of llvm.expect against the assigned branch weights.
//
/// \param I the Instruction being checked
void checkFrontendInstrumentation(Instruction &I);

} // namespace misexpect
} // namespace llvm
11 changes: 11 additions & 0 deletions lib/IR/DiagnosticInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,5 +370,16 @@ std::string DiagnosticInfoOptimizationBase::getMsg() const {
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() {}
12 changes: 12 additions & 0 deletions lib/IR/MDBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,15 @@ MDNode *MDBuilder::createIrrLoopHeaderWeight(uint64_t Weight) {
};
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);
}
3 changes: 3 additions & 0 deletions lib/Transforms/IPO/SampleProfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <algorithm>
#include <cassert>
#include <cstdint>
Expand Down Expand Up @@ -1446,6 +1447,8 @@ void SampleProfileLoader::propagateWeights(Function &F) {
}
}

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.
Expand Down
4 changes: 4 additions & 0 deletions lib/Transforms/Instrumentation/PGOInstrumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <algorithm>
#include <cassert>
#include <cstdint>
Expand Down Expand Up @@ -1776,6 +1777,9 @@ void llvm::setProfMetadata(Module *M, Instruction *TI,
: 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);
Expand Down
31 changes: 23 additions & 8 deletions lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
#include "llvm/Transforms/Scalar.h"
#include "llvm/Transforms/Utils/MisExpect.h"

using namespace llvm;

Expand Down Expand Up @@ -71,15 +72,20 @@ static bool handleSwitchExpect(SwitchInst &SI) {
unsigned n = SI.getNumCases(); // +1 for default case.
SmallVector<uint32_t, 16> 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_misexpect,
MDBuilder(CI->getContext())
.createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));

SI.setCondition(ArgValue);
misexpect::checkFrontendInstrumentation(SI);

SI.setMetadata(LLVMContext::MD_prof,
MDBuilder(CI->getContext()).createBranchWeights(Weights));

SI.setCondition(ArgValue);
return true;
}

Expand Down Expand Up @@ -280,19 +286,28 @@ template <class BrSelInst> static bool handleBrSelExpect(BrSelInst &BSI) {

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);
else
BSI.setCondition(ArgValue);

misexpect::checkFrontendInstrumentation(BSI);

BSI.setMetadata(LLVMContext::MD_prof, Node);

return true;
}

Expand Down
1 change: 1 addition & 0 deletions lib/Transforms/Utils/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ add_llvm_library(LLVMTransformUtils
LowerSwitch.cpp
Mem2Reg.cpp
MetaRenamer.cpp
MisExpect.cpp
ModuleUtils.cpp
NameAnonGlobals.cpp
PredicateInfo.cpp
Expand Down
Loading

0 comments on commit 0c36de7

Please sign in to comment.