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 @@ -175,7 +175,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, @@ -605,10 +608,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; } @@ -710,13 +719,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)); } @@ -725,13 +735,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. @@ -761,16 +771,20 @@ } static StringRef getCanonicalFnName(StringRef FnName, StringRef Attr = "") { - static const char *knownSuffixes[] = { ".llvm.", ".part." }; + static const char *knownSuffixes[] = {".llvm.", ".part.", ".__uniq."}; 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 == ".__uniq." && 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); @@ -820,9 +834,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 + /// 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 * + findFunctionSamples(const DILocation *DIL, + SampleProfileReaderItaniumRemapper *Remapper = nullptr, + const StringMap *SymbolMap = nullptr) const; static bool ProfileIsProbeBased; @@ -837,6 +857,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()); @@ -451,8 +455,13 @@ /// Return whether names in the profile are all MD5 numbers. virtual bool useMD5() { return false; } + /// 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. /// @@ -488,6 +497,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 { @@ -648,8 +662,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. @@ -680,8 +692,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(); } @@ -717,8 +730,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. @@ -737,8 +748,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 @@ -92,8 +92,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); // Query context profile for a given location. The full context // is identified by input DILocation. FunctionSamples *getContextSamplesFor(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,21 @@ const FunctionSamples *FunctionSamples::findFunctionSamplesAt( const LineLocation &Loc, StringRef CalleeName, - SampleProfileReaderItaniumRemapper *Remapper) const { + SampleProfileReaderItaniumRemapper *Remapper, + const StringMap *SymbolMap) const { + if (SymbolMap && !CalleeName.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. + Function *Callee = SymbolMap->lookup(CalleeName); + if (Callee) { + CalleeName = getCanonicalFnName(*Callee); + } else { + CalleeName = getCanonicalFnName(CalleeName, "selected"); + } + } + 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 @@ -583,6 +583,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; @@ -614,11 +616,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() { @@ -647,8 +651,17 @@ } 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; @@ -657,6 +670,7 @@ return sampleprof_error::success; } + // Load function profiles on demand. if (Remapper) { for (auto Name : FuncsToUse) { Remapper->insert(Name); @@ -774,13 +788,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)); @@ -992,6 +1011,8 @@ Flags.append("fixlenmd5,"); else if (hasSecFlag(Entry, SecNameTableFlags::SecFlagMD5Name)) Flags.append("md5,"); + else if (hasSecFlag(Entry, SecNameTableFlags::SecFlagUniqSuffix)) + Flags.append("uniq,"); break; case SecProfSummary: if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagPartial)) @@ -1099,11 +1120,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 @@ -203,6 +203,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(".__uniq.") != 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 @@ -187,9 +187,9 @@ } } -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"); // CSFDO-TODO: We use CalleeName to differentiate indirect call // We need to get sample for indirect callee too. @@ -197,6 +197,19 @@ if (!DIL) return nullptr; + if (SymbolMap && !CalleeName.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. + Function *Callee = SymbolMap->lookup(CalleeName); + if (Callee) { + CalleeName = getCanonicalFnName(*Callee); + } else { + CalleeName = getCanonicalFnName(CalleeName, "selected"); + } + } + ContextTrieNode *CalleeContext = getCalleeContextFor(DIL, CalleeName); if (CalleeContext) { FunctionSamples *FSamples = CalleeContext->getFunctionSamples(); 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 @@ -892,17 +892,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 @@ -972,7 +974,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; } @@ -1122,7 +1124,7 @@ uint64_t Sum; for (const auto *FS : findIndirectCallFunctionSamples(*I, Sum)) { if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { - FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), + FS->findInlinedFunctions(InlinedGUIDs, F.getParent(), SymbolMap, PSI->getOrCompHotCountThreshold()); continue; } @@ -1176,7 +1178,8 @@ } } else if (LTOPhase == ThinOrFullLTOPhase::ThinLTOPreLink) { findCalleeFunctionSamples(*I)->findInlinedFunctions( - InlinedGUIDs, F.getParent(), PSI->getOrCompHotCountThreshold()); + InlinedGUIDs, F.getParent(), SymbolMap, + PSI->getOrCompHotCountThreshold()); } } if (LocalChanged) { @@ -1946,7 +1949,9 @@ return false; } Reader = std::move(ReaderOrErr.get()); - 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); ProfileIsValid = (Reader->read() == sampleprof_error::success); PSL = Reader->getProfileSymbolList(); @@ -2019,12 +2024,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 @@ -2037,12 +2041,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("") == 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() {