Author: William Junda Huang Date: 2024-08-28T21:33:24-04:00 New Revision: 92885bbeab632875929827a09841237cd59405fb
URL: https://github.com/llvm/llvm-project/commit/92885bbeab632875929827a09841237cd59405fb DIFF: https://github.com/llvm/llvm-project/commit/92885bbeab632875929827a09841237cd59405fb.diff LOG: Revert "[llvm-profdata] Enabled functionality to write split-layout profile (…" This reverts commit 75e9d191f52b047ea839f75ab2a7a7d9f8c6becd. Added: Modified: llvm/docs/CommandGuide/llvm-profdata.rst llvm/include/llvm/ProfileData/SampleProfReader.h llvm/include/llvm/ProfileData/SampleProfWriter.h llvm/lib/ProfileData/SampleProfReader.cpp llvm/lib/ProfileData/SampleProfWriter.cpp llvm/tools/llvm-profdata/llvm-profdata.cpp Removed: llvm/test/tools/llvm-profdata/Inputs/split-layout.profdata llvm/test/tools/llvm-profdata/sample-split-layout.test ################################################################################ diff --git a/llvm/docs/CommandGuide/llvm-profdata.rst b/llvm/docs/CommandGuide/llvm-profdata.rst index af840f3994b3d6..acf016a6dbcd70 100644 --- a/llvm/docs/CommandGuide/llvm-profdata.rst +++ b/llvm/docs/CommandGuide/llvm-profdata.rst @@ -162,12 +162,6 @@ OPTIONS coverage for the optimized target. This option can only be used with sample-based profile in extbinary format. -.. option:: --split-layout=[true|false] - - Split the profile data section to two with one containing sample profiles with - inlined functions and the other not. This option can only be used with - sample-based profile in extbinary format. - .. option:: --convert-sample-profile-layout=[nest|flat] Convert the merged profile into a profile with a new layout. Supported diff --git a/llvm/include/llvm/ProfileData/SampleProfReader.h b/llvm/include/llvm/ProfileData/SampleProfReader.h index 0fd86600de21f0..f053946a5db0a9 100644 --- a/llvm/include/llvm/ProfileData/SampleProfReader.h +++ b/llvm/include/llvm/ProfileData/SampleProfReader.h @@ -495,9 +495,9 @@ class SampleProfileReader { /// are present. virtual void setProfileUseMD5() { ProfileIsMD5 = true; } - /// Don't read profile without context if the flag is set. - void setSkipFlatProf(bool Skip) { SkipFlatProf = Skip; } - + /// 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; } @@ -581,10 +581,6 @@ class SampleProfileReader { /// Whether the profile uses MD5 for Sample Contexts and function names. This /// can be one-way overriden by the user to force use MD5. bool ProfileIsMD5 = false; - - /// If SkipFlatProf is true, skip functions marked with !Flat in text mode or - /// sections with SecFlagFlat flag in ExtBinary mode. - bool SkipFlatProf = false; }; class SampleProfileReaderText : public SampleProfileReader { @@ -793,6 +789,10 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary { /// The set containing the functions to use when compiling a module. DenseSet<StringRef> FuncsToUse; + /// If SkipFlatProf is true, skip the sections with + /// SecFlagFlat flag. + bool SkipFlatProf = false; + public: SampleProfileReaderExtBinaryBase(std::unique_ptr<MemoryBuffer> B, LLVMContext &C, SampleProfileFormat Format) @@ -815,6 +815,8 @@ class SampleProfileReaderExtBinaryBase : public SampleProfileReaderBinary { return std::move(ProfSymList); }; + void setSkipFlatProf(bool Skip) override { SkipFlatProf = Skip; } + private: /// Read the profiles on-demand for the given functions. This is used after /// stale call graph matching finds new functions whose profiles aren't loaded diff --git a/llvm/include/llvm/ProfileData/SampleProfWriter.h b/llvm/include/llvm/ProfileData/SampleProfWriter.h index 4b659eaf950b3e..5398a44f13ba36 100644 --- a/llvm/include/llvm/ProfileData/SampleProfWriter.h +++ b/llvm/include/llvm/ProfileData/SampleProfWriter.h @@ -28,9 +28,9 @@ namespace sampleprof { enum SectionLayout { DefaultLayout, - // The layout splits profile with inlined functions from profile without - // inlined functions. When Thinlto is enabled, ThinLTO postlink phase only - // has to load profile with inlined functions and can skip the other part. + // The layout splits profile with context information from profile without + // context information. When Thinlto is enabled, ThinLTO postlink phase only + // has to load profile with context information and can skip the other part. CtxSplitLayout, NumOfLayout, }; @@ -128,7 +128,7 @@ class SampleProfileWriter { virtual void setToCompressAllSections() {} virtual void setUseMD5() {} virtual void setPartialProfile() {} - virtual void setUseCtxSplitLayout() {} + virtual void resetSecLayout(SectionLayout SL) {} protected: SampleProfileWriter(std::unique_ptr<raw_ostream> &OS) @@ -176,22 +176,12 @@ class SampleProfileWriterText : public SampleProfileWriter { return sampleprof_error::success; } - void setUseCtxSplitLayout() override { - MarkFlatProfiles = true; - } - private: /// Indent level to use when writing. /// /// This is used when printing inlined callees. unsigned Indent = 0; - /// If set, writes metadata "!Flat" to functions without inlined functions. - /// This flag is for manual inspection only, it has no effect for the profile - /// reader because a text sample profile is read sequentially and functions - /// cannot be skipped. - bool MarkFlatProfiles = false; - friend ErrorOr<std::unique_ptr<SampleProfileWriter>> SampleProfileWriter::create(std::unique_ptr<raw_ostream> &OS, SampleProfileFormat Format); @@ -252,11 +242,11 @@ const std::array<SmallVector<SecHdrTableEntry, 8>, NumOfLayout> // CtxSplitLayout SmallVector<SecHdrTableEntry, 8>({{SecProfSummary, 0, 0, 0, 0}, {SecNameTable, 0, 0, 0, 0}, - // profile with inlined functions + // profile with context // for next two sections {SecFuncOffsetTable, 0, 0, 0, 0}, {SecLBRProfile, 0, 0, 0, 0}, - // profile without inlined functions + // profile without context // for next two sections {SecFuncOffsetTable, 0, 0, 0, 0}, {SecLBRProfile, 0, 0, 0, 0}, @@ -293,11 +283,7 @@ class SampleProfileWriterExtBinaryBase : public SampleProfileWriterBinary { ProfSymList = PSL; }; - void setUseCtxSplitLayout() override { - resetSecLayout(SectionLayout::CtxSplitLayout); - } - - void resetSecLayout(SectionLayout SL) { + void resetSecLayout(SectionLayout SL) override { verifySecLayout(SL); #ifndef NDEBUG // Make sure resetSecLayout is called before any flag setting. diff --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp index 98c78443785275..71464e8dae65ce 100644 --- a/llvm/lib/ProfileData/SampleProfReader.cpp +++ b/llvm/lib/ProfileData/SampleProfReader.cpp @@ -215,8 +215,7 @@ static bool ParseLine(const StringRef &Input, LineType &LineTy, uint32_t &Depth, uint64_t &NumSamples, uint32_t &LineOffset, uint32_t &Discriminator, StringRef &CalleeName, DenseMap<StringRef, uint64_t> &TargetCountMap, - uint64_t &FunctionHash, uint32_t &Attributes, - bool &IsFlat) { + uint64_t &FunctionHash, uint32_t &Attributes) { for (Depth = 0; Input[Depth] == ' '; Depth++) ; if (Depth == 0) @@ -224,13 +223,6 @@ static bool ParseLine(const StringRef &Input, LineType &LineTy, uint32_t &Depth, if (Input[Depth] == '!') { LineTy = LineType::Metadata; - // This metadata is only for manual inspection only. We already created a - // FunctionSamples and put it in the profile map, so there is no point - // to skip profiles even they have no use for ThinLTO. - if (Input == StringRef(" !Flat")) { - IsFlat = true; - return true; - } return parseMetadata(Input.substr(Depth), FunctionHash, Attributes); } @@ -333,8 +325,6 @@ std::error_code SampleProfileReaderText::readImpl() { // top-level or nested function profile. uint32_t DepthMetadata = 0; - std::vector<SampleContext *> FlatSamples; - ProfileIsFS = ProfileIsFSDisciminator; FunctionSamples::ProfileIsFS = ProfileIsFS; for (; !LineIt.is_at_eof(); ++LineIt) { @@ -378,10 +368,9 @@ std::error_code SampleProfileReaderText::readImpl() { LineType LineTy; uint64_t FunctionHash = 0; uint32_t Attributes = 0; - bool IsFlat = false; if (!ParseLine(*LineIt, LineTy, Depth, NumSamples, LineOffset, Discriminator, FName, TargetCountMap, FunctionHash, - Attributes, IsFlat)) { + Attributes)) { reportError(LineIt.line_number(), "Expected 'NUM[.NUM]: NUM[ mangled_name:NUM]*', found " + *LineIt); @@ -437,26 +426,12 @@ std::error_code SampleProfileReaderText::readImpl() { if (Attributes & (uint32_t)ContextShouldBeInlined) ProfileIsPreInlined = true; DepthMetadata = Depth; - if (IsFlat) { - if (Depth == 1) - FlatSamples.push_back(&FProfile.getContext()); - else - Ctx.diagnose(DiagnosticInfoSampleProfile( - Buffer->getBufferIdentifier(), LineIt.line_number(), - "!Flat may only be used at top level function.", DS_Warning)); - } break; } } } } - // Honor the option to skip flat functions. Since they are already added to - // the profile map, remove them all here. - if (SkipFlatProf) - for (SampleContext *FlatSample : FlatSamples) - Profiles.erase(*FlatSample); - assert((CSProfileCount == 0 || CSProfileCount == Profiles.size()) && "Cannot have both context-sensitive and regular profile"); ProfileIsCS = (CSProfileCount > 0); @@ -1051,7 +1026,7 @@ std::error_code SampleProfileReaderExtBinaryBase::readImpl() { if (!Entry.Size) continue; - // Skip sections without inlined functions when SkipFlatProf is true. + // Skip sections without context when SkipFlatProf is true. if (SkipFlatProf && hasSecFlag(Entry, SecCommonFlags::SecFlagFlat)) continue; diff --git a/llvm/lib/ProfileData/SampleProfWriter.cpp b/llvm/lib/ProfileData/SampleProfWriter.cpp index 8ded44fdc073b8..1630fefb4fcfba 100644 --- a/llvm/lib/ProfileData/SampleProfWriter.cpp +++ b/llvm/lib/ProfileData/SampleProfWriter.cpp @@ -624,9 +624,6 @@ std::error_code SampleProfileWriterText::writeSample(const FunctionSamples &S) { LineCount++; } - if (Indent == 0 && MarkFlatProfiles && S.getCallsiteSamples().size() == 0) - OS << " !Flat\n"; - return sampleprof_error::success; } diff --git a/llvm/test/tools/llvm-profdata/Inputs/split-layout.profdata b/llvm/test/tools/llvm-profdata/Inputs/split-layout.profdata deleted file mode 100644 index bc473ae76558f1..00000000000000 Binary files a/llvm/test/tools/llvm-profdata/Inputs/split-layout.profdata and /dev/null diff er diff --git a/llvm/test/tools/llvm-profdata/sample-split-layout.test b/llvm/test/tools/llvm-profdata/sample-split-layout.test deleted file mode 100644 index a1927c80b2e29b..00000000000000 --- a/llvm/test/tools/llvm-profdata/sample-split-layout.test +++ /dev/null @@ -1,22 +0,0 @@ -RUN: llvm-profdata merge --sample --extbinary --split-layout %p/Inputs/sample-profile.proftext -o %t-output -RUN: diff %t-output %p/Inputs/split-layout.profdata - -RUN: llvm-profdata merge --sample --text --split-layout %t-output | FileCheck %s -CHECK: main:184019:0 -CHECK-NEXT: 4: 534 -CHECK-NEXT: 4.2: 534 -CHECK-NEXT: 5: 1075 -CHECK-NEXT: 5.1: 1075 -CHECK-NEXT: 6: 2080 -CHECK-NEXT: 7: 534 -CHECK-NEXT: 9: 2064 _Z3bari:1471 _Z3fooi:631 -CHECK-NEXT: 10: inline1:1000 -CHECK-NEXT: 1: 1000 -CHECK-NEXT: 10: inline2:2000 -CHECK-NEXT: 1: 2000 -CHECK-NEXT: _Z3bari:20301:1437 -CHECK-NEXT: 1: 1437 -CHECK-NEXT: !Flat -CHECK-NEXT: _Z3fooi:7711:610 -CHECK-NEXT: 1: 610 -CHECK-NEXT: !Flat diff --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp index 3b775de11e0405..1f6c4c604d57b5 100644 --- a/llvm/tools/llvm-profdata/llvm-profdata.cpp +++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp @@ -207,12 +207,6 @@ cl::opt<bool> GenPartialProfile( "gen-partial-profile", cl::init(false), cl::Hidden, cl::sub(MergeSubcommand), cl::desc("Generate a partial profile (only meaningful for -extbinary)")); -cl::opt<bool> SplitLayout( - "split-layout", cl::init(false), cl::Hidden, - cl::sub(MergeSubcommand), - cl::desc("Split the profile to two sections with one containing sample " - "profiles with inlined functions and the other without (only " - "meaningful for -extbinary)")); cl::opt<std::string> SupplInstrWithSample( "supplement-instr-with-sample", cl::init(""), cl::Hidden, cl::sub(MergeSubcommand), @@ -1473,13 +1467,6 @@ static void handleExtBinaryWriter(sampleprof::SampleProfileWriter &Writer, sampleprof::ProfileSymbolList &WriterList, bool CompressAllSections, bool UseMD5, bool GenPartialProfile) { - if (SplitLayout) { - if (OutputFormat == PF_Binary) - warn("-split-layout is ignored. Specify -extbinary to enable it"); - else - Writer.setUseCtxSplitLayout(); - } - populateProfileSymbolList(Buffer, WriterList); if (WriterList.size() > 0 && OutputFormat != PF_Ext_Binary) warn("Profile Symbol list is not empty but the output format is not " _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits