diff options
Diffstat (limited to 'llvm/lib')
-rw-r--r-- | llvm/lib/ProfileData/InstrProf.cpp | 43 | ||||
-rw-r--r-- | llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp | 16 | ||||
-rw-r--r-- | llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp | 28 |
3 files changed, 71 insertions, 16 deletions
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 77c6ffc9c25..74acd9e5e20 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -811,4 +811,47 @@ bool needsComdatForCounter(const Function &F, const Module &M) { return true; } + +// Check if INSTR_PROF_RAW_VERSION_VAR is defined. +bool isIRPGOFlagSet(const Module *M) { + auto IRInstrVar = + M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR)); + if (!IRInstrVar || IRInstrVar->isDeclaration() || + IRInstrVar->hasLocalLinkage()) + return false; + + // Check if the flag is set. + if (!IRInstrVar->hasInitializer()) + return false; + + const Constant *InitVal = IRInstrVar->getInitializer(); + if (!InitVal) + return false; + + return (dyn_cast<ConstantInt>(InitVal)->getZExtValue() & + VARIANT_MASK_IR_PROF) != 0; +} + +// Check if we can safely rename this Comdat function. +bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) { + if (F.getName().empty()) + return false; + if (!needsComdatForCounter(F, *(F.getParent()))) + return false; + // Unsafe to rename the address-taken function (which can be used in + // function comparison). + if (CheckAddressTaken && F.hasAddressTaken()) + return false; + // Only safe to do if this function may be discarded if it is not used + // in the compilation unit. + if (!GlobalValue::isDiscardableIfUnused(F.getLinkage())) + return false; + + // For AvailableExternallyLinkage functions. + if (!F.hasComdat()) { + assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage); + return true; + } + return true; +} } // end namespace llvm diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp index 8da3e31200f..adea7e77244 100644 --- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -32,6 +32,11 @@ cl::opt<bool> DoNameCompression("enable-name-compression", cl::desc("Enable name string compression"), cl::init(true)); +cl::opt<bool> DoHashBasedCounterSplit( + "hash-based-counter-split", + cl::desc("Rename counter variable of a comdat function based on cfg hash"), + cl::init(true)); + cl::opt<bool> ValueProfileStaticAlloc( "vp-static-alloc", cl::desc("Do static counter allocation for value profiler"), @@ -272,7 +277,16 @@ void InstrProfiling::lowerCoverageData(GlobalVariable *CoverageNamesVar) { static std::string getVarName(InstrProfIncrementInst *Inc, StringRef Prefix) { StringRef NamePrefix = getInstrProfNameVarPrefix(); StringRef Name = Inc->getName()->getName().substr(NamePrefix.size()); - return (Prefix + Name).str(); + Function *F = Inc->getParent()->getParent(); + Module *M = F->getParent(); + if (!DoHashBasedCounterSplit || !isIRPGOFlagSet(M) || + !canRenameComdatFunc(*F)) + return (Prefix + Name).str(); + uint64_t FuncHash = Inc->getHash()->getZExtValue(); + SmallVector<char, 24> HashPostfix; + if (Name.endswith((Twine(".") + Twine(FuncHash)).toStringRef(HashPostfix))) + return (Prefix + Name).str(); + return (Prefix + Name + "." + Twine(FuncHash)).str(); } static inline bool shouldRecordFunctionAddr(Function *F) { diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index 28f4f7ea145..04f9a64bef9 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -119,7 +119,7 @@ static cl::opt<unsigned> MaxNumAnnotations( // Command line option to control appending FunctionHash to the name of a COMDAT // function. This is to avoid the hash mismatch caused by the preinliner. static cl::opt<bool> DoComdatRenaming( - "do-comdat-renaming", cl::init(true), cl::Hidden, + "do-comdat-renaming", cl::init(false), cl::Hidden, cl::desc("Append function hash to the name of COMDAT function to avoid " "function hash mismatch due to the preinliner")); @@ -134,6 +134,12 @@ static cl::opt<bool> PGOWarnMissing("pgo-warn-missing-function", static cl::opt<bool> NoPGOWarnMismatch("no-pgo-warn-mismatch", cl::init(false), cl::Hidden); +// Command line option to enable/disable the warning about a hash mismatch in +// the profile data for Comdat functions, which often turns out to be false +// positive due to the pre-instrumentation inline. +static cl::opt<bool> NoPGOWarnMismatchComdat("no-pgo-warn-mismatch-comdat", + cl::init(true), cl::Hidden); + // Command line option to enable/disable select instruction instrumentation. static cl::opt<bool> PGOInstrSelect("pgo-instr-select", cl::init(true), cl::Hidden); @@ -407,21 +413,9 @@ void FuncPGOInstrumentation<Edge, BBInfo>::computeCFGHash() { static bool canRenameComdat( Function &F, std::unordered_multimap<Comdat *, GlobalValue *> &ComdatMembers) { - if (F.getName().empty()) - return false; - if (!needsComdatForCounter(F, *(F.getParent()))) - return false; - // Only safe to do if this function may be discarded if it is not used - // in the compilation unit. - if (!GlobalValue::isDiscardableIfUnused(F.getLinkage())) + if (!DoComdatRenaming || !canRenameComdatFunc(F, true)) return false; - // For AvailableExternallyLinkage functions. - if (!F.hasComdat()) { - assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage); - return true; - } - // FIXME: Current only handle those Comdat groups that only containing one // function and function aliases. // (1) For a Comdat group containing multiple functions, we need to have a @@ -803,7 +797,11 @@ bool PGOUseFunc::readCounters(IndexedInstrProfReader *PGOReader) { } else if (Err == instrprof_error::hash_mismatch || Err == instrprof_error::malformed) { NumOfPGOMismatch++; - SkipWarning = NoPGOWarnMismatch; + SkipWarning = + NoPGOWarnMismatch || + (NoPGOWarnMismatchComdat && + (F.hasComdat() || + F.getLinkage() == GlobalValue::AvailableExternallyLinkage)); } if (SkipWarning) |