[CodeGen] Separate MachineFunctionSplitter logic for different profile types

[shenhan]

Summary

In D152577 @xur has a post-submit comment regarding to an awkward usage of MFS for Autofdo - instead of just using -fsplit-machine-function, the user needs to add "-mllvm -mfs-psi-cutoff=0" to choose the right logic for AutoFDO. The compiler should choose the right default values for such case.

This CL separate MFS logic for different profile types.

[wenlei]

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

112

Can we just use query PSI directly? PSI has the following API, which should be sufficient and we should be able to avoid passing ProfileKind around.

PSI->hasSampleProfile()
PSI->hasInstrumentationProfile()
PSI->hasCSInstrumentationProfile()

126

This is common code that can be sinked out of the switch.

[wenlei]

lgtm, thanks.

[xur]

looks good to me.

llvm/include/llvm/Analysis/ProfileSummaryInfo.h

public:
   }
 
   /// Returns true if module \c M has context sensitive instrumentation profile.
   bool hasCSInstrumentationProfile() const {
     return hasProfileSummary() &&
            Summary->getKind() == ProfileSummary::PSK_CSInstr;
   }
 
+  std::optional<ProfileSummary::Kind> getProfileKind() const {
+    return hasProfileSummary()
+               ? std::make_optional<ProfileSummary::Kind>(Summary->getKind())
+               : std::nullopt;
+  }
+
   /// Handle the invalidation of this information.
   ///
   /// When used as a result of \c ProfileSummaryAnalysis this method will be
   /// called when the module this was computed for changes. Since profile
   /// summary is immutable after it is annotated on the module, we return false
   /// here.
   bool invalidate(Module &, const PreservedAnalyses &,
                   ModuleAnalysisManager::Invalidator &) {

llvm/lib/CodeGen/MachineFunctionSplitter.cpp

auto Comparator = [](const MachineBasicBlock &X, const MachineBasicBlock &Y) {
     return X.getSectionID().Type < Y.getSectionID().Type;
   };
   llvm::sortBasicBlocksAndUpdateBranches(MF, Comparator);
   llvm::avoidZeroOffsetLandingPad(MF);
 }
 
 static bool isColdBlock(const MachineBasicBlock &MBB,
                         const MachineBlockFrequencyInfo *MBFI,
-                        ProfileSummaryInfo *PSI, bool HasAccurateProfile) {
+                        ProfileSummaryInfo *PSI,
+                        ProfileSummary::Kind ProfileKind) {
   std::optional<uint64_t> Count = MBFI->getBlockProfileCount(&MBB);
-  // If using accurate profile, no count means cold.
-  // If no accurate profile, no count means "do not judge
-  // coldness".
+  switch (ProfileKind) {
+  case ProfileSummary::PSK_Instr:
+  case ProfileSummary::PSK_CSInstr:
+    // If using instrument profile, which is deemed "accurate", no count means
+    // cold.
     if (!Count)
-    return HasAccurateProfile;
-
+      return true;
     if (PercentileCutoff > 0)
       return PSI->isColdCountNthPercentile(PercentileCutoff, *Count);
     return (*Count < ColdCountThreshold);
+  case llvm::ProfileSummary::PSK_Sample:
+    // For sample profile, no count means "do not judege coldness".
+    if (!Count)
+      return false;
+    return *Count < ColdCountThreshold;
+  }
+  return false;
 }
 
 bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
   // We target functions with profile data. Static information in the form
   // of exception handling code may be split to cold if user passes the
   // mfs-split-ehcode flag.
-  bool UseProfileData = MF.getFunction().hasProfileData();
+  ProfileSummaryInfo *PSI =
+      &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
+  std::optional<ProfileSummary::Kind> ProfileKind = PSI->getProfileKind();
+  bool UseProfileData = ProfileKind && MF.getFunction().hasProfileData();
   if (!UseProfileData && !SplitAllEHCode)
     return false;
 
   // TODO: We don't split functions where a section attribute has been set
   // since the split part may not be placed in a contiguous region. It may also
   // be more beneficial to augment the linker to ensure contiguous layout of
   // split functions within the same section as specified by the attribute.
   if (MF.getFunction().hasSection() ||
bool MachineFunctionSplitter::runOnMachineFunction(MachineFunction &MF) {
   // Renumbering blocks here preserves the order of the blocks as
   // sortBasicBlocksAndUpdateBranches uses the numeric identifier to sort
   // blocks. Preserving the order of blocks is essential to retaining decisions
   // made by prior passes such as MachineBlockPlacement.
   MF.RenumberBlocks();
   MF.setBBSectionsType(BasicBlockSection::Preset);
 
   MachineBlockFrequencyInfo *MBFI = nullptr;
-  ProfileSummaryInfo *PSI = nullptr;
-  // Whether this pass is using FSAFDO profile (not accurate) or IRPGO
-  // (accurate). HasAccurateProfile is only used when UseProfileData is true,
-  // but giving it a default value to silent any possible warning.
-  bool HasAccurateProfile = false;
   if (UseProfileData) {
     MBFI = &getAnalysis<MachineBlockFrequencyInfo>();
-    PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
-    // "HasAccurateProfile" is false for FSAFDO, true when using IRPGO
-    // (traditional instrumented FDO) or CSPGO profiles.
-    HasAccurateProfile =
-        PSI->hasInstrumentationProfile() || PSI->hasCSInstrumentationProfile();
-    // If HasAccurateProfile is false, we only trust hot functions,
-    // which have many samples, and consider them as split
-    // candidates. On the other hand, if HasAccurateProfile (likeIRPGO), we
-    // trust both cold and hot functions.
-    if (!HasAccurateProfile && !PSI->isFunctionHotInCallGraph(&MF, *MBFI)) {
+    // If we don't have a good profile (sample profile is not deemed
+    // as a "good profile") and the function is not hot, then early
+    // return. (Because we can only trust hot functions when profile
+    // quality is not good.)
+    if ((*ProfileKind == ProfileSummary::PSK_Sample &&
+         !PSI->isFunctionHotInCallGraph(&MF, *MBFI))) {
       // Split all EH code and it's descendant statically by default.
       if (SplitAllEHCode)
         setDescendantEHBlocksCold(MF);
       finishAdjustingBasicBlocksAndLandingPads(MF);
       return true;
     }
   }
 
   SmallVector<MachineBasicBlock *, 2> LandingPads;
   for (auto &MBB : MF) {
     if (MBB.isEntryBlock())
       continue;
 
     if (MBB.isEHPad())
       LandingPads.push_back(&MBB);
-    else if (UseProfileData &&
-             isColdBlock(MBB, MBFI, PSI, HasAccurateProfile) && !SplitAllEHCode)
+    else if (UseProfileData && isColdBlock(MBB, MBFI, PSI, *ProfileKind) &&
+             !SplitAllEHCode)
       MBB.setSectionID(MBBSectionID::ColdSectionID);
   }
 
   // Split all EH code and it's descendant statically by default.
   if (SplitAllEHCode)
     setDescendantEHBlocksCold(MF);
   // We only split out eh pads if all of them are cold.
   else {
     // Here we have UseProfileData == true.
     bool HasHotLandingPads = false;
     for (const MachineBasicBlock *LP : LandingPads) {
-      if (!isColdBlock(*LP, MBFI, PSI, HasAccurateProfile))
+      if (!isColdBlock(*LP, MBFI, PSI, *ProfileKind))
         HasHotLandingPads = true;
     }
     if (!HasHotLandingPads) {
       for (MachineBasicBlock *LP : LandingPads)
         LP->setSectionID(MBBSectionID::ColdSectionID);
     }
   }