xur updated this revision to Diff 455682. xur added a comment. Fixed unneeded &
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132600/new/ https://reviews.llvm.org/D132600 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/include/llvm/ProfileData/SampleProf.h llvm/test/tools/llvm-profdata/Inputs/FUnique.afdotext llvm/test/tools/llvm-profdata/Inputs/FUnique.proftext llvm/test/tools/llvm-profdata/Inputs/NoFUnique.afdotext llvm/test/tools/llvm-profdata/Inputs/NoFUnique.proftext llvm/test/tools/llvm-profdata/suppl-instr-with-sample-static-func.test llvm/tools/llvm-profdata/llvm-profdata.cpp
Index: llvm/tools/llvm-profdata/llvm-profdata.cpp =================================================================== --- llvm/tools/llvm-profdata/llvm-profdata.cpp +++ llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -31,6 +31,7 @@ #include "llvm/Support/Format.h" #include "llvm/Support/FormattedStream.h" #include "llvm/Support/InitLLVM.h" +#include "llvm/Support/MD5.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Path.h" #include "llvm/Support/ThreadPool.h" @@ -42,6 +43,10 @@ using namespace llvm; +// We use this string to indicate that there are +// multiple static functions map to the same name. +const std::string DuplicateNameStr = "----"; + enum ProfileFormat { PF_None = 0, PF_Text, @@ -478,7 +483,7 @@ ZeroCounterRatio = (float)ZeroCntNum / CntNum; } -/// Either set all the counters in the instr profile entry \p IFE to -1 +// Either set all the counters in the instr profile entry \p IFE to -1 /// in order to drop the profile or scale up the counters in \p IFP to /// be above hot threshold. We use the ratio of zero counters in the /// profile of a function to decide the profile is helpful or harmful @@ -539,7 +544,73 @@ unsigned InstrProfColdThreshold) { // Function to its entry in instr profile. StringMap<InstrProfileEntry> InstrProfileMap; + StringMap<StringRef> StaticFuncMap; InstrProfSummaryBuilder IPBuilder(ProfileSummaryBuilder::DefaultCutoffs); + + auto checkSampleProfileHasFUnique = [&Reader]() { + for (const auto &PD : Reader->getProfiles()) { + auto &FContext = PD.first; + if (FContext.toString().find(FunctionSamples::UniqSuffix) != + std::string::npos) { + return true; + } + } + return false; + }; + + bool SampleProfileHasFUnique = checkSampleProfileHasFUnique(); + + auto buildStaticFuncMap = [&StaticFuncMap, + SampleProfileHasFUnique](const StringRef Name) { + std::string Prefixes[] = {".cpp:", "cc:", ".c:", ".hpp:", ".h:"}; + size_t PrefixPos = StringRef::npos; + for (auto &Prefix : Prefixes) { + PrefixPos = Name.find_insensitive(Prefix); + if (PrefixPos == StringRef::npos) + continue; + PrefixPos += Prefix.size(); + break; + } + + if (PrefixPos == StringRef::npos) { + return; + } + + StringRef NewName = Name.drop_front(PrefixPos); + StringRef FName = Name.substr(0, PrefixPos - 1); + if (NewName.size() == 0) { + return; + } + + // This name should have a static linkage. + size_t PostfixPos = NewName.find(FunctionSamples::UniqSuffix); + bool ProfileHasFUnique = (PostfixPos != StringRef::npos); + + if (SampleProfileHasFUnique) { + // If profile also uses funqiue, nothing to do here. + // Otherwise, we need to build the map. + if (!ProfileHasFUnique) { + std::string NStr = + NewName.str() + getUniqueInternalLinkagePostfix(FName); + NewName = StringRef(NStr); + StaticFuncMap[NewName] = Name; + return; + } + } else { + // If profile does not use -funique-internal-linakge-symbols, nothing to + // do here. Otherwise, we need to trim. + if (ProfileHasFUnique) { + NewName = NewName.substr(0, PostfixPos); + } + } + + if (StaticFuncMap.find(NewName) == StaticFuncMap.end()) { + StaticFuncMap[NewName] = Name; + } else { + StaticFuncMap[NewName] = DuplicateNameStr; + } + }; + for (auto &PD : WC->Writer.getProfileData()) { // Populate IPBuilder. for (const auto &PDV : PD.getValue()) { @@ -553,7 +624,9 @@ // Initialize InstrProfileMap. InstrProfRecord *R = &PD.getValue().begin()->second; - InstrProfileMap[PD.getKey()] = InstrProfileEntry(R); + StringRef FullName = PD.getKey(); + InstrProfileMap[FullName] = InstrProfileEntry(R); + buildStaticFuncMap(FullName); } ProfileSummary InstrPS = *IPBuilder.getSummary(); @@ -581,16 +654,27 @@ // Find hot/warm functions in sample profile which is cold in instr profile // and adjust the profiles of those functions in the instr profile. for (const auto &PD : Reader->getProfiles()) { - auto &FContext = PD.first; const sampleprof::FunctionSamples &FS = PD.second; + if (FS.getMaxCountInside() <= ColdSampleThreshold || + FS.getBodySamples().size() < SupplMinSizeThreshold) + continue; + auto &FContext = PD.first; auto It = InstrProfileMap.find(FContext.toString()); - if (FS.getHeadSamples() > ColdSampleThreshold && - It != InstrProfileMap.end() && - It->second.MaxCount <= ColdInstrThreshold && - FS.getBodySamples().size() >= SupplMinSizeThreshold) { - updateInstrProfileEntry(It->second, HotInstrThreshold, - ZeroCounterThreshold); + if (It == InstrProfileMap.end()) { + auto NewName = StaticFuncMap.find(FContext.toString()); + if (NewName != StaticFuncMap.end()) { + It = InstrProfileMap.find(NewName->second.str()); + if (NewName->second == DuplicateNameStr) { + WithColor::warning() + << "Static function " << FContext.toString() + << " has multiple promoted names, cannot adjust profile.\n"; + } + } } + if (It == InstrProfileMap.end() || It->second.MaxCount > ColdInstrThreshold) + continue; + updateInstrProfileEntry(It->second, HotInstrThreshold, + ZeroCounterThreshold); } } Index: llvm/test/tools/llvm-profdata/suppl-instr-with-sample-static-func.test =================================================================== --- /dev/null +++ llvm/test/tools/llvm-profdata/suppl-instr-with-sample-static-func.test @@ -0,0 +1,15 @@ +Some basic tests for supplementing instrumentation profile with sample profile for static funcs. + +RUN: llvm-profdata merge -supplement-instr-with-sample=%p/Inputs/NoFUnique.afdotext -suppl-min-size-threshold=2 %p/Inputs/NoFUnique.proftext -o %t1 +RUN: llvm-profdata show -function=foo -counts %t1 | FileCheck %s + +RUN: llvm-profdata merge -supplement-instr-with-sample=%p/Inputs/FUnique.afdotext -suppl-min-size-threshold=2 %p/Inputs/FUnique.proftext -o %t2 +RUN: llvm-profdata show -function=foo -counts %t2 | FileCheck %s + +RUN: llvm-profdata merge -supplement-instr-with-sample=%p/Inputs/NoFUnique.afdotext -suppl-min-size-threshold=2 %p/Inputs/FUnique.proftext -o %t3 +RUN: llvm-profdata show -function=foo -counts %t3 | FileCheck %s + +RUN: llvm-profdata merge -supplement-instr-with-sample=%p/Inputs/FUnique.afdotext -suppl-min-size-threshold=2 %p/Inputs/NoFUnique.proftext -o %t4 +RUN: llvm-profdata show -function=foo -counts %t4 | FileCheck %s + +CHECK: Block counts: [18446744073709551615, 18446744073709551615, 18446744073709551615] Index: llvm/test/tools/llvm-profdata/Inputs/NoFUnique.proftext =================================================================== --- /dev/null +++ llvm/test/tools/llvm-profdata/Inputs/NoFUnique.proftext @@ -0,0 +1,30 @@ +# IR level Instrumentation Flag +:ir +_Z3barmi +# Func Hash: +784007056844089447 +# Num Counters: +2 +# Counter Values: +0 +0 + +main +# Func Hash: +784007059655560962 +# Num Counters: +2 +# Counter Values: +1 +0 + +test.c:_ZL3foom +# Func Hash: +1124680652115249575 +# Num Counters: +3 +# Counter Values: +0 +0 +0 + Index: llvm/test/tools/llvm-profdata/Inputs/NoFUnique.afdotext =================================================================== --- /dev/null +++ llvm/test/tools/llvm-profdata/Inputs/NoFUnique.afdotext @@ -0,0 +1,17 @@ +_ZL3foom:14064855:0 + 0: 0 + 2.1: 0 + 2.2: 290944 + 2.3073: 290944 + 3: 290944 + 4: 256 bar:256 + 5: 304048 bar:307919 + 7: 0 +_Z3barmi:6447198:308175 + 1: 302866 + 2: 5551 + 3: 296598 + 3.13824: 5551 + 3.2952803840: 296598 + 4: 5551 + 4.4160749568: 296598 Index: llvm/test/tools/llvm-profdata/Inputs/FUnique.proftext =================================================================== --- /dev/null +++ llvm/test/tools/llvm-profdata/Inputs/FUnique.proftext @@ -0,0 +1,30 @@ +# IR level Instrumentation Flag +:ir +_Z3barmi +# Func Hash: +784007056844089447 +# Num Counters: +2 +# Counter Values: +0 +0 + +main +# Func Hash: +784007059655560962 +# Num Counters: +2 +# Counter Values: +1 +0 + +test.c:_ZL3foom.__uniq.276699478366846449772231447066107882794 +# Func Hash: +1124680652115249575 +# Num Counters: +3 +# Counter Values: +0 +0 +0 + Index: llvm/test/tools/llvm-profdata/Inputs/FUnique.afdotext =================================================================== --- /dev/null +++ llvm/test/tools/llvm-profdata/Inputs/FUnique.afdotext @@ -0,0 +1,17 @@ +_ZL3foom.__uniq.276699478366846449772231447066107882794:14064855:0 + 0: 0 + 2.1: 0 + 2.2: 290944 + 2.3073: 290944 + 3: 290944 + 4: 256 bar:256 + 5: 304048 bar:307919 + 7: 0 +_Z3barmi:6447198:308175 + 1: 302866 + 2: 5551 + 3: 296598 + 3.13824: 5551 + 3.2952803840: 296598 + 4: 5551 + 4.4160749568: 296598 Index: llvm/include/llvm/ProfileData/SampleProf.h =================================================================== --- llvm/include/llvm/ProfileData/SampleProf.h +++ llvm/include/llvm/ProfileData/SampleProf.h @@ -16,6 +16,7 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/IR/Function.h" @@ -1331,6 +1332,26 @@ return LHS == RHS; } }; + +// Prepend "__uniq" before the hash for tools like profilers to understand +// that this symbol is of internal linkage type. The "__uniq" is the +// pre-determined prefix that is used to tell tools that this symbol was +// created with -funique-internal-linakge-symbols and the tools can strip or +// keep the prefix as needed. +inline std::string getUniqueInternalLinkagePostfix(const StringRef &FName) { + llvm::MD5 Md5; + Md5.update(FName); + llvm::MD5::MD5Result R; + Md5.final(R); + SmallString<32> Str; + llvm::MD5::stringifyResult(R, Str); + // Convert MD5hash to Decimal. Demangler suffixes can either contain + // numbers or characters but not both. + llvm::APInt IntHash(128, Str.str(), 16); + return toString(IntHash, /* Radix = */ 10, /* Signed = */ false) + .insert(0, FunctionSamples::UniqSuffix); +}; + } // end namespace llvm #endif // LLVM_PROFILEDATA_SAMPLEPROF_H Index: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -59,12 +59,12 @@ #include "llvm/IR/Module.h" #include "llvm/IR/ProfileSummary.h" #include "llvm/ProfileData/InstrProfReader.h" +#include "llvm/ProfileData/SampleProf.h" #include "llvm/Support/CRC.h" #include "llvm/Support/CodeGen.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/ErrorHandling.h" -#include "llvm/Support/MD5.h" #include "llvm/Support/TimeProfiler.h" #include "llvm/Support/X86TargetParser.h" @@ -206,22 +206,7 @@ Path = Entry.second + Path.substr(Entry.first.size()); break; } - llvm::MD5 Md5; - Md5.update(Path); - llvm::MD5::MD5Result R; - Md5.final(R); - SmallString<32> Str; - llvm::MD5::stringifyResult(R, Str); - // Convert MD5hash to Decimal. Demangler suffixes can either contain - // numbers or characters but not both. - llvm::APInt IntHash(128, Str.str(), 16); - // Prepend "__uniq" before the hash for tools like profilers to understand - // that this symbol is of internal linkage type. The "__uniq" is the - // pre-determined prefix that is used to tell tools that this symbol was - // created with -funique-internal-linakge-symbols and the tools can strip or - // keep the prefix as needed. - ModuleNameHash = (Twine(".__uniq.") + - Twine(toString(IntHash, /* Radix = */ 10, /* Signed = */false))).str(); + ModuleNameHash = llvm::getUniqueInternalLinkagePostfix(Path); } }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits