[llvm] c5da949 - Revert "[SampleFDO] Add flag for partial profile." show-prof-info.test breaks on some platforms.

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 7 12:55:52 PDT 2020


Author: Wei Mi
Date: 2020-04-07T12:54:51-07:00
New Revision: c5da949ae8ce241ff171b2a027e730f4fd34232e

URL: https://github.com/llvm/llvm-project/commit/c5da949ae8ce241ff171b2a027e730f4fd34232e
DIFF: https://github.com/llvm/llvm-project/commit/c5da949ae8ce241ff171b2a027e730f4fd34232e.diff

LOG: Revert "[SampleFDO] Add flag for partial profile." show-prof-info.test breaks on some platforms.

This reverts commit e3ba652a1440794eff0b43ce747f1b0488585d22.

Added: 
    llvm/test/tools/llvm-profdata/show-prof-size.test

Modified: 
    llvm/include/llvm/IR/ProfileSummary.h
    llvm/include/llvm/ProfileData/SampleProf.h
    llvm/include/llvm/ProfileData/SampleProfWriter.h
    llvm/lib/ProfileData/SampleProfReader.cpp
    llvm/tools/llvm-profdata/llvm-profdata.cpp

Removed: 
    llvm/test/tools/llvm-profdata/show-prof-info.test


################################################################################
diff  --git a/llvm/include/llvm/IR/ProfileSummary.h b/llvm/include/llvm/IR/ProfileSummary.h
index 7532b44edd98..78635ec4386c 100644
--- a/llvm/include/llvm/IR/ProfileSummary.h
+++ b/llvm/include/llvm/IR/ProfileSummary.h
@@ -49,12 +49,6 @@ class ProfileSummary {
   SummaryEntryVector DetailedSummary;
   uint64_t TotalCount, MaxCount, MaxInternalCount, MaxFunctionCount;
   uint32_t NumCounts, NumFunctions;
-  /// If 'Partial' is false, it means the profile being used to optimize
-  /// a target is collected from the same target.
-  /// If 'Partial' is true, it means the profile is for common/shared
-  /// code. The common profile is usually merged from profiles collected
-  /// from running other targets.
-  bool Partial = false;
   /// Return detailed summary as metadata.
   Metadata *getDetailedSummaryMD(LLVMContext &Context);
 
@@ -82,8 +76,6 @@ class ProfileSummary {
   uint64_t getTotalCount() { return TotalCount; }
   uint64_t getMaxCount() { return MaxCount; }
   uint64_t getMaxInternalCount() { return MaxInternalCount; }
-  void setPartialProfile(bool PP) { Partial = PP; }
-  bool isPartialProfile() { return Partial; }
 };
 
 } // end namespace llvm

diff  --git a/llvm/include/llvm/ProfileData/SampleProf.h b/llvm/include/llvm/ProfileData/SampleProf.h
index f1ddbe3ba579..7b1086c74919 100644
--- a/llvm/include/llvm/ProfileData/SampleProf.h
+++ b/llvm/include/llvm/ProfileData/SampleProf.h
@@ -169,13 +169,6 @@ enum class SecNameTableFlags : uint32_t {
   SecFlagInValid = 0,
   SecFlagMD5Name = (1 << 0)
 };
-enum class SecProfSummaryFlags : uint32_t {
-  SecFlagInValid = 0,
-  /// SecFlagPartial means the profile is for common/shared code.
-  /// The common profile is usually merged from profiles collected
-  /// from running other targets.
-  SecFlagPartial = (1 << 0)
-};
 
 // Verify section specific flag is used for the correct section.
 template <class SecFlagType>
@@ -190,9 +183,6 @@ static inline void verifySecFlag(SecType Type, SecFlagType Flag) {
   case SecNameTable:
     IsFlagLegal = std::is_same<SecNameTableFlags, SecFlagType>();
     break;
-  case SecProfSummary:
-    IsFlagLegal = std::is_same<SecProfSummaryFlags, SecFlagType>();
-    break;
   default:
     break;
   }

diff  --git a/llvm/include/llvm/ProfileData/SampleProfWriter.h b/llvm/include/llvm/ProfileData/SampleProfWriter.h
index 7d0df9e44f58..54500f513ad1 100644
--- a/llvm/include/llvm/ProfileData/SampleProfWriter.h
+++ b/llvm/include/llvm/ProfileData/SampleProfWriter.h
@@ -59,7 +59,6 @@ class SampleProfileWriter {
   virtual void setProfileSymbolList(ProfileSymbolList *PSL) {}
   virtual void setToCompressAllSections() {}
   virtual void setUseMD5() {}
-  virtual void setPartialProfile() {}
 
 protected:
   SampleProfileWriter(std::unique_ptr<raw_ostream> &OS)
@@ -218,13 +217,6 @@ class SampleProfileWriterExtBinary : public SampleProfileWriterExtBinaryBase {
     addSectionFlag(SecNameTable, SecNameTableFlags::SecFlagMD5Name);
   }
 
-  // Set the profile to be partial. It means the profile is for
-  // common/shared code. The common profile is usually merged from
-  // profiles collected from running other targets.
-  virtual void setPartialProfile() override {
-    addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagPartial);
-  }
-
 private:
   virtual void initSectionHdrLayout() override {
     // Note that SecFuncOffsetTable section is written after SecLBRProfile

diff  --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index e27bf0771b14..33efb51dca07 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -478,8 +478,6 @@ std::error_code SampleProfileReaderExtBinary::readOneSection(
   case SecProfSummary:
     if (std::error_code EC = readSummary())
       return EC;
-    if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagPartial))
-      Summary->setPartialProfile(true);
     break;
   case SecNameTable:
     if (std::error_code EC = readNameTableSec(
@@ -833,40 +831,11 @@ uint64_t SampleProfileReaderExtBinaryBase::getFileSize() {
   return FileSize;
 }
 
-static std::string getSecFlagsStr(const SecHdrTableEntry &Entry) {
-  std::string Flags;
-  if (hasSecFlag(Entry, SecCommonFlags::SecFlagCompress))
-    Flags.append("{compressed,");
-  else
-    Flags.append("{");
-
-  switch (Entry.Type) {
-  case SecNameTable:
-    if (hasSecFlag(Entry, SecNameTableFlags::SecFlagMD5Name))
-      Flags.append("md5,");
-    break;
-  case SecProfSummary:
-    if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagPartial))
-      Flags.append("partial,");
-    break;
-  default:
-    break;
-  }
-  char &last = Flags.back();
-  if (last == ',')
-    last = '}';
-  else
-    Flags.append("}");
-  return Flags;
-}
-
 bool SampleProfileReaderExtBinaryBase::dumpSectionInfo(raw_ostream &OS) {
   uint64_t TotalSecsSize = 0;
   for (auto &Entry : SecHdrTable) {
     OS << getSecName(Entry.Type) << " - Offset: " << Entry.Offset
-       << ", Size: " << Entry.Size << ", Flags: " << getSecFlagsStr(Entry)
-       << "\n";
-    ;
+       << ", Size: " << Entry.Size << "\n";
     TotalSecsSize += getSectionSize(Entry.Type);
   }
   uint64_t HeaderSize = SecHdrTable.front().Offset;

diff  --git a/llvm/test/tools/llvm-profdata/show-prof-info.test b/llvm/test/tools/llvm-profdata/show-prof-info.test
deleted file mode 100644
index 3016ac26d31d..000000000000
--- a/llvm/test/tools/llvm-profdata/show-prof-info.test
+++ /dev/null
@@ -1,11 +0,0 @@
-; RUN: llvm-profdata merge -sample -extbinary -use-md5 -compress-all-sections -partial-profile -prof-sym-list=%S/Inputs/profile-symbol-list-1.text %S/Inputs/sample-profile.proftext -o %t.1.output
-; RUN: wc -c < %t.1.output > %t.txt
-; RUN: llvm-profdata show -sample -show-sec-info-only %t.1.output >> %t.txt
-; RUN: FileCheck %s --input-file=%t.txt
-; CHECK: [[FILESIZE:.*]]
-; To check llvm-profdata shows the correct flags for ProfileSummarySection.
-; CHECK: ProfileSummarySection {{.*}} Flags: {compressed,partial}
-; To check llvm-profdata shows the correct flags for NameTableSection.
-; CHECK: NameTableSection {{.*}} Flags: {compressed,md5}
-; To check llvm-profdata shows the correct file size.
-; CHECK: [[FILESIZE]]

diff  --git a/llvm/test/tools/llvm-profdata/show-prof-size.test b/llvm/test/tools/llvm-profdata/show-prof-size.test
new file mode 100644
index 000000000000..a77360c8a673
--- /dev/null
+++ b/llvm/test/tools/llvm-profdata/show-prof-size.test
@@ -0,0 +1,7 @@
+; RUN: llvm-profdata merge -sample -extbinary -prof-sym-list=%S/Inputs/profile-symbol-list-1.text %S/Inputs/sample-profile.proftext -o %t.1.output
+; RUN: wc -c < %t.1.output > %t.txt
+; RUN: llvm-profdata show -sample -show-sec-info-only %t.1.output >> %t.txt
+; RUN: FileCheck %s --input-file=%t.txt
+; Check llvm-profdata shows the correct file size.
+; CHECK: [[FILESIZE:.*]]
+; CHECK: [[FILESIZE]]

diff  --git a/llvm/tools/llvm-profdata/llvm-profdata.cpp b/llvm/tools/llvm-profdata/llvm-profdata.cpp
index 482bd1c842d1..90ce8400c845 100644
--- a/llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ b/llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -448,8 +448,7 @@ static void handleExtBinaryWriter(sampleprof::SampleProfileWriter &Writer,
                                   ProfileFormat OutputFormat,
                                   MemoryBuffer *Buffer,
                                   sampleprof::ProfileSymbolList &WriterList,
-                                  bool CompressAllSections, bool UseMD5,
-                                  bool PartialProfile) {
+                                  bool CompressAllSections, bool UseMD5) {
   populateProfileSymbolList(Buffer, WriterList);
   if (WriterList.size() > 0 && OutputFormat != PF_Ext_Binary)
     warn("Profile Symbol list is not empty but the output format is not "
@@ -469,19 +468,13 @@ static void handleExtBinaryWriter(sampleprof::SampleProfileWriter &Writer,
     else
       Writer.setUseMD5();
   }
-  if (PartialProfile) {
-    if (OutputFormat != PF_Ext_Binary)
-      warn("-partial-profile is ignored. Specify -extbinary to enable it");
-    else
-      Writer.setPartialProfile();
-  }
 }
 
 static void
 mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
                    StringRef OutputFilename, ProfileFormat OutputFormat,
                    StringRef ProfileSymbolListFile, bool CompressAllSections,
-                   bool UseMD5, bool PartialProfile, FailureMode FailMode) {
+                   bool UseMD5, FailureMode FailMode) {
   using namespace sampleprof;
   StringMap<FunctionSamples> ProfileMap;
   SmallVector<std::unique_ptr<sampleprof::SampleProfileReader>, 5> Readers;
@@ -538,7 +531,7 @@ mergeSampleProfile(const WeightedFileVector &Inputs, SymbolRemapper *Remapper,
   // Make sure Buffer lives as long as WriterList.
   auto Buffer = getInputFileBuf(ProfileSymbolListFile);
   handleExtBinaryWriter(*Writer, OutputFormat, Buffer.get(), WriterList,
-                        CompressAllSections, UseMD5, PartialProfile);
+                        CompressAllSections, UseMD5);
   Writer->write(ProfileMap);
 }
 
@@ -670,10 +663,6 @@ static int merge_main(int argc, const char *argv[]) {
       "use-md5", cl::init(false), cl::Hidden,
       cl::desc("Choose to use MD5 to represent string in name table (only "
                "meaningful for -extbinary)"));
-  cl::opt<bool> PartialProfile(
-      "partial-profile", cl::init(false), cl::Hidden,
-      cl::desc("Set the profile to be a partial profile (only meaningful "
-               "for -extbinary)"));
 
   cl::ParseCommandLineOptions(argc, argv, "LLVM profile data merger\n");
 
@@ -708,7 +697,7 @@ static int merge_main(int argc, const char *argv[]) {
   else
     mergeSampleProfile(WeightedInputs, Remapper.get(), OutputFilename,
                        OutputFormat, ProfileSymbolListFile, CompressAllSections,
-                       UseMD5, PartialProfile, FailureMode);
+                       UseMD5, FailureMode);
 
   return 0;
 }


        


More information about the llvm-commits mailing list