Profile summary cleanup.

[eraman]

Summary

Move MaxFunctionCount and NumFunctions to the base class. This allows to get max function count from profile summary (which is needed to phase out MaxFunctionCount module flag) without knowing what format of profile is used. I have retained all the existing interfaces in SampleProfSummary (so, for instance, getMaxHeadSamples and getMaxFunctionCount would return the same thing).

[davidxl]

lgtm

[eraman]

My original change had a bug due to the fact that sample profile head samples are also counted in body samples (I couldn't run the tests yesterday since the head was broken). I had to undo some of the refactoring from my original patch, but the main stuff - moving max function count and num functions to the base class - still remains the same. I have tested this.

[davidxl]

Is there a way to exclude header sample from the body samples?

[eraman]

In D18468#383779, @davidxl wrote:

Is there a way to exclude header sample from the body samples?

That'll definitely make it consistent with instrumented profile and I'll let Diego chime in. However, that should be a separate patch.

[dnovillo]

In D18468#383779, @davidxl wrote:

Is there a way to exclude header sample from the body samples?

I'm surprised that body samples count header samples. That seems like a bug to me. Easwaran, making both instrumented and sample consistent is a good idea. Thanks.

[dnovillo]

LGTM

include/llvm/ProfileData/ProfileCommon.h

153 ↗

(On Diff #51686)

Perhaps we should deprecate getMaxHeadSamples? IIUC, MaxFunctionCount is the number of times a function was called, correct? That's essentially the same information provided by counting the number of samples in the function header.

There is a difference between the two, but I don't think the distinction is worth keeping. Dehao, what do you think?

[danielcdh]

include/llvm/ProfileData/ProfileCommon.h

153 ↗

(On Diff #51686)

Agree, MaxHeadSamples should be the same as MaxFunctionCount.

[eraman]

include/llvm/ProfileData/ProfileCommon.h

153 ↗

(On Diff #51686)

I thought it is useful in terms of readability to have interfaces named in SampleProfile specific terms. Both MaxHeadSamples and MaxFunctionCount are indeed same and hence getMaxHeadSamples returns the MaxFunctionCount. If this is causing confusion, I'll deprecate this in a separate change.

llvm/trunk/include/llvm/ProfileData/ProfileCommon.h

private:
   static const char *KindStr[2];
   // 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:
   SummaryEntryVector DetailedSummary;
   std::vector<uint32_t> DetailedSummaryCutoffs;
-  uint64_t TotalCount, MaxCount;
-  uint32_t NumCounts;
+  uint64_t TotalCount, MaxCount, MaxFunctionCount;
+  uint32_t NumCounts, NumFunctions;
   ProfileSummary(Kind K, std::vector<uint32_t> Cutoffs)
       : PSK(K), DetailedSummaryCutoffs(Cutoffs), TotalCount(0), MaxCount(0),
-        NumCounts(0) {}
-  ProfileSummary(Kind K) : PSK(K), TotalCount(0), MaxCount(0), NumCounts(0) {}
+        MaxFunctionCount(0), NumCounts(0), NumFunctions(0) {}
+  ProfileSummary(Kind K)
+      : PSK(K), TotalCount(0), MaxCount(0), MaxFunctionCount(0), NumCounts(0),
+        NumFunctions(0) {}
   ProfileSummary(Kind K, SummaryEntryVector DetailedSummary,
-                 uint64_t TotalCount, uint64_t MaxCount, uint32_t NumCounts)
+                 uint64_t TotalCount, uint64_t MaxCount,
+                 uint64_t MaxFunctionCount, uint32_t NumCounts,
+                 uint32_t NumFunctions)
       : PSK(K), DetailedSummary(DetailedSummary), TotalCount(TotalCount),
-        MaxCount(MaxCount), NumCounts(NumCounts) {}
+        MaxCount(MaxCount), MaxFunctionCount(MaxFunctionCount),
+        NumCounts(NumCounts), NumFunctions(NumFunctions) {}
   ~ProfileSummary() = default;
   inline void addCount(uint64_t Count);
   /// \brief Return metadata specific to the profile format.
   /// Derived classes implement this method to return a vector of Metadata.
   virtual std::vector<Metadata *> getFormatSpecificMD(LLVMContext &Context) = 0;
   /// \brief Return detailed summary as metadata.
   Metadata *getDetailedSummaryMD(LLVMContext &Context);
 
 public:
   static const int Scale = 1000000;
   Kind getKind() const { return PSK; }
   const char *getKindStr() const { return KindStr[PSK]; }
   // \brief Returns true if F is a hot function.
   static bool isFunctionHot(const Function *F);
   // \brief Returns true if F is unlikley executed.
   static bool isFunctionUnlikely(const Function *F);
   inline SummaryEntryVector &getDetailedSummary();
   void computeDetailedSummary();
   /// \brief A vector of useful cutoff values for detailed summary.
   static const std::vector<uint32_t> DefaultCutoffs;
   /// \brief Return summary information as metadata.
   Metadata *getMD(LLVMContext &Context);
   /// \brief Construct profile summary from metdata.
   static ProfileSummary *getFromMD(Metadata *MD);
+  uint32_t getNumFunctions() { return NumFunctions; }
+  uint64_t getMaxFunctionCount() { return MaxFunctionCount; }
 };
 
 class InstrProfSummary final : public ProfileSummary {
-  uint64_t MaxInternalBlockCount, MaxFunctionCount;
-  uint32_t NumFunctions;
+  uint64_t MaxInternalBlockCount;
   inline void addEntryCount(uint64_t Count);
   inline void addInternalCount(uint64_t Count);
 
 protected:
   std::vector<Metadata *> getFormatSpecificMD(LLVMContext &Context) override;
 
 public:
   InstrProfSummary(std::vector<uint32_t> Cutoffs)
-      : ProfileSummary(PSK_Instr, Cutoffs), MaxInternalBlockCount(0),
-        MaxFunctionCount(0), NumFunctions(0) {}
+      : ProfileSummary(PSK_Instr, Cutoffs), MaxInternalBlockCount(0) {}
   InstrProfSummary(const IndexedInstrProf::Summary &S);
   InstrProfSummary(uint64_t TotalCount, uint64_t MaxBlockCount,
                    uint64_t MaxInternalBlockCount, uint64_t MaxFunctionCount,
                    uint32_t NumBlocks, uint32_t NumFunctions,
                    SummaryEntryVector Summary)
       : ProfileSummary(PSK_Instr, Summary, TotalCount, MaxBlockCount,
-                       NumBlocks),
-        MaxInternalBlockCount(MaxInternalBlockCount),
-        MaxFunctionCount(MaxFunctionCount), NumFunctions(NumFunctions) {}
+                       MaxFunctionCount, NumBlocks, NumFunctions),
+        MaxInternalBlockCount(MaxInternalBlockCount) {}
   static bool classof(const ProfileSummary *PS) {
     return PS->getKind() == PSK_Instr;
   }
   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 MaxCount; }
   uint64_t getMaxInternalBlockCount() { return MaxInternalBlockCount; }
 };
 
 class SampleProfileSummary final : public ProfileSummary {
-  uint64_t MaxHeadSamples;
-  uint32_t NumFunctions;
-
 protected:
   std::vector<Metadata *> getFormatSpecificMD(LLVMContext &Context) override;
 
 public:
   uint32_t getNumLinesWithSamples() { return NumCounts; }
   uint64_t getTotalSamples() { return TotalCount; }
-  uint32_t getNumFunctions() { return NumFunctions; }
-  uint64_t getMaxHeadSamples() { return MaxHeadSamples; }
+  uint64_t getMaxHeadSamples() { return MaxFunctionCount; }
   uint64_t getMaxSamplesPerLine() { return MaxCount; }
   void addRecord(const sampleprof::FunctionSamples &FS);
   SampleProfileSummary(std::vector<uint32_t> Cutoffs)
-      : ProfileSummary(PSK_Sample, Cutoffs), MaxHeadSamples(0),
-        NumFunctions(0) {}
+      : ProfileSummary(PSK_Sample, Cutoffs) {}
   SampleProfileSummary(uint64_t TotalSamples, uint64_t MaxSamplesPerLine,
                        uint64_t MaxHeadSamples, int32_t NumLinesWithSamples,
                        uint32_t NumFunctions,
                        SummaryEntryVector DetailedSummary)
       : ProfileSummary(PSK_Sample, DetailedSummary, TotalSamples,
-                       MaxSamplesPerLine, NumLinesWithSamples),
-        MaxHeadSamples(MaxHeadSamples), NumFunctions(NumFunctions) {}
+                       MaxSamplesPerLine, MaxHeadSamples, NumLinesWithSamples,
+                       NumFunctions) {}
   static bool classof(const ProfileSummary *PS) {
     return PS->getKind() == PSK_Sample;
   }
 };
 
 // This is called when a count is seen in the profile.
 void ProfileSummary::addCount(uint64_t Count) {
   TotalCount += Count;

llvm/trunk/lib/ProfileData/ProfileSummary.cpp

void InstrProfSummary::addRecord(const InstrProfRecord &R) {
   for (size_t I = 1, E = R.Counts.size(); I < E; ++I)
     addInternalCount(R.Counts[I]);
 }
 
 // To compute the detailed summary, we consider each line containing samples as
 // equivalent to a block with a count in the instrumented profile.
 void SampleProfileSummary::addRecord(const sampleprof::FunctionSamples &FS) {
   NumFunctions++;
-  if (FS.getHeadSamples() > MaxHeadSamples)
-    MaxHeadSamples = FS.getHeadSamples();
+  if (FS.getHeadSamples() > MaxFunctionCount)
+    MaxFunctionCount = FS.getHeadSamples();
   for (const auto &I : FS.getBodySamples())
     addCount(I.second.getSamples());
 }
 
 // The argument to this method is a vector of cutoff percentages and the return
 // value is a vector of (Cutoff, MinCount, NumCounts) triplets.
 void ProfileSummary::computeDetailedSummary() {
   if (DetailedSummaryCutoffs.empty())
bool ProfileSummary::isFunctionUnlikely(const Function *F) {
   }
   // FIXME: update when summary data is stored in module's metadata.
   return (*F->getEntryCount()) == 0;
 }
 
 InstrProfSummary::InstrProfSummary(const IndexedInstrProf::Summary &S)
     : ProfileSummary(PSK_Instr),
       MaxInternalBlockCount(
-          S.get(IndexedInstrProf::Summary::MaxInternalBlockCount)),
-      MaxFunctionCount(S.get(IndexedInstrProf::Summary::MaxFunctionCount)),
-      NumFunctions(S.get(IndexedInstrProf::Summary::TotalNumFunctions)) {
+          S.get(IndexedInstrProf::Summary::MaxInternalBlockCount)) {
 
   TotalCount = S.get(IndexedInstrProf::Summary::TotalBlockCount);
   MaxCount = S.get(IndexedInstrProf::Summary::MaxBlockCount);
+  MaxFunctionCount = S.get(IndexedInstrProf::Summary::MaxFunctionCount);
   NumCounts = S.get(IndexedInstrProf::Summary::TotalNumBlocks);
+  NumFunctions = S.get(IndexedInstrProf::Summary::TotalNumFunctions);
 
   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) {