llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-pgo Author: Zequan Wu (ZequanWu) <details> <summary>Changes</summary> Refactor some code from https://github.com/llvm/llvm-project/pull/69493. #<!-- -->70712 was reverted due to linking failures. So, I removed `-profile-correlate=` flag and kept `-debug-info-correlate` in this change. --- Patch is 21.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/70856.diff 11 Files Affected: - (modified) compiler-rt/lib/profile/InstrProfiling.c (+4) - (modified) compiler-rt/lib/profile/InstrProfiling.h (+6) - (modified) compiler-rt/lib/profile/InstrProfilingBuffer.c (+11) - (modified) compiler-rt/lib/profile/InstrProfilingMerge.c (+6-5) - (modified) compiler-rt/lib/profile/InstrProfilingWriter.c (+10-11) - (modified) compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c (+1-1) - (modified) llvm/include/llvm/ProfileData/InstrProfCorrelator.h (+10-3) - (modified) llvm/include/llvm/ProfileData/InstrProfReader.h (+2) - (modified) llvm/lib/ProfileData/InstrProfCorrelator.cpp (+63-37) - (modified) llvm/lib/ProfileData/InstrProfReader.cpp (+2-2) - (modified) llvm/tools/llvm-profdata/llvm-profdata.cpp (+6-3) ``````````diff diff --git a/compiler-rt/lib/profile/InstrProfiling.c b/compiler-rt/lib/profile/InstrProfiling.c index da04d8ebdec95bb..7d69e37815c948f 100644 --- a/compiler-rt/lib/profile/InstrProfiling.c +++ b/compiler-rt/lib/profile/InstrProfiling.c @@ -89,3 +89,7 @@ COMPILER_RT_VISIBILITY void __llvm_profile_reset_counters(void) { } lprofSetProfileDumped(0); } + +inline int hasCorrelation() { + return (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) != 0ULL; +} diff --git a/compiler-rt/lib/profile/InstrProfiling.h b/compiler-rt/lib/profile/InstrProfiling.h index e143149fca82707..b8104af5f12b910 100644 --- a/compiler-rt/lib/profile/InstrProfiling.h +++ b/compiler-rt/lib/profile/InstrProfiling.h @@ -261,6 +261,9 @@ uint64_t __llvm_profile_get_magic(void); /*! \brief Get the version of the file format. */ uint64_t __llvm_profile_get_version(void); +/*! \brief If the binary is compiled with profile correlation. */ +int hasCorrelation(); + /*! \brief Get the number of entries in the profile data section. */ uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin, const __llvm_profile_data *End); @@ -282,6 +285,9 @@ uint64_t __llvm_profile_get_counters_size(const char *Begin, const char *End); uint64_t __llvm_profile_get_num_bitmap_bytes(const char *Begin, const char *End); +/*! \brief Get the size of the profile name section in bytes. */ +uint64_t __llvm_profile_get_name_size(const char *Begin, const char *End); + /* ! \brief Given the sizes of the data and counter information, return the * number of padding bytes before and after the counters, and after the names, * in the raw profile. diff --git a/compiler-rt/lib/profile/InstrProfilingBuffer.c b/compiler-rt/lib/profile/InstrProfilingBuffer.c index c7217b2dfef8a97..69965142d978746 100644 --- a/compiler-rt/lib/profile/InstrProfilingBuffer.c +++ b/compiler-rt/lib/profile/InstrProfilingBuffer.c @@ -56,6 +56,8 @@ uint64_t __llvm_profile_get_size_for_buffer(void) { COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin, const __llvm_profile_data *End) { + if (hasCorrelation()) + return 0; intptr_t BeginI = (intptr_t)Begin, EndI = (intptr_t)End; return ((EndI + sizeof(__llvm_profile_data) - 1) - BeginI) / sizeof(__llvm_profile_data); @@ -64,6 +66,8 @@ uint64_t __llvm_profile_get_num_data(const __llvm_profile_data *Begin, COMPILER_RT_VISIBILITY uint64_t __llvm_profile_get_data_size(const __llvm_profile_data *Begin, const __llvm_profile_data *End) { + if (hasCorrelation()) + return 0; return __llvm_profile_get_num_data(Begin, End) * sizeof(__llvm_profile_data); } @@ -92,6 +96,13 @@ uint64_t __llvm_profile_get_num_bitmap_bytes(const char *Begin, return (End - Begin); } +COMPILER_RT_VISIBILITY +uint64_t __llvm_profile_get_name_size(const char *Begin, const char *End) { + if (hasCorrelation()) + return 0; + return End - Begin; +} + /// Calculate the number of padding bytes needed to add to \p Offset in order /// for (\p Offset + Padding) to be page-aligned. static uint64_t calculateBytesNeededToPageAlign(uint64_t Offset) { diff --git a/compiler-rt/lib/profile/InstrProfilingMerge.c b/compiler-rt/lib/profile/InstrProfilingMerge.c index c5f168bf7517718..b08973debda94f3 100644 --- a/compiler-rt/lib/profile/InstrProfilingMerge.c +++ b/compiler-rt/lib/profile/InstrProfilingMerge.c @@ -69,8 +69,9 @@ int __llvm_profile_check_compatibility(const char *ProfileData, Header->NumBitmapBytes != __llvm_profile_get_num_bitmap_bytes(__llvm_profile_begin_bitmap(), __llvm_profile_end_bitmap()) || - Header->NamesSize != (uint64_t)(__llvm_profile_end_names() - - __llvm_profile_begin_names()) || + Header->NamesSize != + __llvm_profile_get_name_size(__llvm_profile_begin_names(), + __llvm_profile_end_names()) || Header->ValueKindLast != IPVK_Last) return 1; @@ -138,9 +139,9 @@ int __llvm_profile_merge_from_buffer(const char *ProfileData, if (SrcNameStart < SrcCountersStart || SrcNameStart < SrcBitmapStart) return 1; - // Merge counters by iterating the entire counter section when debug info - // correlation is enabled. - if (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) { + // Merge counters by iterating the entire counter section when correlation is + // enabled. + if (hasCorrelation()) { for (SrcCounter = SrcCountersStart, DstCounter = __llvm_profile_begin_counters(); SrcCounter < SrcCountersEnd;) { diff --git a/compiler-rt/lib/profile/InstrProfilingWriter.c b/compiler-rt/lib/profile/InstrProfilingWriter.c index 3b61f3def9f6ef0..d35ee6b20504f3e 100644 --- a/compiler-rt/lib/profile/InstrProfilingWriter.c +++ b/compiler-rt/lib/profile/InstrProfilingWriter.c @@ -262,21 +262,19 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin, const char *BitmapBegin, const char *BitmapEnd, VPDataReaderType *VPDataReader, const char *NamesBegin, const char *NamesEnd, int SkipNameDataWrite) { - int DebugInfoCorrelate = - (__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE) != 0ULL; + int ProfileCorrelation = hasCorrelation(); /* Calculate size of sections. */ const uint64_t DataSectionSize = - DebugInfoCorrelate ? 0 : __llvm_profile_get_data_size(DataBegin, DataEnd); - const uint64_t NumData = - DebugInfoCorrelate ? 0 : __llvm_profile_get_num_data(DataBegin, DataEnd); + __llvm_profile_get_data_size(DataBegin, DataEnd); + const uint64_t NumData = __llvm_profile_get_num_data(DataBegin, DataEnd); const uint64_t CountersSectionSize = __llvm_profile_get_counters_size(CountersBegin, CountersEnd); const uint64_t NumCounters = __llvm_profile_get_num_counters(CountersBegin, CountersEnd); const uint64_t NumBitmapBytes = __llvm_profile_get_num_bitmap_bytes(BitmapBegin, BitmapEnd); - const uint64_t NamesSize = DebugInfoCorrelate ? 0 : NamesEnd - NamesBegin; + const uint64_t NamesSize = __llvm_profile_get_name_size(NamesBegin, NamesEnd); /* Create the header. */ __llvm_profile_header Header; @@ -304,7 +302,7 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin, #endif /* The data and names sections are omitted in lightweight mode. */ - if (DebugInfoCorrelate) { + if (ProfileCorrelation) { Header.CountersDelta = 0; Header.NamesDelta = 0; } @@ -320,21 +318,22 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin, /* Write the profile data. */ ProfDataIOVec IOVecData[] = { - {DebugInfoCorrelate ? NULL : DataBegin, sizeof(uint8_t), DataSectionSize, + {ProfileCorrelation ? NULL : DataBegin, sizeof(uint8_t), DataSectionSize, 0}, {NULL, sizeof(uint8_t), PaddingBytesBeforeCounters, 1}, {CountersBegin, sizeof(uint8_t), CountersSectionSize, 0}, {NULL, sizeof(uint8_t), PaddingBytesAfterCounters, 1}, {BitmapBegin, sizeof(uint8_t), NumBitmapBytes, 0}, {NULL, sizeof(uint8_t), PaddingBytesAfterBitmapBytes, 1}, - {(SkipNameDataWrite || DebugInfoCorrelate) ? NULL : NamesBegin, + {(SkipNameDataWrite || ProfileCorrelation) ? NULL : NamesBegin, sizeof(uint8_t), NamesSize, 0}, {NULL, sizeof(uint8_t), PaddingBytesAfterNames, 1}}; if (Writer->Write(Writer, IOVecData, sizeof(IOVecData) / sizeof(*IOVecData))) return -1; - /* Value profiling is not yet supported in continuous mode. */ - if (__llvm_profile_is_continuous_mode_enabled()) + /* Value profiling is not yet supported in continuous mode and profile + * correlation mode. */ + if (__llvm_profile_is_continuous_mode_enabled() || ProfileCorrelation) return 0; return writeValueProfData(Writer, VPDataReader, DataBegin, DataEnd); diff --git a/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c b/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c index 226d678aca347a4..245dc798910425b 100644 --- a/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c +++ b/compiler-rt/test/profile/Linux/instrprof-show-debug-info-correlation.c @@ -4,7 +4,7 @@ // RUN: %clang_pgogen -o %t.no.dbg -mllvm --debug-info-correlate -mllvm --disable-vp=true %s // RUN: not llvm-profdata show --debug-info=%t.no.dbg 2>&1 | FileCheck %s --check-prefix NO-DBG -// NO-DBG: unable to correlate profile: could not find any profile metadata in debug info +// NO-DBG: unable to correlate profile: could not find any profile data metadata in correlated file // YAML: Probes: // YAML: - Function Name: a diff --git a/llvm/include/llvm/ProfileData/InstrProfCorrelator.h b/llvm/include/llvm/ProfileData/InstrProfCorrelator.h index dd062951f277c1c..a3a0805a294a20c 100644 --- a/llvm/include/llvm/ProfileData/InstrProfCorrelator.h +++ b/llvm/include/llvm/ProfileData/InstrProfCorrelator.h @@ -31,8 +31,11 @@ class ObjectFile; /// to their functions. class InstrProfCorrelator { public: + /// Indicate which kind correlator to use. + enum ProfCorrelatorKind { NONE, DEBUG_INFO }; + static llvm::Expected<std::unique_ptr<InstrProfCorrelator>> - get(StringRef DebugInfoFilename); + get(StringRef Filename, ProfCorrelatorKind FileKind); /// Construct a ProfileData vector used to correlate raw instrumentation data /// to their functions. @@ -104,7 +107,7 @@ class InstrProfCorrelator { private: static llvm::Expected<std::unique_ptr<InstrProfCorrelator>> - get(std::unique_ptr<MemoryBuffer> Buffer); + get(std::unique_ptr<MemoryBuffer> Buffer, ProfCorrelatorKind FileKind); const InstrProfCorrelatorKind Kind; }; @@ -128,7 +131,7 @@ class InstrProfCorrelatorImpl : public InstrProfCorrelator { static llvm::Expected<std::unique_ptr<InstrProfCorrelatorImpl<IntPtrT>>> get(std::unique_ptr<InstrProfCorrelator::Context> Ctx, - const object::ObjectFile &Obj); + const object::ObjectFile &Obj, ProfCorrelatorKind FileKind); protected: std::vector<RawInstrProf::ProfileData<IntPtrT>> Data; @@ -138,6 +141,8 @@ class InstrProfCorrelatorImpl : public InstrProfCorrelator { int MaxWarnings, InstrProfCorrelator::CorrelationData *Data = nullptr) = 0; + virtual Error correlateProfileNameImpl() = 0; + Error dumpYaml(int MaxWarnings, raw_ostream &OS) override; void addProbe(StringRef FunctionName, uint64_t CFGHash, IntPtrT CounterOffset, @@ -205,6 +210,8 @@ class DwarfInstrProfCorrelator : public InstrProfCorrelatorImpl<IntPtrT> { void correlateProfileDataImpl( int MaxWarnings, InstrProfCorrelator::CorrelationData *Data = nullptr) override; + + Error correlateProfileNameImpl() override; }; } // end namespace llvm diff --git a/llvm/include/llvm/ProfileData/InstrProfReader.h b/llvm/include/llvm/ProfileData/InstrProfReader.h index cf6429a324d36b3..1084a58eb098d12 100644 --- a/llvm/include/llvm/ProfileData/InstrProfReader.h +++ b/llvm/include/llvm/ProfileData/InstrProfReader.h @@ -377,6 +377,8 @@ class RawInstrProfReader : public InstrProfReader { return (Version & VARIANT_MASK_DBG_CORRELATE) != 0; } + bool useCorrelate() const { return useDebugInfoCorrelate(); } + bool hasSingleByteCoverage() const override { return (Version & VARIANT_MASK_BYTE_COVERAGE) != 0; } diff --git a/llvm/lib/ProfileData/InstrProfCorrelator.cpp b/llvm/lib/ProfileData/InstrProfCorrelator.cpp index 2138368500bed09..e429b9f1da54cba 100644 --- a/llvm/lib/ProfileData/InstrProfCorrelator.cpp +++ b/llvm/lib/ProfileData/InstrProfCorrelator.cpp @@ -24,15 +24,20 @@ using namespace llvm; -/// Get the __llvm_prf_cnts section. -Expected<object::SectionRef> getCountersSection(const object::ObjectFile &Obj) { +/// Get profile section. +Expected<object::SectionRef> getInstrProfSection(const object::ObjectFile &Obj, + InstrProfSectKind IPSK) { + Triple::ObjectFormatType ObjFormat = Obj.getTripleObjectFormat(); + std::string ExpectedSectionName = + getInstrProfSectionName(IPSK, ObjFormat, + /*AddSegmentInfo=*/false); for (auto &Section : Obj.sections()) if (auto SectionName = Section.getName()) - if (SectionName.get() == INSTR_PROF_CNTS_SECT_NAME) + if (SectionName.get() == ExpectedSectionName) return Section; return make_error<InstrProfError>( instrprof_error::unable_to_correlate_profile, - "could not find counter section (" INSTR_PROF_CNTS_SECT_NAME ")"); + "could not find section (" + Twine(ExpectedSectionName) + ")"); } const char *InstrProfCorrelator::FunctionNameAttributeName = "Function Name"; @@ -42,7 +47,7 @@ const char *InstrProfCorrelator::NumCountersAttributeName = "Num Counters"; llvm::Expected<std::unique_ptr<InstrProfCorrelator::Context>> InstrProfCorrelator::Context::get(std::unique_ptr<MemoryBuffer> Buffer, const object::ObjectFile &Obj) { - auto CountersSection = getCountersSection(Obj); + auto CountersSection = getInstrProfSection(Obj, IPSK_cnts); if (auto Err = CountersSection.takeError()) return std::move(Err); auto C = std::make_unique<Context>(); @@ -54,30 +59,32 @@ InstrProfCorrelator::Context::get(std::unique_ptr<MemoryBuffer> Buffer, } llvm::Expected<std::unique_ptr<InstrProfCorrelator>> -InstrProfCorrelator::get(StringRef DebugInfoFilename) { - auto DsymObjectsOrErr = - object::MachOObjectFile::findDsymObjectMembers(DebugInfoFilename); - if (auto Err = DsymObjectsOrErr.takeError()) - return std::move(Err); - if (!DsymObjectsOrErr->empty()) { - // TODO: Enable profile correlation when there are multiple objects in a - // dSYM bundle. - if (DsymObjectsOrErr->size() > 1) - return make_error<InstrProfError>( - instrprof_error::unable_to_correlate_profile, - "using multiple objects is not yet supported"); - DebugInfoFilename = *DsymObjectsOrErr->begin(); +InstrProfCorrelator::get(StringRef Filename, ProfCorrelatorKind FileKind) { + if (FileKind == DEBUG_INFO) { + auto DsymObjectsOrErr = + object::MachOObjectFile::findDsymObjectMembers(Filename); + if (auto Err = DsymObjectsOrErr.takeError()) + return std::move(Err); + if (!DsymObjectsOrErr->empty()) { + // TODO: Enable profile correlation when there are multiple objects in a + // dSYM bundle. + if (DsymObjectsOrErr->size() > 1) + return make_error<InstrProfError>( + instrprof_error::unable_to_correlate_profile, + "using multiple objects is not yet supported"); + Filename = *DsymObjectsOrErr->begin(); + } } - auto BufferOrErr = - errorOrToExpected(MemoryBuffer::getFile(DebugInfoFilename)); + auto BufferOrErr = errorOrToExpected(MemoryBuffer::getFile(Filename)); if (auto Err = BufferOrErr.takeError()) return std::move(Err); - return get(std::move(*BufferOrErr)); + return get(std::move(*BufferOrErr), FileKind); } llvm::Expected<std::unique_ptr<InstrProfCorrelator>> -InstrProfCorrelator::get(std::unique_ptr<MemoryBuffer> Buffer) { +InstrProfCorrelator::get(std::unique_ptr<MemoryBuffer> Buffer, + ProfCorrelatorKind FileKind) { auto BinOrErr = object::createBinary(*Buffer); if (auto Err = BinOrErr.takeError()) return std::move(Err); @@ -88,9 +95,11 @@ InstrProfCorrelator::get(std::unique_ptr<MemoryBuffer> Buffer) { return std::move(Err); auto T = Obj->makeTriple(); if (T.isArch64Bit()) - return InstrProfCorrelatorImpl<uint64_t>::get(std::move(*CtxOrErr), *Obj); + return InstrProfCorrelatorImpl<uint64_t>::get(std::move(*CtxOrErr), *Obj, + FileKind); if (T.isArch32Bit()) - return InstrProfCorrelatorImpl<uint32_t>::get(std::move(*CtxOrErr), *Obj); + return InstrProfCorrelatorImpl<uint32_t>::get(std::move(*CtxOrErr), *Obj, + FileKind); } return make_error<InstrProfError>( instrprof_error::unable_to_correlate_profile, "not an object file"); @@ -132,29 +141,33 @@ template <class IntPtrT> llvm::Expected<std::unique_ptr<InstrProfCorrelatorImpl<IntPtrT>>> InstrProfCorrelatorImpl<IntPtrT>::get( std::unique_ptr<InstrProfCorrelator::Context> Ctx, - const object::ObjectFile &Obj) { - if (Obj.isELF() || Obj.isMachO()) { - auto DICtx = DWARFContext::create(Obj); - return std::make_unique<DwarfInstrProfCorrelator<IntPtrT>>(std::move(DICtx), - std::move(Ctx)); + const object::ObjectFile &Obj, ProfCorrelatorKind FileKind) { + if (FileKind == DEBUG_INFO) { + if (Obj.isELF() || Obj.isMachO()) { + auto DICtx = DWARFContext::create(Obj); + return std::make_unique<DwarfInstrProfCorrelator<IntPtrT>>( + std::move(DICtx), std::move(Ctx)); + } + return make_error<InstrProfError>( + instrprof_error::unable_to_correlate_profile, + "unsupported debug info format (only DWARF is supported)"); } return make_error<InstrProfError>( instrprof_error::unable_to_correlate_profile, - "unsupported debug info format (only DWARF is supported)"); + "unsupported correlation file type (only DWARF is supported)"); } template <class IntPtrT> Error InstrProfCorrelatorImpl<IntPtrT>::correlateProfileData(int MaxWarnings) { assert(Data.empty() && Names.empty() && NamesVec.empty()); correlateProfileDataImpl(MaxWarnings); - if (Data.empty() || NamesVec.empty()) + if (this->Data.empty()) return make_error<InstrProfError>( instrprof_error::unable_to_correlate_profile, - "could not find any profile metadata in debug info"); - auto Result = - collectGlobalObjectNameStrings(NamesVec, /*doCompression=*/false, Names); - CounterOffsets.clear(); - NamesVec.clear(); + "could not find any profile data metadata in correlated file"); + Error Result = correlateProfileNameImpl(); + this->CounterOffsets.clear(); + this->NamesVec.clear(); return Result; } @@ -189,7 +202,7 @@ Error InstrProfCorrelatorImpl<IntPtrT>::dumpYaml(int MaxWarnings, if (Data.Probes.empty()) return make_error<InstrProfError>( instrprof_error::unable_to_correlate_profile, - "could not find any profile metadata in debug info"); + "could not find any profile data metadata in debug info"); yaml::Output YamlOS(OS); YamlOS << Data; return Error::success(); @@ -365,3 +378,16 @@ void DwarfInstrProfCorrelator<IntPtrT>::correlateProfileDataImpl( WithColor::warning() << format("Suppressed %d additional warnings\n", NumSuppressedWarnings); } + +template <class IntPtrT> +Error DwarfInstrProfCorrelator<IntPtrT>::correlateProfileNameImpl() { + if (this->NamesVec.empty()) { + return make_error<InstrProfError>( + instrprof_error::unable_to_correlate_profile, + "could not find any profile name metadata in debug info"); + } + auto Result = + collectGlobalObjectNameStrings(this->NamesVec, + /*doCompression=*/false, this->Names); + return Result; +} diff --git a/llvm/lib/ProfileData/InstrProfReader.cpp b/llvm/lib/ProfileData/InstrProfReader.cpp index 31d3ff2bcbf6ca7..dd66f4247ec73ea 100644 --- a/llvm/lib/ProfileData/InstrProfReader.cpp +++ b/llvm/lib/ProfileData/InstrProfReader.cpp @@ -563,9 +563,9 @@ Error RawInstrProfReader<IntPtrT>::readHeader( "\nPLEASE update this tool to version in the raw profile, or " "regenerate raw profile with expected version.") .str()); - if (useDebugInfoCorrelate() && !Correlator) + if (useCorrelate() && !Correlator) return error(instrprof_error::missing_debug_info_for_correlation); - if (!useDebugInfoCorrelate() && Correlator) + if (!useCorrelate() && Correlator) return error(instrprof_error::unexpected_debug_info_for_correlation); BinaryIdsSize = swap(Header.BinaryIdsSize); diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profda... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/70856 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits