Revert "[PGO] Context sensitive PGO (part 1)"
This reverts commit r354930, it was causing UBSan failures.
llvm-svn: 354953
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index c956f4e..bb2e335 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -65,7 +65,6 @@
#include "llvm/Analysis/IndirectCallVisitor.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
-#include "llvm/Analysis/ProfileSummaryInfo.h"
#include "llvm/IR/Attributes.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/CFG.h"
@@ -133,19 +132,6 @@
STATISTIC(NumOfPGOMismatch, "Number of functions having mismatch profile.");
STATISTIC(NumOfPGOMissing, "Number of functions without profile.");
STATISTIC(NumOfPGOICall, "Number of indirect call value instrumentations.");
-STATISTIC(NumOfCSPGOInstrument, "Number of edges instrumented in CSPGO.");
-STATISTIC(NumOfCSPGOSelectInsts,
- "Number of select instruction instrumented in CSPGO.");
-STATISTIC(NumOfCSPGOMemIntrinsics,
- "Number of mem intrinsics instrumented in CSPGO.");
-STATISTIC(NumOfCSPGOEdge, "Number of edges in CSPGO.");
-STATISTIC(NumOfCSPGOBB, "Number of basic-blocks in CSPGO.");
-STATISTIC(NumOfCSPGOSplit, "Number of critical edge splits in CSPGO.");
-STATISTIC(NumOfCSPGOFunc,
- "Number of functions having valid profile counts in CSPGO.");
-STATISTIC(NumOfCSPGOMismatch,
- "Number of functions having mismatch profile in CSPGO.");
-STATISTIC(NumOfCSPGOMissing, "Number of functions without profile in CSPGO.");
// Command line option to specify the file to read profile from. This is
// mainly used for testing.
@@ -397,8 +383,7 @@
public:
static char ID;
- PGOInstrumentationGenLegacyPass(bool IsCS = false)
- : ModulePass(ID), IsCS(IsCS) {
+ PGOInstrumentationGenLegacyPass() : ModulePass(ID) {
initializePGOInstrumentationGenLegacyPassPass(
*PassRegistry::getPassRegistry());
}
@@ -406,8 +391,6 @@
StringRef getPassName() const override { return "PGOInstrumentationGenPass"; }
private:
- // Is this is context-sensitive instrumentation.
- bool IsCS;
bool runOnModule(Module &M) override;
void getAnalysisUsage(AnalysisUsage &AU) const override {
@@ -420,8 +403,8 @@
static char ID;
// Provide the profile filename as the parameter.
- PGOInstrumentationUseLegacyPass(std::string Filename = "", bool IsCS = false)
- : ModulePass(ID), ProfileFileName(std::move(Filename)), IsCS(IsCS) {
+ PGOInstrumentationUseLegacyPass(std::string Filename = "")
+ : ModulePass(ID), ProfileFileName(std::move(Filename)) {
if (!PGOTestProfileFile.empty())
ProfileFileName = PGOTestProfileFile;
initializePGOInstrumentationUseLegacyPassPass(
@@ -432,38 +415,14 @@
private:
std::string ProfileFileName;
- // Is this is context-sensitive instrumentation use.
- bool IsCS;
bool runOnModule(Module &M) override;
void getAnalysisUsage(AnalysisUsage &AU) const override {
- AU.addRequired<ProfileSummaryInfoWrapperPass>();
AU.addRequired<BlockFrequencyInfoWrapperPass>();
}
};
-class PGOInstrumentationGenCreateVarLegacyPass : public ModulePass {
-public:
- static char ID;
- StringRef getPassName() const override {
- return "PGOInstrumentationGenCreateVarPass";
- }
- PGOInstrumentationGenCreateVarLegacyPass(std::string CSInstrName = "")
- : ModulePass(ID), InstrProfileOutput(CSInstrName) {
- initializePGOInstrumentationGenCreateVarLegacyPassPass(
- *PassRegistry::getPassRegistry());
- }
-
-private:
- bool runOnModule(Module &M) override {
- createProfileFileNameVar(M, InstrProfileOutput);
- createIRLevelProfileFlagVar(M, true);
- return false;
- }
- std::string InstrProfileOutput;
-};
-
} // end anonymous namespace
char PGOInstrumentationGenLegacyPass::ID = 0;
@@ -475,8 +434,8 @@
INITIALIZE_PASS_END(PGOInstrumentationGenLegacyPass, "pgo-instr-gen",
"PGO instrumentation.", false, false)
-ModulePass *llvm::createPGOInstrumentationGenLegacyPass(bool IsCS) {
- return new PGOInstrumentationGenLegacyPass(IsCS);
+ModulePass *llvm::createPGOInstrumentationGenLegacyPass() {
+ return new PGOInstrumentationGenLegacyPass();
}
char PGOInstrumentationUseLegacyPass::ID = 0;
@@ -485,25 +444,11 @@
"Read PGO instrumentation profile.", false, false)
INITIALIZE_PASS_DEPENDENCY(BlockFrequencyInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(BranchProbabilityInfoWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
INITIALIZE_PASS_END(PGOInstrumentationUseLegacyPass, "pgo-instr-use",
"Read PGO instrumentation profile.", false, false)
-ModulePass *llvm::createPGOInstrumentationUseLegacyPass(StringRef Filename,
- bool IsCS) {
- return new PGOInstrumentationUseLegacyPass(Filename.str(), IsCS);
-}
-
-char PGOInstrumentationGenCreateVarLegacyPass::ID = 0;
-
-INITIALIZE_PASS(PGOInstrumentationGenCreateVarLegacyPass,
- "pgo-instr-gen-create-var",
- "Create PGO instrumentation version variable for CSPGO.", false,
- false)
-
-ModulePass *
-llvm::createPGOInstrumentationGenCreateVarLegacyPass(StringRef CSInstrName) {
- return new PGOInstrumentationGenCreateVarLegacyPass(CSInstrName);
+ModulePass *llvm::createPGOInstrumentationUseLegacyPass(StringRef Filename) {
+ return new PGOInstrumentationUseLegacyPass(Filename.str());
}
namespace {
@@ -551,9 +496,6 @@
private:
Function &F;
- // Is this is context-sensitive instrumentation.
- bool IsCS;
-
// A map that stores the Comdat group in function F.
std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers;
@@ -593,23 +535,15 @@
Function &Func,
std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
bool CreateGlobalVar = false, BranchProbabilityInfo *BPI = nullptr,
- BlockFrequencyInfo *BFI = nullptr, bool IsCS = false)
- : F(Func), IsCS(IsCS), ComdatMembers(ComdatMembers),
- ValueSites(IPVK_Last + 1), SIVisitor(Func), MIVisitor(Func),
- MST(F, BPI, BFI) {
+ BlockFrequencyInfo *BFI = nullptr)
+ : F(Func), ComdatMembers(ComdatMembers), ValueSites(IPVK_Last + 1),
+ SIVisitor(Func), MIVisitor(Func), MST(F, BPI, BFI) {
// This should be done before CFG hash computation.
SIVisitor.countSelects(Func);
MIVisitor.countMemIntrinsics(Func);
- if (!IsCS) {
- NumOfPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
- NumOfPGOMemIntrinsics += MIVisitor.getNumOfMemIntrinsics();
- NumOfPGOBB += MST.BBInfos.size();
- ValueSites[IPVK_IndirectCallTarget] = findIndirectCalls(Func);
- } else {
- NumOfCSPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
- NumOfCSPGOMemIntrinsics += MIVisitor.getNumOfMemIntrinsics();
- NumOfCSPGOBB += MST.BBInfos.size();
- }
+ NumOfPGOSelectInsts += SIVisitor.getNumOfSelectInsts();
+ NumOfPGOMemIntrinsics += MIVisitor.getNumOfMemIntrinsics();
+ ValueSites[IPVK_IndirectCallTarget] = findIndirectCalls(Func);
ValueSites[IPVK_MemOPSize] = MIVisitor.findMemIntrinsics(Func);
FuncName = getPGOFuncName(F);
@@ -618,12 +552,13 @@
renameComdatFunction();
LLVM_DEBUG(dumpInfo("after CFGMST"));
+ NumOfPGOBB += MST.BBInfos.size();
for (auto &E : MST.AllEdges) {
if (E->Removed)
continue;
- IsCS ? NumOfCSPGOEdge++ : NumOfPGOEdge++;
+ NumOfPGOEdge++;
if (!E->InMST)
- IsCS ? NumOfCSPGOInstrument++ : NumOfPGOInstrument++;
+ NumOfPGOInstrument++;
}
if (CreateGlobalVar)
@@ -662,17 +597,9 @@
}
}
JC.update(Indexes);
-
- // Hash format for context sensitive profile. Reserve 4 bits for other
- // information.
FunctionHash = (uint64_t)SIVisitor.getNumOfSelectInsts() << 56 |
(uint64_t)ValueSites[IPVK_IndirectCallTarget].size() << 48 |
- //(uint64_t)ValueSites[IPVK_MemOPSize].size() << 40 |
(uint64_t)MST.AllEdges.size() << 32 | JC.getCRC();
- // Reserve bit 60-63 for other information purpose.
- FunctionHash &= 0x0FFFFFFFFFFFFFFF;
- if (IsCS)
- NamedInstrProfRecord::setCSFlagInHash(FunctionHash);
LLVM_DEBUG(dbgs() << "Function Hash Computation for " << F.getName() << ":\n"
<< " CRC = " << JC.getCRC()
<< ", Selects = " << SIVisitor.getNumOfSelectInsts()
@@ -778,7 +705,7 @@
// For a critical edge, we have to split. Instrument the newly
// created BB.
- IsCS ? NumOfCSPGOSplit++ : NumOfPGOSplit++;
+ NumOfPGOSplit++;
LLVM_DEBUG(dbgs() << "Split critical edge: " << getBBInfo(SrcBB).Index
<< " --> " << getBBInfo(DestBB).Index << "\n");
unsigned SuccNum = GetSuccessorNumber(SrcBB, DestBB);
@@ -793,14 +720,12 @@
// Critical edges will be split.
static void instrumentOneFunc(
Function &F, Module *M, BranchProbabilityInfo *BPI, BlockFrequencyInfo *BFI,
- std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
- bool IsCS) {
+ std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) {
// Split indirectbr critical edges here before computing the MST rather than
// later in getInstrBB() to avoid invalidating it.
SplitIndirectBrCriticalEdges(F, BPI, BFI);
-
FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(F, ComdatMembers, true, BPI,
- BFI, IsCS);
+ BFI);
unsigned NumCounters = FuncInfo.getNumCounters();
uint32_t I = 0;
@@ -927,10 +852,10 @@
PGOUseFunc(Function &Func, Module *Modu,
std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers,
BranchProbabilityInfo *BPI = nullptr,
- BlockFrequencyInfo *BFIin = nullptr, bool IsCS = false)
+ BlockFrequencyInfo *BFIin = nullptr)
: F(Func), M(Modu), BFI(BFIin),
- FuncInfo(Func, ComdatMembers, false, BPI, BFIin, IsCS),
- FreqAttr(FFA_Normal), IsCS(IsCS) {}
+ FuncInfo(Func, ComdatMembers, false, BPI, BFIin),
+ FreqAttr(FFA_Normal) {}
// Read counts for the instrumented BB from profile.
bool readCounters(IndexedInstrProfReader *PGOReader, bool &AllZeros);
@@ -1003,9 +928,6 @@
// Function hotness info derived from profile.
FuncFreqAttr FreqAttr;
- // Is to use the context sensitive profile.
- bool IsCS;
-
// Find the Instrumented BB and set the value.
void setInstrumentedCounts(const std::vector<uint64_t> &CountFromProfile);
@@ -1099,31 +1021,23 @@
handleAllErrors(std::move(E), [&](const InstrProfError &IPE) {
auto Err = IPE.get();
bool SkipWarning = false;
- LLVM_DEBUG(dbgs() << "Error in reading profile for Func "
- << FuncInfo.FuncName << ": ");
if (Err == instrprof_error::unknown_function) {
- IsCS ? NumOfCSPGOMissing++ : NumOfPGOMissing++;
+ NumOfPGOMissing++;
SkipWarning = !PGOWarnMissing;
- LLVM_DEBUG(dbgs() << "unknown function");
} else if (Err == instrprof_error::hash_mismatch ||
Err == instrprof_error::malformed) {
- IsCS ? NumOfCSPGOMismatch++ : NumOfPGOMismatch++;
+ NumOfPGOMismatch++;
SkipWarning =
NoPGOWarnMismatch ||
(NoPGOWarnMismatchComdat &&
(F.hasComdat() ||
F.getLinkage() == GlobalValue::AvailableExternallyLinkage));
- LLVM_DEBUG(dbgs() << "hash mismatch (skip=" << SkipWarning << ")");
}
- LLVM_DEBUG(dbgs() << " IsCS=" << IsCS << "\n");
if (SkipWarning)
return;
- std::string Msg = IPE.message() + std::string(" ") + F.getName().str() +
- std::string(" Hash = ") +
- std::to_string(FuncInfo.FunctionHash);
-
+ std::string Msg = IPE.message() + std::string(" ") + F.getName().str();
Ctx.diagnose(
DiagnosticInfoPGOProfile(M->getName().data(), Msg, DS_Warning));
});
@@ -1132,7 +1046,7 @@
ProfileRecord = std::move(Result.get());
std::vector<uint64_t> &CountFromProfile = ProfileRecord.Counts;
- IsCS ? NumOfCSPGOFunc++ : NumOfPGOFunc++;
+ NumOfPGOFunc++;
LLVM_DEBUG(dbgs() << CountFromProfile.size() << " counts\n");
uint64_t ValueSum = 0;
for (unsigned I = 0, S = CountFromProfile.size(); I < S; I++) {
@@ -1147,11 +1061,7 @@
getBBInfo(nullptr).UnknownCountInEdge = 2;
setInstrumentedCounts(CountFromProfile);
-#if 0
- ProgramMaxCount = PGOReader->getMaximumFunctionCount(IsCS);
-#else
ProgramMaxCount = PGOReader->getMaximumFunctionCount();
-#endif
return true;
}
@@ -1256,8 +1166,7 @@
// Assign the scaled count values to the BB with multiple out edges.
void PGOUseFunc::setBranchWeights() {
// Generate MD_prof metadata for every branch instruction.
- LLVM_DEBUG(dbgs() << "\nSetting branch weights for func " << F.getName()
- << " IsCS=" << IsCS << "\n");
+ LLVM_DEBUG(dbgs() << "\nSetting branch weights.\n");
for (auto &BB : F) {
Instruction *TI = BB.getTerminator();
if (TI->getNumSuccessors() < 2)
@@ -1265,7 +1174,6 @@
if (!(isa<BranchInst>(TI) || isa<SwitchInst>(TI) ||
isa<IndirectBrInst>(TI)))
continue;
-
if (getBBInfo(&BB).CountValue == 0)
continue;
@@ -1443,6 +1351,24 @@
}
}
+// Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
+// aware this is an ir_level profile so it can set the version flag.
+static void createIRLevelProfileFlagVariable(Module &M) {
+ Type *IntTy64 = Type::getInt64Ty(M.getContext());
+ uint64_t ProfileVersion = (INSTR_PROF_RAW_VERSION | VARIANT_MASK_IR_PROF);
+ auto IRLevelVersionVariable = new GlobalVariable(
+ M, IntTy64, true, GlobalVariable::ExternalLinkage,
+ Constant::getIntegerValue(IntTy64, APInt(64, ProfileVersion)),
+ INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
+ IRLevelVersionVariable->setVisibility(GlobalValue::DefaultVisibility);
+ Triple TT(M.getTargetTriple());
+ if (!TT.supportsCOMDAT())
+ IRLevelVersionVariable->setLinkage(GlobalValue::WeakAnyLinkage);
+ else
+ IRLevelVersionVariable->setComdat(M.getOrInsertComdat(
+ StringRef(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR))));
+}
+
// Collect the set of members for each Comdat in module M and store
// in ComdatMembers.
static void collectComdatMembers(
@@ -1463,11 +1389,8 @@
static bool InstrumentAllFunctions(
Module &M, function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
- function_ref<BlockFrequencyInfo *(Function &)> LookupBFI, bool IsCS) {
- // For the context-sensitve instrumentation, we should have a separated pass
- // (before LTO/ThinLTO linking) to create these variables.
- if (!IsCS)
- createIRLevelProfileFlagVar(M, /* IsCS */ false);
+ function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
+ createIRLevelProfileFlagVariable(M);
std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
collectComdatMembers(M, ComdatMembers);
@@ -1476,7 +1399,7 @@
continue;
auto *BPI = LookupBPI(F);
auto *BFI = LookupBFI(F);
- instrumentOneFunc(F, &M, BPI, BFI, ComdatMembers, IsCS);
+ instrumentOneFunc(F, &M, BPI, BFI, ComdatMembers);
}
return true;
}
@@ -1491,7 +1414,7 @@
auto LookupBFI = [this](Function &F) {
return &this->getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI();
};
- return InstrumentAllFunctions(M, LookupBPI, LookupBFI, IsCS);
+ return InstrumentAllFunctions(M, LookupBPI, LookupBFI);
}
PreservedAnalyses PGOInstrumentationGen::run(Module &M,
@@ -1505,7 +1428,7 @@
return &FAM.getResult<BlockFrequencyAnalysis>(F);
};
- if (!InstrumentAllFunctions(M, LookupBPI, LookupBFI, IsCS))
+ if (!InstrumentAllFunctions(M, LookupBPI, LookupBFI))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
@@ -1514,7 +1437,7 @@
static bool annotateAllFunctions(
Module &M, StringRef ProfileFileName, StringRef ProfileRemappingFileName,
function_ref<BranchProbabilityInfo *(Function &)> LookupBPI,
- function_ref<BlockFrequencyInfo *(Function &)> LookupBFI, bool IsCS) {
+ function_ref<BlockFrequencyInfo *(Function &)> LookupBFI) {
LLVM_DEBUG(dbgs() << "Read in profile counters: ");
auto &Ctx = M.getContext();
// Read the counter array from file.
@@ -1535,11 +1458,6 @@
StringRef("Cannot get PGOReader")));
return false;
}
-#if 0
- if (!PGOReader->hasCSIRLevelProfile() && IsCS)
- return false;
-#endif
-
// TODO: might need to change the warning once the clang option is finalized.
if (!PGOReader->isIRLevelProfile()) {
Ctx.diagnose(DiagnosticInfoPGOProfile(
@@ -1559,7 +1477,7 @@
// Split indirectbr critical edges here before computing the MST rather than
// later in getInstrBB() to avoid invalidating it.
SplitIndirectBrCriticalEdges(F, BPI, BFI);
- PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI, IsCS);
+ PGOUseFunc Func(F, &M, ComdatMembers, BPI, BFI);
bool AllZeros = false;
if (!Func.readCounters(PGOReader.get(), AllZeros))
continue;
@@ -1607,14 +1525,7 @@
}
}
}
-#if 0
- M.setProfileSummary(PGOReader->getSummary(IsCS).getMD(M.getContext()),
- IsCS ? ProfileSummary::PSK_CSInstr
- : ProfileSummary::PSK_Instr);
-#else
M.setProfileSummary(PGOReader->getSummary().getMD(M.getContext()));
-#endif
-
// Set function hotness attribute from the profile.
// We have to apply these attributes at the end because their presence
// can affect the BranchProbabilityInfo of any callers, resulting in an
@@ -1633,10 +1544,9 @@
}
PGOInstrumentationUse::PGOInstrumentationUse(std::string Filename,
- std::string RemappingFilename,
- bool IsCS)
+ std::string RemappingFilename)
: ProfileFileName(std::move(Filename)),
- ProfileRemappingFileName(std::move(RemappingFilename)), IsCS(IsCS) {
+ ProfileRemappingFileName(std::move(RemappingFilename)) {
if (!PGOTestProfileFile.empty())
ProfileFileName = PGOTestProfileFile;
if (!PGOTestProfileRemappingFile.empty())
@@ -1656,7 +1566,7 @@
};
if (!annotateAllFunctions(M, ProfileFileName, ProfileRemappingFileName,
- LookupBPI, LookupBFI, IsCS))
+ LookupBPI, LookupBFI))
return PreservedAnalyses::all();
return PreservedAnalyses::none();
@@ -1673,8 +1583,7 @@
return &this->getAnalysis<BlockFrequencyInfoWrapperPass>(F).getBFI();
};
- return annotateAllFunctions(M, ProfileFileName, "", LookupBPI, LookupBFI,
- IsCS);
+ return annotateAllFunctions(M, ProfileFileName, "", LookupBPI, LookupBFI);
}
static std::string getSimpleNodeName(const BasicBlock *Node) {