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, @@ -623,10 +626,16 @@ /// the restriction to return the FunctionSamples at callsite location /// \p Loc with the maximum total sample count. If \p Remapper is not /// nullptr, use \p Remapper to find FunctionSamples with equivalent name - /// as \p CalleeName. + /// as \p CalleeName. \p SymbolMap is used to map CalleeName to Function + /// definition or declaration in current module. + /// If \p SymbolMap is not nullptr, findFunctionSamplesAt will do + /// canonication for \p CalleeName. If \p SymbolMap is not provided, user + /// has to make sure \p CalleeName has already been canonicalized or has no + /// need of canonicalization. const FunctionSamples * findFunctionSamplesAt(const LineLocation &Loc, StringRef CalleeName, - SampleProfileReaderItaniumRemapper *Remapper) const; + SampleProfileReaderItaniumRemapper *Remapper, + const StringMap *SymbolMap = nullptr) const; bool empty() const { return TotalSamples == 0; } @@ -729,13 +738,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)); } @@ -744,13 +754,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. @@ -781,17 +791,30 @@ return getCanonicalFnName(F.getName(), Attr); } + /// 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 = "") { - static const char *knownSuffixes[] = { ".llvm.", ".part." }; + // 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); @@ -805,6 +828,33 @@ return FnName; } + // Get the canonical function name in current module. The \p SymbolMap is + // used to get the Function definition or declaration in current module with + // the \p FnName being its name. It will return its canonical name according + // to the function's suffix elision strategy. + static StringRef + getCanonicalFnNameInModule(const StringMap *SymbolMap, + StringRef FnName) { + StringRef CanonName = FnName; + if (SymbolMap && !CanonName.empty()) { + // When a function is fully inlined, there may not be definition + // or declaration left in the module. In that case, we cannot + // get the suffix elision strategy from function attribute and will + // use "selected" strategy instead. + Function *Callee = SymbolMap->lookup(CanonName); + if (Callee) { + CanonName = getCanonicalFnName(*Callee); + } else { + // The default strategy in getCanonicalFnName will strip all the + // suffixes including ".__uniq.". It will introduce profile mismatch + // if the profile is generated from a binary enabling + // -funique-internal-linkage-names and contains ".__uniq." suffix. + CanonName = getCanonicalFnName(CanonName, "selected"); + } + } + return CanonName; + } + /// Translate \p Name into its original name. /// When profile doesn't use MD5, \p Name needs no translation. /// When profile uses MD5, \p Name in current FunctionSamples @@ -841,9 +891,15 @@ /// \returns the FunctionSamples pointer to the inlined instance. /// If \p Remapper is not nullptr, it will be used to find matching /// FunctionSamples with not exactly the same but equivalent name. - const FunctionSamples *findFunctionSamples( - const DILocation *DIL, - SampleProfileReaderItaniumRemapper *Remapper = nullptr) const; + /// + /// If \p SymbolMap is not nullptr, findFunctionSamplesAt will do + /// canonicalization for \p CalleeName. If \p SymbolMap is not provided, user + /// has to make sure \p CalleeName has already been canonicalized or has no + /// need of canonicalization. + const FunctionSamples * + findFunctionSamples(const DILocation *DIL, + SampleProfileReaderItaniumRemapper *Remapper = nullptr, + const StringMap *SymbolMap = nullptr) const; static bool ProfileIsProbeBased; @@ -858,6 +914,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/include/llvm/Transforms/IPO/SampleContextTracker.h b/llvm/include/llvm/Transforms/IPO/SampleContextTracker.h --- a/llvm/include/llvm/Transforms/IPO/SampleContextTracker.h +++ b/llvm/include/llvm/Transforms/IPO/SampleContextTracker.h @@ -96,8 +96,14 @@ SampleContextTracker(StringMap &Profiles); // Query context profile for a specific callee with given name at a given // call-site. The full context is identified by location of call instruction. - FunctionSamples *getCalleeContextSamplesFor(const CallBase &Inst, - StringRef CalleeName); + // + // If \p SymbolMap is not nullptr, getCalleeContextSamplesFor will do + // canonication for \p CalleeName. If \p SymbolMap is not provided, user + // has to make sure \p CalleeName has already been canonicalized or has no + // need of canonicalization. + FunctionSamples * + getCalleeContextSamplesFor(const CallBase &Inst, StringRef CalleeName, + const StringMap *SymbolMap = nullptr); // Get samples for indirect call targets for call site at given location. std::vector getIndirectCalleeContextSamplesFor(const DILocation *DIL); 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 @@ -34,7 +34,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 @@ -198,7 +199,8 @@ } const FunctionSamples *FunctionSamples::findFunctionSamples( - const DILocation *DIL, SampleProfileReaderItaniumRemapper *Remapper) const { + const DILocation *DIL, SampleProfileReaderItaniumRemapper *Remapper, + const StringMap *SymbolMap) const { assert(DIL); SmallVector, 10> S; @@ -213,7 +215,8 @@ return this; const FunctionSamples *FS = this; for (int i = S.size() - 1; i >= 0 && FS != nullptr; i--) { - FS = FS->findFunctionSamplesAt(S[i].first, S[i].second, Remapper); + FS = + FS->findFunctionSamplesAt(S[i].first, S[i].second, Remapper, SymbolMap); } return FS; } @@ -234,7 +237,10 @@ const FunctionSamples *FunctionSamples::findFunctionSamplesAt( const LineLocation &Loc, StringRef CalleeName, - SampleProfileReaderItaniumRemapper *Remapper) const { + SampleProfileReaderItaniumRemapper *Remapper, + const StringMap *SymbolMap) const { + CalleeName = getCanonicalFnNameInModule(SymbolMap, 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 @@ -191,14 +191,17 @@ } } -FunctionSamples * -SampleContextTracker::getCalleeContextSamplesFor(const CallBase &Inst, - StringRef CalleeName) { +FunctionSamples *SampleContextTracker::getCalleeContextSamplesFor( + const CallBase &Inst, StringRef CalleeName, + const StringMap *SymbolMap) { LLVM_DEBUG(dbgs() << "Getting callee context for instr: " << Inst << "\n"); DILocation *DIL = Inst.getDebugLoc(); if (!DIL) return nullptr; + CalleeName = + FunctionSamples::getCanonicalFnNameInModule(SymbolMap, 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 @@ -594,17 +594,19 @@ StringRef CalleeName; if (Function *Callee = Inst.getCalledFunction()) - CalleeName = FunctionSamples::getCanonicalFnName(*Callee); + CalleeName = Callee->getName(); if (ProfileIsCS) - return ContextTracker->getCalleeContextSamplesFor(Inst, CalleeName); + return ContextTracker->getCalleeContextSamplesFor(Inst, CalleeName, + &SymbolMap); const FunctionSamples *FS = findFunctionSamples(Inst); if (FS == nullptr) return nullptr; return FS->findFunctionSamplesAt(FunctionSamples::getCallSiteIdentifier(DIL), - CalleeName, Reader->getRemapper()); + CalleeName, Reader->getRemapper(), + &SymbolMap); } /// Returns a vector of FunctionSamples that are the indirect call targets @@ -685,7 +687,7 @@ it.first->second = ContextTracker->getContextSamplesFor(DIL); else it.first->second = - Samples->findFunctionSamples(DIL, Reader->getRemapper()); + Samples->findFunctionSamples(DIL, Reader->getRemapper(), &SymbolMap); } return it.first->second; } @@ -945,7 +947,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; } @@ -967,7 +969,8 @@ } } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { findCalleeFunctionSamples(*I)->findInlinedFunctions( - InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold()); + InlinedGUIDs, F.getParent(), SymbolMap, + PSI->getOrCompHotCountThreshold()); } } Changed |= LocalChanged; @@ -1221,7 +1224,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; } @@ -1268,7 +1271,8 @@ } } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { findCalleeFunctionSamples(*I)->findInlinedFunctions( - InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold()); + InlinedGUIDs, F.getParent(), SymbolMap, + PSI->getOrCompHotCountThreshold()); } } @@ -1632,7 +1636,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)); @@ -1716,12 +1722,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 @@ -1734,12 +1739,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() {