Add a profile summary class specific to instrumented profiles.

[eraman]

Summary

Modify ProfileSummary class to make it not instrumented profile specific. Add a new InstrumentedProfileSummary class that inherits from ProfileSummary. This would avoid the ugliness of having to bases sample profile specific summary information in instrumented profiling terms.

I have also moved MaxFunctionCount to the derived class even though the notion of max of weights at function entry is relevant to SampleProfile. I want to keep the base class only about a set of weights, their count, sum, max and detailed summary.

[davidxl]

include/llvm/ProfileData/InstrProfReader.h

356

s/InstrumentedProfileSummary/InstrProfSummary/

include/llvm/ProfileData/ProfileCommon.h

36

I am not particularly fond of the term 'weight' here. # of samples in SampleFDO is basically sample count, so it seems perfectly fine to keep using 'Count'

37

Why not NumCounts (which is also better for Instr Profile than NumBlocks)?

82

Why is MaxFunctionCount specific to InstrProfile?

[eraman]

include/llvm/ProfileData/ProfileCommon.h

82

It is not. As I said in the summary, the base class computes and maintains useful statistics about a set of value (counts or samples). Since function count is a "special" value - a value that satisfies a particular property - I moved it to derived class.

[eraman]

Address David's comments

[davidxl]

lgtm.

include/llvm/ProfileData/InstrProf.h

struct Summary {
   void set(SummaryFieldKind K, uint64_t V) {
     getSummaryDataBase()[K] = V;
   }
 
   const Entry &getEntry(uint32_t I) const { return getCutoffEntryBase()[I]; }
   void setEntry(uint32_t I, const ProfileSummaryEntry &E) {
     Entry &ER = getCutoffEntryBase()[I];
     ER.Cutoff = E.Cutoff;
-    ER.MinBlockCount = E.MinBlockCount;
-    ER.NumBlocks = E.NumBlocks;
+    ER.MinBlockCount = E.MinCount;
+    ER.NumBlocks = E.NumCounts;
   }
 
   Summary(uint32_t Size) { memset(this, 0, Size); }
   void operator delete(void *ptr) { ::operator delete(ptr); }
 
   Summary() = delete;
 };
 

include/llvm/ProfileData/InstrProfReader.h

 /// Reader for the indexed binary instrprof format.
 class IndexedInstrProfReader : public InstrProfReader {
 private:
   /// The profile data file contents.
   std::unique_ptr<MemoryBuffer> DataBuffer;
   /// The index into the profile data.
   std::unique_ptr<InstrProfReaderIndexBase> Index;
   /// Profile summary data.
-  std::unique_ptr<ProfileSummary> Summary;
+  std::unique_ptr<InstrProfSummary> Summary;
 
   IndexedInstrProfReader(const IndexedInstrProfReader &) = delete;
   IndexedInstrProfReader &operator=(const IndexedInstrProfReader &) = delete;
 
   // Read the profile summary. Return a pointer pointing to one byte past the
   // end of the summary data if it exists or the input \c Cur.
   const unsigned char *readSummary(IndexedInstrProf::ProfVersion Version,
                                    const unsigned char *Cur);
public:
   void setValueProfDataEndianness(support::endianness Endianness) {
     Index->setValueProfDataEndianness(Endianness);
   }
 
   // See description in the base class. This interface is designed
   // to be used by llvm-profdata (for dumping). Avoid using this when
   // the client is the compiler.
   InstrProfSymtab &getSymtab() override;
-  ProfileSummary &getSummary() { return *(Summary.get()); }
+  InstrProfSummary &getSummary() { return *(Summary.get()); }
 };
 
 } // end namespace llvm
 
 #endif

include/llvm/ProfileData/ProfileCommon.h

 #ifndef LLVM_PROFILEDATA_PROFILE_COMMON_H
 #define LLVM_PROFILEDATA_PROFILE_COMMON_H
 
 namespace llvm {
 namespace IndexedInstrProf {
 struct Summary;
 }
 struct InstrProfRecord;
-///// Profile summary computation ////
-// The 'show' command displays richer summary of the profile data. The profile
-// summary is one or more (Cutoff, MinBlockCount, NumBlocks) triplets. Given a
-// target execution count percentile, we compute the minimum number of blocks
-// needed to reach this target and the minimum execution count of these blocks.
+// The profile summary is one or more (Cutoff, MinCount, NumCounts) triplets.
+// The semantics of counts depend on the type of profile. For instrumentation
+// profile, counts are block counts and for sample profile, counts are
+// per-line samples. Given a target counts percentile, we compute the minimum
+// number of counts needed to reach this target and the minimum among these
+// counts.
 struct ProfileSummaryEntry {
-  uint32_t Cutoff;        ///< The required percentile of total execution count.
-  uint64_t MinBlockCount; ///< The minimum execution count for this percentile.
-  uint64_t NumBlocks;     ///< Number of blocks >= the minumum execution count.
-  ProfileSummaryEntry(uint32_t TheCutoff, uint64_t TheMinBlockCount,
-                      uint64_t TheNumBlocks)
-      : Cutoff(TheCutoff), MinBlockCount(TheMinBlockCount),
-        NumBlocks(TheNumBlocks) {}
+  uint32_t Cutoff;    ///< The required percentile of counts.
+  uint64_t MinCount;  ///< The minimum count for this percentile.
+  uint64_t NumCounts; ///< Number of counts >= the minimum count.
+  ProfileSummaryEntry(uint32_t TheCutoff, uint64_t TheMinCount,
+                      uint64_t TheNumCounts)
+      : Cutoff(TheCutoff), MinCount(TheMinCount), NumCounts(TheNumCounts) {}
 };
 
 class ProfileSummary {
-  // We keep track of the number of times a count appears in the profile and
-  // keep the map sorted in the descending order of counts.
+  // We keep track of the number of times a count (block count or samples)
+  // appears in the profile. The map is kept sorted in the descending order of
+  // counts.
   std::map<uint64_t, uint32_t, std::greater<uint64_t>> CountFrequencies;
+
+protected:
   std::vector<ProfileSummaryEntry> DetailedSummary;
   std::vector<uint32_t> DetailedSummaryCutoffs;
-  // Sum of all counts.
-  uint64_t TotalCount;
-  uint64_t MaxBlockCount, MaxInternalBlockCount, MaxFunctionCount;
-  uint32_t NumBlocks, NumFunctions;
-  inline void addCount(uint64_t Count, bool IsEntry);
+  uint64_t TotalCount, MaxCount;
+  uint32_t NumCounts;
+  ProfileSummary(std::vector<uint32_t> Cutoffs)
+      : DetailedSummaryCutoffs(Cutoffs), TotalCount(0), MaxCount(0),
+        NumCounts(0) {}
+  ProfileSummary() : TotalCount(0), MaxCount(0), NumCounts(0) {}
+  inline void addCount(uint64_t Count);
 
 public:
   static const int Scale = 1000000;
-  ProfileSummary(std::vector<uint32_t> Cutoffs)
-      : DetailedSummaryCutoffs(Cutoffs), TotalCount(0), MaxBlockCount(0),
-        MaxInternalBlockCount(0), MaxFunctionCount(0), NumBlocks(0),
-        NumFunctions(0) {}
-  ProfileSummary(const IndexedInstrProf::Summary &S);
-  void addRecord(const InstrProfRecord &);
   inline std::vector<ProfileSummaryEntry> &getDetailedSummary();
   void computeDetailedSummary();
-  uint32_t getNumBlocks() { return NumBlocks; }
+};
+
+class InstrProfSummary : public ProfileSummary {
+  uint64_t MaxInternalBlockCount, MaxFunctionCount;
+  uint32_t NumFunctions;
+  inline void addEntryCount(uint64_t Count);
+  inline void addInternalCount(uint64_t Count);
+
+public:
+  InstrProfSummary(std::vector<uint32_t> Cutoffs)
+      : ProfileSummary(Cutoffs), MaxInternalBlockCount(0), MaxFunctionCount(0),
+        NumFunctions(0) {}
+  InstrProfSummary(const IndexedInstrProf::Summary &S);
+  void addRecord(const InstrProfRecord &);
+  uint32_t getNumBlocks() { return NumCounts; }
   uint64_t getTotalCount() { return TotalCount; }
   uint32_t getNumFunctions() { return NumFunctions; }
   uint64_t getMaxFunctionCount() { return MaxFunctionCount; }
-  uint64_t getMaxBlockCount() { return MaxBlockCount; }
+  uint64_t getMaxBlockCount() { return MaxCount; }
   uint64_t getMaxInternalBlockCount() { return MaxInternalBlockCount; }
 };
 
 // This is called when a count is seen in the profile.
-void ProfileSummary::addCount(uint64_t Count, bool IsEntry) {
+void ProfileSummary::addCount(uint64_t Count) {
   TotalCount += Count;
-  if (Count > MaxBlockCount)
-    MaxBlockCount = Count;
-  if (!IsEntry && Count > MaxInternalBlockCount)
-    MaxInternalBlockCount = Count;
-  NumBlocks++;
+  if (Count > MaxCount)
+    MaxCount = Count;
+  NumCounts++;
   CountFrequencies[Count]++;
 }
 
 std::vector<ProfileSummaryEntry> &ProfileSummary::getDetailedSummary() {
   if (!DetailedSummaryCutoffs.empty() && DetailedSummary.empty())
     computeDetailedSummary();
   return DetailedSummary;
 }
 
 } // end namespace llvm
 #endif

lib/ProfileData/InstrProfReader.cpp

if (Version >= IndexedInstrProf::Version4) {
     std::unique_ptr<IndexedInstrProf::Summary> SummaryData =
         IndexedInstrProf::allocSummary(SummarySize);
 
     const uint64_t *Src = reinterpret_cast<const uint64_t *>(SummaryInLE);
     uint64_t *Dst = reinterpret_cast<uint64_t *>(SummaryData.get());
     for (unsigned I = 0; I < SummarySize / sizeof(uint64_t); I++)
       Dst[I] = endian::byte_swap<uint64_t, little>(Src[I]);
 
-    // initialize ProfileSummary using the SummaryData from disk.
-    this->Summary = llvm::make_unique<ProfileSummary>(*(SummaryData.get()));
+    // initialize InstrProfSummary using the SummaryData from disk.
+    this->Summary = llvm::make_unique<InstrProfSummary>(*(SummaryData.get()));
     return Cur + SummarySize;
   } else {
     // For older version of profile data, we need to compute on the fly:
     using namespace IndexedInstrProf;
     std::vector<uint32_t> Cutoffs(&SummaryCutoffs[0],
                                   &SummaryCutoffs[NumSummaryCutoffs]);
-    this->Summary = llvm::make_unique<ProfileSummary>(Cutoffs);
+    this->Summary = llvm::make_unique<InstrProfSummary>(Cutoffs);
     this->Summary->computeDetailedSummary();
     return Cur;
   }
 }
 
 std::error_code IndexedInstrProfReader::readHeader() {
   const unsigned char *Start =
       (const unsigned char *)DataBuffer->getBufferStart();

lib/ProfileData/InstrProfWriter.cpp

public:
 
   typedef const InstrProfWriter::ProfilingData *const data_type;
   typedef const InstrProfWriter::ProfilingData *const data_type_ref;
 
   typedef uint64_t hash_value_type;
   typedef uint64_t offset_type;
 
   support::endianness ValueProfDataEndianness;
-  ProfileSummary *TheProfileSummary;
+  InstrProfSummary *TheProfileSummary;
 
   InstrProfRecordWriterTrait() : ValueProfDataEndianness(support::little) {}
   static hash_value_type ComputeHash(key_type_ref K) {
     return IndexedInstrProf::ComputeHash(K);
   }
 
   static std::pair<offset_type, offset_type>
   EmitKeyDataLength(raw_ostream &Out, key_type_ref K, data_type_ref V) {
for (const auto &Func : PD) {
     if (std::any_of(IPR.Counts.begin(), IPR.Counts.end(),
                     [](uint64_t Count) { return Count > 0; }))
       return true;
   }
   return false;
 }
 
 static void setSummary(IndexedInstrProf::Summary *TheSummary,
-                       ProfileSummary &PS) {
+                       InstrProfSummary &PS) {
   using namespace IndexedInstrProf;
   std::vector<ProfileSummaryEntry> &Res = PS.getDetailedSummary();
   TheSummary->NumSummaryFields = Summary::NumKinds;
   TheSummary->NumCutoffEntries = Res.size();
   TheSummary->set(Summary::MaxFunctionCount, PS.getMaxFunctionCount());
   TheSummary->set(Summary::MaxBlockCount, PS.getMaxBlockCount());
   TheSummary->set(Summary::MaxInternalBlockCount,
                   PS.getMaxInternalBlockCount());
   TheSummary->set(Summary::TotalBlockCount, PS.getTotalCount());
   TheSummary->set(Summary::TotalNumBlocks, PS.getNumBlocks());
   TheSummary->set(Summary::TotalNumFunctions, PS.getNumFunctions());
   for (unsigned I = 0; I < Res.size(); I++)
     TheSummary->setEntry(I, Res[I]);
 }
 
 void InstrProfWriter::writeImpl(ProfOStream &OS) {
   OnDiskChainedHashTableGenerator<InstrProfRecordWriterTrait> Generator;
 
   using namespace IndexedInstrProf;
   std::vector<uint32_t> Cutoffs(&SummaryCutoffs[0],
                                 &SummaryCutoffs[NumSummaryCutoffs]);
-  ProfileSummary PS(Cutoffs);
+  InstrProfSummary PS(Cutoffs);
   InfoObj->TheProfileSummary = &PS;
 
   // Populate the hash table generator.
   for (const auto &I : FunctionData)
     if (shouldEncodeData(I.getValue()))
       Generator.insert(I.getKey(), &I.getValue());
   // Write the header.
   IndexedInstrProf::Header Header;

lib/ProfileData/ProfileSummary.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ProfileData/ProfileCommon.h"
 #include "llvm/ProfileData/InstrProf.h"
 
 using namespace llvm;
 
-void ProfileSummary::addRecord(const InstrProfRecord &R) {
-  NumFunctions++;
-  if (R.Counts[0] > MaxFunctionCount)
-    MaxFunctionCount = R.Counts[0];
-
-  for (size_t I = 0, E = R.Counts.size(); I < E; ++I)
-    addCount(R.Counts[I], (I == 0));
+void InstrProfSummary::addRecord(const InstrProfRecord &R) {
+  addEntryCount(R.Counts[0]);
+  for (size_t I = 1, E = R.Counts.size(); I < E; ++I)
+    addInternalCount(R.Counts[I]);
 }
 
 // The argument to this method is a vector of cutoff percentages and the return
-// value is a vector of (Cutoff, MinBlockCount, NumBlocks) triplets.
+// value is a vector of (Cutoff, MinCount, NumCounts) triplets.
 void ProfileSummary::computeDetailedSummary() {
   if (DetailedSummaryCutoffs.empty())
     return;
   auto Iter = CountFrequencies.begin();
   auto End = CountFrequencies.end();
   std::sort(DetailedSummaryCutoffs.begin(), DetailedSummaryCutoffs.end());
 
-  uint32_t BlocksSeen = 0;
+  uint32_t CountsSeen = 0;
   uint64_t CurrSum = 0, Count = 0;
 
   for (uint32_t Cutoff : DetailedSummaryCutoffs) {
     assert(Cutoff <= 999999);
     APInt Temp(128, TotalCount);
     APInt N(128, Cutoff);
     APInt D(128, ProfileSummary::Scale);
     Temp *= N;
     Temp = Temp.sdiv(D);
     uint64_t DesiredCount = Temp.getZExtValue();
     assert(DesiredCount <= TotalCount);
     while (CurrSum < DesiredCount && Iter != End) {
       Count = Iter->first;
       uint32_t Freq = Iter->second;
       CurrSum += (Count * Freq);
-      BlocksSeen += Freq;
+      CountsSeen += Freq;
       Iter++;
     }
     assert(CurrSum >= DesiredCount);
-    ProfileSummaryEntry PSE = {Cutoff, Count, BlocksSeen};
+    ProfileSummaryEntry PSE = {Cutoff, Count, CountsSeen};
     DetailedSummary.push_back(PSE);
   }
 }
 
-ProfileSummary::ProfileSummary(const IndexedInstrProf::Summary &S)
-    : TotalCount(S.get(IndexedInstrProf::Summary::TotalBlockCount)),
-      MaxBlockCount(S.get(IndexedInstrProf::Summary::MaxBlockCount)),
-      MaxInternalBlockCount(
-          S.get(IndexedInstrProf::Summary::MaxInternalBlockCount)),
+InstrProfSummary::InstrProfSummary(const IndexedInstrProf::Summary &S)
+    : ProfileSummary(), MaxInternalBlockCount(S.get(
+                            IndexedInstrProf::Summary::MaxInternalBlockCount)),
       MaxFunctionCount(S.get(IndexedInstrProf::Summary::MaxFunctionCount)),
-      NumBlocks(S.get(IndexedInstrProf::Summary::TotalNumBlocks)),
       NumFunctions(S.get(IndexedInstrProf::Summary::TotalNumFunctions)) {
+
+  TotalCount = S.get(IndexedInstrProf::Summary::TotalBlockCount);
+  MaxCount = S.get(IndexedInstrProf::Summary::MaxBlockCount);
+  NumCounts = S.get(IndexedInstrProf::Summary::TotalNumBlocks);
+
   for (unsigned I = 0; I < S.NumCutoffEntries; I++) {
     const IndexedInstrProf::Summary::Entry &Ent = S.getEntry(I);
     DetailedSummary.emplace_back((uint32_t)Ent.Cutoff, Ent.MinBlockCount,
                                  Ent.NumBlocks);
   }
 }
+void InstrProfSummary::addEntryCount(uint64_t Count) {
+  addCount(Count);
+  NumFunctions++;
+  if (Count > MaxFunctionCount)
+    MaxFunctionCount = Count;
+}
+
+void InstrProfSummary::addInternalCount(uint64_t Count) {
+  addCount(Count);
+  if (Count > MaxInternalBlockCount)
+    MaxInternalBlockCount = Count;
+}

tools/llvm-profdata/llvm-profdata.cpp

static int showInstrProfile(std::string Filename, bool ShowCounts,
                             std::vector<uint32_t> DetailedSummaryCutoffs,
                             bool ShowAllFunctions, std::string ShowFunction,
                             bool TextFormat, raw_fd_ostream &OS) {
   auto ReaderOrErr = InstrProfReader::create(Filename);
   std::vector<uint32_t> Cutoffs(DetailedSummaryCutoffs);
   if (ShowDetailedSummary && DetailedSummaryCutoffs.empty()) {
     Cutoffs = {800000, 900000, 950000, 990000, 999000, 999900, 999990};
   }
-  ProfileSummary PS(Cutoffs);
+  InstrProfSummary PS(Cutoffs);
   if (std::error_code EC = ReaderOrErr.getError())
     exitWithErrorCode(EC, Filename);
 
   auto Reader = std::move(ReaderOrErr.get());
   bool IsIRInstr = Reader->isIRLevelProfile();
   size_t ShownFunctions = 0;
   for (const auto &Func : *Reader) {
     bool Show =
static int showInstrProfile(std::string Filename, bool ShowCounts,
   OS << "Maximum function count: " << PS.getMaxFunctionCount() << "\n";
   OS << "Maximum internal block count: " << PS.getMaxInternalBlockCount() << "\n";
 
   if (ShowDetailedSummary) {
     OS << "Detailed summary:\n";
     OS << "Total number of blocks: " << PS.getNumBlocks() << "\n";
     OS << "Total count: " << PS.getTotalCount() << "\n";
     for (auto Entry : PS.getDetailedSummary()) {
-      OS << Entry.NumBlocks << " blocks with count >= " << Entry.MinBlockCount
+      OS << Entry.NumCounts << " blocks with count >= " << Entry.MinCount
          << " account for "
          << format("%0.6g", (float)Entry.Cutoff / ProfileSummary::Scale * 100)
          << " percentage of the total counts.\n";
     }
   }
   return 0;
 }
 

unittests/ProfileData/InstrProfTest.cpp

TEST_F(InstrProfTest, get_profile_summary) {
   InstrProfRecord Record4("func4", 0x1234, {0});
   Writer.addRecord(std::move(Record1));
   Writer.addRecord(std::move(Record2));
   Writer.addRecord(std::move(Record3));
   Writer.addRecord(std::move(Record4));
   auto Profile = Writer.writeBuffer();
   readProfile(std::move(Profile));
 
-  ProfileSummary &PS = Reader->getSummary();
+  InstrProfSummary &PS = Reader->getSummary();
   ASSERT_EQ(2305843009213693952U, PS.getMaxFunctionCount());
   ASSERT_EQ(2305843009213693952U, PS.getMaxBlockCount());
   ASSERT_EQ(10U, PS.getNumBlocks());
   ASSERT_EQ(4539628424389557499U, PS.getTotalCount());
   std::vector<ProfileSummaryEntry> &Details = PS.getDetailedSummary();
   uint32_t Cutoff = 800000;
   auto Predicate = [&Cutoff](const ProfileSummaryEntry &PE) {
     return PE.Cutoff == Cutoff;
   };
   auto EightyPerc = std::find_if(Details.begin(), Details.end(), Predicate);
   Cutoff = 900000;
   auto NinetyPerc = std::find_if(Details.begin(), Details.end(), Predicate);
   Cutoff = 950000;
   auto NinetyFivePerc = std::find_if(Details.begin(), Details.end(), Predicate);
   Cutoff = 990000;
   auto NinetyNinePerc = std::find_if(Details.begin(), Details.end(), Predicate);
-  ASSERT_EQ(576460752303423488U, EightyPerc->MinBlockCount);
-  ASSERT_EQ(288230376151711744U, NinetyPerc->MinBlockCount);
-  ASSERT_EQ(288230376151711744U, NinetyFivePerc->MinBlockCount);
-  ASSERT_EQ(72057594037927936U, NinetyNinePerc->MinBlockCount);
+  ASSERT_EQ(576460752303423488U, EightyPerc->MinCount);
+  ASSERT_EQ(288230376151711744U, NinetyPerc->MinCount);
+  ASSERT_EQ(288230376151711744U, NinetyFivePerc->MinCount);
+  ASSERT_EQ(72057594037927936U, NinetyNinePerc->MinCount);
 }
 
 TEST_P(MaybeSparseInstrProfTest, get_icall_data_read_write) {
   InstrProfRecord Record1("caller", 0x1234, {1, 2});
   InstrProfRecord Record2("callee1", 0x1235, {3, 4});
   InstrProfRecord Record3("callee2", 0x1235, {3, 4});
   InstrProfRecord Record4("callee3", 0x1235, {3, 4});