diff --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h --- a/llvm/include/llvm/ProfileData/SampleProf.h +++ b/llvm/include/llvm/ProfileData/SampleProf.h @@ -177,7 +177,10 @@ SecFlagMD5Name = (1 << 0), // Store MD5 in fixed length instead of ULEB128 so NameTable can be // accessed like an array. - SecFlagFixedLengthMD5 = (1 << 1) + SecFlagFixedLengthMD5 = (1 << 1), + // Profile contains ".__uniq." suffix name. Compiler shouldn't strip + // the suffix when doing profile matching when seeing the flag. + SecFlagUniqSuffix = (1 << 2) }; enum class SecProfSummaryFlags : uint32_t { SecFlagInValid = 0, @@ -728,13 +731,14 @@ /// GUID to \p S. Also traverse the BodySamples to add hot CallTarget's GUID /// to \p S. void findInlinedFunctions(DenseSet &S, const Module *M, + const StringMap &SymbolMap, uint64_t Threshold) const { if (TotalSamples <= Threshold) return; auto isDeclaration = [](const Function *F) { return !F || F->isDeclaration(); }; - if (isDeclaration(M->getFunction(getFuncName()))) { + if (isDeclaration(SymbolMap.lookup(getFuncName()))) { // Add to the import list only when it's defined out of module. S.insert(getGUID(Name)); } @@ -743,13 +747,13 @@ for (const auto &BS : BodySamples) for (const auto &TS : BS.second.getCallTargets()) if (TS.getValue() > Threshold) { - const Function *Callee = M->getFunction(getFuncName(TS.getKey())); + const Function *Callee = SymbolMap.lookup(getFuncName(TS.getKey())); if (isDeclaration(Callee)) S.insert(getGUID(TS.getKey())); } for (const auto &CS : CallsiteSamples) for (const auto &NameFS : CS.second) - NameFS.second.findInlinedFunctions(S, M, Threshold); + NameFS.second.findInlinedFunctions(S, M, SymbolMap, Threshold); } /// Set the name of the function. @@ -780,17 +784,31 @@ return getCanonicalFnName(F.getName(), Attr); } - static StringRef getCanonicalFnName(StringRef FnName, StringRef Attr = "") { - static const char *knownSuffixes[] = { ".llvm.", ".part." }; + /// Name suffixes which canonicalization should handle to avoid + /// profile mismatch. + static constexpr const char *LLVMSuffix = ".llvm."; + static constexpr const char *PartSuffix = ".part."; + static constexpr const char *UniqSuffix = ".__uniq."; + + static StringRef getCanonicalFnName(StringRef FnName, + StringRef Attr = "selected") { + // Note the sequence of the suffixes in the knownSuffixes array matters. + // If suffix "A" is appended after the suffix "B", "A" should be in front + // of "B" in knownSuffixes. + const char *knownSuffixes[] = {LLVMSuffix, PartSuffix, UniqSuffix}; if (Attr == "" || Attr == "all") { return FnName.split('.').first; } else if (Attr == "selected") { StringRef Cand(FnName); for (const auto &Suf : knownSuffixes) { StringRef Suffix(Suf); + // If the profile contains ".__uniq." suffix, don't strip the + // suffix for names in the IR. + if (Suffix == UniqSuffix && FunctionSamples::HasUniqSuffix) + continue; auto It = Cand.rfind(Suffix); if (It == StringRef::npos) - return Cand; + continue; auto Dit = Cand.rfind('.'); if (Dit == It + Suffix.size() - 1) Cand = Cand.substr(0, It); @@ -861,6 +879,9 @@ /// Whether the profile uses MD5 to represent string. static bool UseMD5; + /// Whether the profile contains any ".__uniq." suffix in a name. + static bool HasUniqSuffix; + /// GUIDToFuncNameMap saves the mapping from GUID to the symbol name, for /// all the function symbols defined or declared in current module. DenseMap *GUIDToFuncNameMap = nullptr; diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h --- a/llvm/include/llvm/ProfileData/SampleProfReader.h +++ b/llvm/include/llvm/ProfileData/SampleProfReader.h @@ -363,7 +363,11 @@ /// Print the profile for \p FName on stream \p OS. void dumpFunctionProfile(StringRef FName, raw_ostream &OS = dbgs()); - virtual void collectFuncsFrom(const Module &M) {} + /// Collect functions with definitions in Module M. For reader which + /// support loading function profiles on demand, return true when the + /// reader has been given a module. Always return false for reader + /// which doesn't support loading function profiles on demand. + virtual bool collectFuncsFromModule() { return false; } /// Print all the profiles on stream \p OS. void dump(raw_ostream &OS = dbgs()); @@ -454,9 +458,13 @@ /// Don't read profile without context if the flag is set. This is only meaningful /// for ExtBinary format. virtual void setSkipFlatProf(bool Skip) {} + /// Return whether any name in the profile contains ".__uniq." suffix. + virtual bool hasUniqSuffix() { return false; } SampleProfileReaderItaniumRemapper *getRemapper() { return Remapper.get(); } + void setModule(const Module *Mod) { M = Mod; } + protected: /// Map every function to its associated profile. /// @@ -496,6 +504,11 @@ /// \brief The format of sample. SampleProfileFormat Format = SPF_None; + + /// \brief The current module being compiled if SampleProfileReader + /// is used by compiler. If SampleProfileReader is used by other + /// tools which are not compiler, M is usually nullptr. + const Module *M = nullptr; }; class SampleProfileReaderText : public SampleProfileReader { @@ -656,8 +669,6 @@ DenseMap FuncOffsetTable; /// The set containing the functions to use when compiling a module. DenseSet FuncsToUse; - /// Use all functions from the input profile. - bool UseAllFuncs = true; /// Use fixed length MD5 instead of ULEB128 encoding so NameTable doesn't /// need to be read in up front and can be directly accessed using index. @@ -692,8 +703,9 @@ uint64_t getFileSize(); virtual bool dumpSectionInfo(raw_ostream &OS = dbgs()) override; - /// Collect functions with definitions in Module \p M. - void collectFuncsFrom(const Module &M) override; + /// Collect functions with definitions in Module M. Return true if + /// the reader has been given a module. + bool collectFuncsFromModule() override; /// Return whether names in the profile are all MD5 numbers. virtual bool useMD5() override { return MD5StringBuf.get(); } @@ -731,8 +743,6 @@ DenseMap FuncOffsetTable; /// The set containing the functions to use when compiling a module. DenseSet FuncsToUse; - /// Use all functions from the input profile. - bool UseAllFuncs = true; virtual std::error_code verifySPMagic(uint64_t Magic) override; virtual std::error_code readNameTable() override; /// Read a string indirectly via the name table. @@ -751,8 +761,9 @@ /// Read samples only for functions to use. std::error_code readImpl() override; - /// Collect functions to be used when compiling Module \p M. - void collectFuncsFrom(const Module &M) override; + /// Collect functions with definitions in Module M. Return true if + /// the reader has been given a module. + bool collectFuncsFromModule() override; /// Return whether names in the profile are all MD5 numbers. virtual bool useMD5() override { return true; } diff --git a/llvm/lib/ProfileData/SampleProf.cpp b/llvm/lib/ProfileData/SampleProf.cpp --- a/llvm/lib/ProfileData/SampleProf.cpp +++ b/llvm/lib/ProfileData/SampleProf.cpp @@ -40,7 +40,8 @@ SampleProfileFormat FunctionSamples::Format; bool FunctionSamples::ProfileIsProbeBased = false; bool FunctionSamples::ProfileIsCS = false; -bool FunctionSamples::UseMD5; +bool FunctionSamples::UseMD5 = false; +bool FunctionSamples::HasUniqSuffix = true; } // namespace sampleprof } // namespace llvm @@ -262,6 +263,8 @@ const FunctionSamples *FunctionSamples::findFunctionSamplesAt( const LineLocation &Loc, StringRef CalleeName, SampleProfileReaderItaniumRemapper *Remapper) const { + CalleeName = getCanonicalFnName(CalleeName); + std::string CalleeGUID; CalleeName = getRepInFormat(CalleeName, UseMD5, CalleeGUID); diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp --- a/llvm/lib/ProfileData/SampleProfReader.cpp +++ b/llvm/lib/ProfileData/SampleProfReader.cpp @@ -584,6 +584,8 @@ bool UseMD5 = hasSecFlag(Entry, SecNameTableFlags::SecFlagMD5Name); assert((!FixedLengthMD5 || UseMD5) && "If FixedLengthMD5 is true, UseMD5 has to be true"); + FunctionSamples::HasUniqSuffix = + hasSecFlag(Entry, SecNameTableFlags::SecFlagUniqSuffix); if (std::error_code EC = readNameTableSec(UseMD5)) return EC; break; @@ -615,11 +617,13 @@ return sampleprof_error::success; } -void SampleProfileReaderExtBinaryBase::collectFuncsFrom(const Module &M) { - UseAllFuncs = false; +bool SampleProfileReaderExtBinaryBase::collectFuncsFromModule() { + if (!M) + return false; FuncsToUse.clear(); - for (auto &F : M) + for (auto &F : *M) FuncsToUse.insert(FunctionSamples::getCanonicalFnName(F)); + return true; } std::error_code SampleProfileReaderExtBinaryBase::readFuncOffsetTable() { @@ -648,14 +652,24 @@ } std::error_code SampleProfileReaderExtBinaryBase::readFuncProfiles() { + // Collect functions used by current module if the Reader has been + // given a module. + // collectFuncsFromModule uses FunctionSamples::getCanonicalFnName + // which will query FunctionSamples::HasUniqSuffix, so it has to be + // called after FunctionSamples::HasUniqSuffix is set, i.e. after + // NameTable section is read. + bool LoadFuncsToBeUsed = collectFuncsFromModule(); + + // When LoadFuncsToBeUsed is false, load all the function profiles. const uint8_t *Start = Data; - if (UseAllFuncs) { + if (!LoadFuncsToBeUsed) { while (Data < End) { if (std::error_code EC = readFuncProfile(Data)) return EC; } assert(Data == End && "More data is read than expected"); } else { + // Load function profiles on demand. if (Remapper) { for (auto Name : FuncsToUse) { Remapper->insert(Name); @@ -688,7 +702,6 @@ } Data = End; } - assert((CSProfileCount == 0 || CSProfileCount == Profiles.size()) && "Cannot have both context-sensitive and regular profile"); ProfileIsCS = (CSProfileCount > 0); @@ -783,13 +796,18 @@ } std::error_code SampleProfileReaderCompactBinary::readImpl() { + // Collect functions used by current module if the Reader has been + // given a module. + bool LoadFuncsToBeUsed = collectFuncsFromModule(); + std::vector OffsetsToUse; - if (UseAllFuncs) { + if (!LoadFuncsToBeUsed) { + // load all the function profiles. for (auto FuncEntry : FuncOffsetTable) { OffsetsToUse.push_back(FuncEntry.second); } - } - else { + } else { + // load function profiles on demand. for (auto Name : FuncsToUse) { auto GUID = std::to_string(MD5Hash(Name)); auto iter = FuncOffsetTable.find(StringRef(GUID)); @@ -1010,6 +1028,8 @@ Flags.append("fixlenmd5,"); else if (hasSecFlag(Entry, SecNameTableFlags::SecFlagMD5Name)) Flags.append("md5,"); + if (hasSecFlag(Entry, SecNameTableFlags::SecFlagUniqSuffix)) + Flags.append("uniq,"); break; case SecProfSummary: if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagPartial)) @@ -1117,11 +1137,13 @@ return sampleprof_error::success; } -void SampleProfileReaderCompactBinary::collectFuncsFrom(const Module &M) { - UseAllFuncs = false; +bool SampleProfileReaderCompactBinary::collectFuncsFromModule() { + if (!M) + return false; FuncsToUse.clear(); - for (auto &F : M) + for (auto &F : *M) FuncsToUse.insert(FunctionSamples::getCanonicalFnName(F)); + return true; } std::error_code SampleProfileReaderBinary::readSummaryEntry( diff --git a/llvm/lib/ProfileData/SampleProfWriter.cpp b/llvm/lib/ProfileData/SampleProfWriter.cpp --- a/llvm/lib/ProfileData/SampleProfWriter.cpp +++ b/llvm/lib/ProfileData/SampleProfWriter.cpp @@ -204,6 +204,17 @@ addName(I.first()); addNames(I.second); } + + // If NameTable contains ".__uniq." suffix, set SecFlagUniqSuffix flag + // so compiler won't strip the suffix during profile matching after + // seeing the flag in the profile. + for (const auto &I : NameTable) { + if (I.first.find(FunctionSamples::UniqSuffix) != StringRef::npos) { + addSectionFlag(SecNameTable, SecNameTableFlags::SecFlagUniqSuffix); + break; + } + } + if (auto EC = writeNameTable()) return EC; return sampleprof_error::success; diff --git a/llvm/lib/Transforms/IPO/SampleContextTracker.cpp b/llvm/lib/Transforms/IPO/SampleContextTracker.cpp --- a/llvm/lib/Transforms/IPO/SampleContextTracker.cpp +++ b/llvm/lib/Transforms/IPO/SampleContextTracker.cpp @@ -199,6 +199,8 @@ if (!DIL) return nullptr; + CalleeName = FunctionSamples::getCanonicalFnName(CalleeName); + // For indirect call, CalleeName will be empty, in which case the context // profile for callee with largest total samples will be returned. ContextTrieNode *CalleeContext = getCalleeContextFor(DIL, CalleeName); 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 @@ -608,7 +608,7 @@ StringRef CalleeName; if (Function *Callee = Inst.getCalledFunction()) - CalleeName = FunctionSamples::getCanonicalFnName(*Callee); + CalleeName = Callee->getName(); if (ProfileIsCS) return ContextTracker->getCalleeContextSamplesFor(Inst, CalleeName); @@ -994,7 +994,7 @@ for (const auto *FS : findIndirectCallFunctionSamples(*I, Sum)) { uint64_t SumOrigin = Sum; if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { - FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), + FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), SymbolMap, PSI->getOrCompHotCountThreshold()); continue; } @@ -1015,7 +1015,8 @@ } } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { findCalleeFunctionSamples(*I)->findInlinedFunctions( - InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold()); + InlinedGUIDs, F.getParent(), SymbolMap, + PSI->getOrCompHotCountThreshold()); } } Changed |= LocalChanged; @@ -1266,7 +1267,7 @@ for (const auto *FS : CalleeSamples) { // TODO: Consider disable pre-lTO ICP for MonoLTO as well if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { - FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), + FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), SymbolMap, PSI->getOrCompHotCountThreshold()); continue; } @@ -1313,7 +1314,8 @@ } } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { findCalleeFunctionSamples(*I)->findInlinedFunctions( - InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold()); + InlinedGUIDs, F.getParent(), SymbolMap, + PSI->getOrCompHotCountThreshold()); } } @@ -1679,7 +1681,9 @@ } Reader = std::move(ReaderOrErr.get()); Reader->setSkipFlatProf(LTOPhase == ThinOrFullLTOPhase::ThinLTOPostLink); - Reader->collectFuncsFrom(M); + // set module before reading the profile so reader may be able to only + // read the function profiles which are used by the current module. + Reader->setModule(&M); if (std::error_code EC = Reader->read()) { std::string Msg = "profile reading failed: " + EC.message(); Ctx.diagnose(DiagnosticInfoSampleProfile(Filename, Msg)); @@ -1763,12 +1767,11 @@ for (const auto &N_F : M.getValueSymbolTable()) { StringRef OrigName = N_F.getKey(); Function *F = dyn_cast(N_F.getValue()); - if (F == nullptr) + if (F == nullptr || OrigName.empty()) continue; SymbolMap[OrigName] = F; - auto pos = OrigName.find('.'); - if (pos != StringRef::npos) { - StringRef NewName = OrigName.substr(0, pos); + StringRef NewName = FunctionSamples::getCanonicalFnName(*F); + if (OrigName != NewName && !NewName.empty()) { auto r = SymbolMap.insert(std::make_pair(NewName, F)); // Failiing to insert means there is already an entry in SymbolMap, // thus there are multiple functions that are mapped to the same @@ -1781,12 +1784,13 @@ // Insert the remapped names into SymbolMap. if (Remapper) { if (auto MapName = Remapper->lookUpNameInProfile(OrigName)) { - if (*MapName == OrigName) - continue; - SymbolMap.insert(std::make_pair(*MapName, F)); + if (*MapName != OrigName && !MapName->empty()) + SymbolMap.insert(std::make_pair(*MapName, F)); } } } + assert(SymbolMap.count(StringRef()) == 0 && + "No empty StringRef should be added in SymbolMap"); bool retval = false; for (auto F : buildFunctionOrder(M, CG)) { diff --git a/llvm/test/Transforms/SampleProfile/Inputs/uniqname.nosuffix.afdo b/llvm/test/Transforms/SampleProfile/Inputs/uniqname.nosuffix.afdo new file mode 100644 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 GIT binary patch literal 0 Hc$@collectFuncsFrom(M); + Reader->setModule(&M); } TempFile createRemapFile() {