[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

127

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()

141

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

[wenlei]

lgtm, thanks.

[xur]

looks good to me.

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) {
   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".
+  // For instrumentation profiles and sample profiles, we use different ways
+  // to judge whether a block is cold and should be split.
+  if (PSI->hasInstrumentationProfile() || PSI->hasCSInstrumentationProfile()) {
+    // 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);
+    // Fallthrough to end of function.
+  } else if (PSI->hasSampleProfile()) {
+    // For sample profile, no count means "do not judege coldness".
+    if (!Count)
+      return false;
+  }
+
   return (*Count < ColdCountThreshold);
 }
 
 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();
   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() ||
       MF.getFunction().hasFnAttribute("implicit-section-name"))
     return false;
 
   // We don't want to proceed further for cold functions
   // or functions of unknown hotness. Lukewarm functions have no prefix.
   std::optional<StringRef> SectionPrefix = MF.getFunction().getSectionPrefix();
   if (SectionPrefix &&
       (*SectionPrefix == "unlikely" || *SectionPrefix == "unknown")) {
     return false;
   }
 
   // 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 (PSI->hasSampleProfile() && !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) && !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))
         HasHotLandingPads = true;
     }
     if (!HasHotLandingPads) {
       for (MachineBasicBlock *LP : LandingPads)
         LP->setSectionID(MBBSectionID::ColdSectionID);
     }
   }