Detect the order and add a summary flag to the profile

Author: wlei-llvm

llvm/include/llvm/ProfileData/SampleProf.h

@@ -201,6 +201,10 @@ enum class SecProfSummaryFlags : uint32_t {
   /// SecFlagIsPreInlined means this profile contains ShouldBeInlined
   /// contexts thus this is CS preinliner computed.
   SecFlagIsPreInlined = (1 << 4),
+  /// SecFlagIsMixedProbeOrder means in a pseude-probe based profile, the
+  /// callsite and BB probe IDs are mixed and sorted in lexcial order instead of
+  /// the order that callsite probe IDs are always after the BB probe IDs.
+  SecFlagIsMixedProbeOrder = (1 << 5),
 };
 
 enum class SecFuncMetadataFlags : uint32_t {
@@ -466,7 +470,7 @@ struct SampleContextFrame {
   LineLocation Location;
 
   SampleContextFrame() : Location(0, 0) {}
-  
+
   SampleContextFrame(FunctionId Func, LineLocation Location)
       : Func(Func), Location(Location) {}
 
@@ -527,7 +531,7 @@ class SampleContext {
       : Func(Name), State(UnknownContext), Attributes(ContextNone) {
         assert(!Name.empty() && "Name is empty");
       }
-  
+
   SampleContext(FunctionId Func)
       : Func(Func), State(UnknownContext), Attributes(ContextNone) {}
 
@@ -1178,6 +1182,8 @@ class FunctionSamples {
 
   static bool ProfileIsProbeBased;
 
+  static bool ProfileIsMixedProbeOrder;
+
   static bool ProfileIsCS;
 
   static bool ProfileIsPreInlined;

llvm/include/llvm/ProfileData/SampleProfReader.h

@@ -525,6 +525,9 @@ class SampleProfileReader {
   /// \brief Whether samples are collected based on pseudo probes.
   bool ProfileIsProbeBased = false;
 
+  /// Whether profiles are in mixed BB and callsite probe order.
+  bool ProfileIsMixedProbeOrder = false;
+
   /// Whether function profiles are context-sensitive flat profiles.
   bool ProfileIsCS = false;
 

llvm/lib/ProfileData/SampleProf.cpp

@@ -41,6 +41,7 @@ static cl::opt<bool> GenerateMergedBaseProfiles(
 namespace llvm {
 namespace sampleprof {
 bool FunctionSamples::ProfileIsProbeBased = false;
+bool FunctionSamples::ProfileIsMixedProbeOrder = false;
 bool FunctionSamples::ProfileIsCS = false;
 bool FunctionSamples::ProfileIsPreInlined = false;
 bool FunctionSamples::UseMD5 = false;

llvm/lib/ProfileData/SampleProfReader.cpp

@@ -704,6 +704,9 @@ std::error_code SampleProfileReaderExtBinaryBase::readOneSection(
       FunctionSamples::ProfileIsPreInlined = ProfileIsPreInlined = true;
     if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagFSDiscriminator))
       FunctionSamples::ProfileIsFS = ProfileIsFS = true;
+    if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagIsMixedProbeOrder))
+      FunctionSamples::ProfileIsMixedProbeOrder = ProfileIsMixedProbeOrder =
+          true;
     break;
   case SecNameTable: {
     bool FixedLengthMD5 =
@@ -1369,6 +1372,8 @@ static std::string getSecFlagsStr(const SecHdrTableEntry &Entry) {
       Flags.append("preInlined,");
     if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagFSDiscriminator))
       Flags.append("fs-discriminator,");
+    if (hasSecFlag(Entry, SecProfSummaryFlags::SecFlagIsMixedProbeOrder))
+      Flags.append("mixed-probe-order,");
     break;
   case SecFuncOffsetTable:
     if (hasSecFlag(Entry, SecFuncOffsetFlags::SecFlagOrdered))

llvm/lib/ProfileData/SampleProfWriter.cpp

@@ -437,6 +437,10 @@ std::error_code SampleProfileWriterExtBinaryBase::writeOneSection(
     addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagIsPreInlined);
   if (Type == SecProfSummary && FunctionSamples::ProfileIsFS)
     addSectionFlag(SecProfSummary, SecProfSummaryFlags::SecFlagFSDiscriminator);
+  if (Type == SecProfSummary && FunctionSamples::ProfileIsProbeBased &&
+      FunctionSamples::ProfileIsMixedProbeOrder)
+    addSectionFlag(SecProfSummary,
+                   SecProfSummaryFlags::SecFlagIsMixedProbeOrder);
 
   uint64_t SectionStart = markSectionStart(Type, LayoutIdx);
   switch (Type) {

llvm/lib/Transforms/IPO/SampleProfile.cpp

@@ -1129,7 +1129,7 @@ void SampleProfileLoader::findExternalInlineCandidate(
         CalleeSample->getContext().hasAttribute(ContextShouldBeInlined);
     if (!PreInline && CalleeSample->getHeadSamplesEstimate() < Threshold)
       continue;
-    
+
     Function *Func = SymbolMap.lookup(CalleeSample->getFunction());
     // Add to the import list only when it's defined out of module.
     if (!Func || Func->isDeclaration())
@@ -2112,6 +2112,14 @@ bool SampleProfileLoader::doInitialization(Module &M,
                                                DS_Warning));
       return false;
     }
+
+    if (!FunctionSamples::ProfileIsMixedProbeOrder) {
+      const char *Msg =
+          "Pseudo-probe-based profile is on an old version ID order which "
+          "could cause profile mismatch(performance regression)";
+      Ctx.diagnose(DiagnosticInfoSampleProfile(M.getModuleIdentifier(), Msg,
+                                               DS_Warning));
+    }
   }
 
   if (ReportProfileStaleness || PersistProfileStaleness ||

llvm/tools/llvm-profgen/ProfileGenerator.cpp

@@ -471,6 +471,66 @@ ProfileGenerator::getTopLevelFunctionProfile(FunctionId FuncName) {
   return ProfileMap.Create(Context);
 }
 
+// Use a heuristic to determine probe order by checking callsite insertion
+// position relative to the BB probes. Sort all the BB probes is in a list, for
+// each calliste, compute "ratio = insert position / length of the list". For
+// the old order, the probe ids for BB should be all before(smaller than) the
+// probe ids for callsite, this ratio should be equal to or close to 1.
+bool checkProbeIDIsInMixedOrder(const SampleProfileMap &Profiles) {
+  // Use flattned profile to maximize the callsites in the profile.
+  SampleProfileMap flattenProfile;
+  ProfileConverter::flattenProfile(Profiles, flattenProfile);
+
+  uint32_t PossibleOldOrderNum = 0;
+  uint32_t PossibleNewOrderNum = 0;
+
+  for (const auto &I : flattenProfile) {
+    const FunctionSamples &FS = I.second;
+    // Skip small functions whose profile order are likely random.
+    if (FS.getBodySamples().size() < 10)
+      continue;
+
+    std::set<uint32_t> PossibleBBProbeIDs;
+    std::set<uint32_t> CallsiteIDs;
+    for (const auto &I : FS.getBodySamples()) {
+      if (I.second.getCallTargets().empty())
+        PossibleBBProbeIDs.insert(I.first.LineOffset);
+      else
+        CallsiteIDs.insert(I.first.LineOffset);
+    }
+
+    if (PossibleBBProbeIDs.empty() || CallsiteIDs.empty())
+      continue;
+
+    uint32_t DistanceToBeginSum = 0;
+    for (const auto &ID : CallsiteIDs)
+      DistanceToBeginSum += std::distance(PossibleBBProbeIDs.begin(),
+                                          PossibleBBProbeIDs.upper_bound(ID));
+    uint32_t LengthSum = PossibleBBProbeIDs.size() * CallsiteIDs.size();
+
+    // Note that PossibleBBProbeIDs could contains some callsite probe id, the
+    // ratio is not exactly 1 for the old order, hence use a smaller threshold
+    // to determine.
+    if ((float)DistanceToBeginSum / LengthSum > 0.8)
+      PossibleOldOrderNum++;
+    else
+      PossibleNewOrderNum++;
+  }
+  return PossibleNewOrderNum >= PossibleOldOrderNum;
+}
+
+void warnOrWriteProbeOrderFlag(const SampleProfileMap &Profiles) {
+  if (FunctionSamples::ProfileIsProbeBased) {
+    FunctionSamples::ProfileIsMixedProbeOrder = true;
+    if (!checkProbeIDIsInMixedOrder(Profiles)) {
+      WithColor::warning()
+          << "Pseudo-probe-based profile is on an old version ID order which "
+             "could cause profile mismatch(performance regression)\n";
+      FunctionSamples::ProfileIsMixedProbeOrder = false;
+    }
+  }
+}
+
 void ProfileGenerator::generateProfile() {
   collectProfiledFunctions();
 
@@ -492,6 +552,7 @@ void ProfileGenerator::postProcessProfiles() {
   computeSummaryAndThreshold(ProfileMap);
   trimColdProfiles(ProfileMap, ColdCountThreshold);
   calculateAndShowDensity(ProfileMap);
+  warnOrWriteProbeOrderFlag(ProfileMap);
 }
 
 void ProfileGenerator::trimColdProfiles(const SampleProfileMap &Profiles,
@@ -1024,6 +1085,7 @@ void CSProfileGenerator::postProcessProfiles() {
     CSConverter.convertCSProfiles();
     FunctionSamples::ProfileIsCS = false;
   }
+  warnOrWriteProbeOrderFlag(ProfileMap);
 }
 
 void ProfileGeneratorBase::computeSummaryAndThreshold(