[Propeller] Promote functions with propeller profiles to .text.hot.

[rahmanl]

Summary

Today, text section prefixes (none, .unlikely, .hot, and .unkown) are determined based on PGO profile. However, Propeller may deem a function hot when PGO doesn't. Besides, when -Wl,-keep-text-section-prefix=true Propeller cannot enforce a global section ordering as the linker can only reorder sections within each output section (.text, .text.hot, .text.unlikely).

This patch promotes all functions with Propeller profiles (functions listed in the basic-block-sections profile) to .text.hot. The feature is hidden behind the flag --bbsections-guided-section-prefix which defaults to true.

The new implementation refactors the parsing of basic block sections profile into a new BasicBlockSectionsProfileReader analysis pass. This allows us to use the information earlier in CodeGenPrepare in order to set the functions text prefix. BasicBlockSectionsProfileReader will be used both by BasicBlockSections pass and CodeGenPrepare.

[rahmanl]

Refactor code.

[rahmanl]

WIP.

[rahmanl]

Fix comment.

[rahmanl]

Cleanup.

[rahmanl]

Remove unnecessary includes.

[shenhan]

This looks good to me. One question, do we have a rough idea, what is the percentage of functions that pgo and propeller disagree about their hotness?

llvm/lib/CodeGen/CodeGenPrepare.cpp

484–485

Are we sure BBSectionsProfileReader here is always not null?

[rahmanl]

Add null checking.

[rahmanl]

In D122930#3492643, @shenhan wrote:

This looks good to me. One question, do we have a rough idea, what is the percentage of functions that pgo and propeller disagree about their hotness?

Yes. For one example we have
31% of Propeller functions in .text.hot,
57% in .text
11% in .text.unlikely

One questions is whether the promotion should be hidden behind a flag or not.

llvm/lib/CodeGen/CodeGenPrepare.cpp

484–485

Thanks. Added the check.

[shenhan]

In D122930#3494676, @rahmanl wrote:

In D122930#3492643, @shenhan wrote:

This looks good to me. One question, do we have a rough idea, what is the percentage of functions that pgo and propeller disagree about their hotness?

Yes. For one example we have
31% of Propeller functions in .text.hot,
57% in .text
11% in .text.unlikely

Thanks.

One questions is whether the promotion should be hidden behind a flag or not.

My thought is that this is a prerequisite for propeller inter-proc layout optimization, so if inter-proc layout optimization is on by default when -fbasic-block-sections=list=<...> is present, then this could also be on by default. On the other hand if we guard inter-proc layout optimization with a flag, then that flag could also cover this.

[rahmanl]

• Hide the feature behind the flag --bbsections-guided-section-prefix.

[rahmanl]

In D122930#3509769, @shenhan wrote:

In D122930#3494676, @rahmanl wrote:

In D122930#3492643, @shenhan wrote:

This looks good to me. One question, do we have a rough idea, what is the percentage of functions that pgo and propeller disagree about their hotness?

Yes. For one example we have
31% of Propeller functions in .text.hot,
57% in .text
11% in .text.unlikely

Thanks.

One questions is whether the promotion should be hidden behind a flag or not.

My thought is that this is a prerequisite for propeller inter-proc layout optimization, so if inter-proc layout optimization is on by default when -fbasic-block-sections=list=<...> is present, then this could also be on by default. On the other hand if we guard inter-proc layout optimization with a flag, then that flag could also cover this.

Thanks. To clarify, this is also needed for function ordering. I went ahead and introduced a flag for this. The feature would also work when --profile-guided-section-prefix=false.

[MaskRay]

llvm/include/llvm/Analysis/BasicBlockSectionsProfileReader.h

30 ↗

(On Diff #429343)

Delete these using llvm::*;

38 ↗

(On Diff #429343)

48 ↗

(On Diff #429343)

Check whether you want to use SmallVector<BBClusterInfo, 0> to make the element size smaller.

llvm/lib/Analysis/BasicBlockSectionsProfileReader.cpp

1 ↗

(On Diff #429343)

Not sure the Reader should be placed in lib/Analysis. There is no precedent.

26 ↗

(On Diff #429343)

delete

[rahmanl]

• Move Reader pass to CodeGen.

[rahmanl]

llvm/include/llvm/Analysis/BasicBlockSectionsProfileReader.h

48 ↗

(On Diff #429343)

How about SmallVector<BBClusterInfo> for the absence of a strongly-motivated choice as advised in https://llvm.org/doxygen/classllvm_1_1SmallVector.html ?

llvm/lib/Analysis/BasicBlockSectionsProfileReader.cpp

1 ↗

(On Diff #429343)

Moved to CodeGen (where we have MIRSampleProfile.cpp).

[MaskRay]

llvm/include/llvm/Analysis/BasicBlockSectionsProfileReader.h

48 ↗

(On Diff #429343)

That's also fine.

[shenhan]

LGTM

llvm/lib/CodeGen/CodeGenPrepare.cpp

192

Maybe add more details in help message - "Other functions won't be impacted, i.e., their prefixes are decided by FDO/AutoFDO profile."

[rahmanl]

Expand flag description.

[rahmanl]

Added missing pipeline tests.

llvm/include/llvm/Analysis/BasicBlockSectionsInfo.h

+//===-- BasicBlockSectionsInfo.h - Basic block sections analysis pass ----====//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This pass creates the basic block cluster info by reading the basic block
+// sections profile. The cluster info will be used by the basic-block-sections
+// pass to arrange basic blocks in their sections.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_ANALYSIS_BASICBLOCKSECTIONSINFO_H
+#define LLVM_ANALYSIS_BASICBLOCKSECTIONSINFO_H
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/InitializePasses.h"
+#include "llvm/Pass.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+
+using llvm::SmallSet;
+using llvm::SmallVector;
+using llvm::StringMap;
+using llvm::StringRef;
+using namespace llvm;
+
+namespace llvm {
+
+// This struct represents the cluster information for a machine basic block.
+struct BBClusterInfo {
+  // MachineBasicBlock ID.
+  unsigned MBBNumber;
+  // Cluster ID this basic block belongs to.
+  unsigned ClusterID;
+  // Position of basic block within the cluster.
+  unsigned PositionInCluster;
+};
+
+using ProgramBBClusterInfoMapTy = StringMap<SmallVector<BBClusterInfo, 4>>;
+
+class BasicBlockSectionsInfo : public ImmutablePass {
+public:
+  static char ID;
+
+  BasicBlockSectionsInfo(const MemoryBuffer *Buf)
+      : ImmutablePass(ID), MBuf(Buf) {
+    initializeBasicBlockSectionsInfoPass(*PassRegistry::getPassRegistry());
+  };
+
+  BasicBlockSectionsInfo() : ImmutablePass(ID) {
+    initializeBasicBlockSectionsInfoPass(*PassRegistry::getPassRegistry());
+  }
+
+  StringRef getPassName() const override {
+    return "Basic Block Sections Info Analysis";
+  }
+
+  std::pair<bool, SmallVector<BBClusterInfo, 4>>
+  getBBClusterInfoForFunction(StringRef FuncName) const;
+
+  /// Read profiles of basic blocks if available here.
+  void initializePass() override;
+
+private:
+  StringRef getAliasName(StringRef FuncName) const {
+    auto R = FuncAliasMap.find(FuncName);
+    return R == FuncAliasMap.end() ? FuncName : R->second;
+  }
+
+  // This contains the basic-block-sections profile.
+  const MemoryBuffer *MBuf = nullptr;
+
+  // This encapsulates the BB cluster information for the whole program.
+  //
+  // For every function name, it contains the cluster information for (all or
+  // some of) its basic blocks. The cluster information for every basic block
+  // includes its cluster ID along with the position of the basic block in that
+  // cluster.
+  ProgramBBClusterInfoMapTy ProgramBBClusterInfo;
+
+  // Some functions have alias names. We use this map to find the main alias
+  // name for which we have mapping in ProgramBBClusterInfo.
+  StringMap<StringRef> FuncAliasMap;
+};
+
+ImmutablePass *createBasicBlockSectionsInfoPass(const MemoryBuffer *Buf);
+
+} // namespace llvm
+#endif // LLVM_ANALYSIS_BASICBLOCKSECTIONSINFO_H

llvm/include/llvm/Analysis/Passes.h

 //===-- llvm/Analysis/Passes.h - Constructors for analyses ------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This header file defines prototypes for accessor functions that expose passes
 // in the analysis libraries.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ANALYSIS_PASSES_H
 #define LLVM_ANALYSIS_PASSES_H
+#include "llvm/Support/MemoryBuffer.h"
 
 namespace llvm {
   class FunctionPass;
   class ImmutablePass;
   class ModulePass;
 
   //===--------------------------------------------------------------------===//
   //

llvm/include/llvm/CodeGen/Passes.h

namespace llvm {
   /// last LLVM modifying pass to clean up blocks that are not reachable from
   /// the entry block.
   FunctionPass *createUnreachableBlockEliminationPass();
 
   /// createBasicBlockSections Pass - This pass assigns sections to machine
   /// basic blocks and is enabled with -fbasic-block-sections. Buf is a memory
   /// buffer that contains the list of functions and basic block ids to
   /// selectively enable basic block sections.
-  MachineFunctionPass *createBasicBlockSectionsPass(const MemoryBuffer *Buf);
+  MachineFunctionPass *createBasicBlockSectionsPass();
 
   /// createMachineFunctionSplitterPass - This pass splits machine functions
   /// using profile information.
   MachineFunctionPass *createMachineFunctionSplitterPass();
 
   /// MachineFunctionPrinter pass - This pass prints out the machine function to
   /// the given stream as a debugging tool.
   MachineFunctionPass *

llvm/include/llvm/InitializePasses.h

 void initializeAnnotation2MetadataLegacyPass(PassRegistry &);
 void initializeAnnotationRemarksLegacyPass(PassRegistry &);
 void initializeOpenMPOptCGSCCLegacyPassPass(PassRegistry &);
 void initializeArgPromotionPass(PassRegistry&);
 void initializeAssumptionCacheTrackerPass(PassRegistry&);
 void initializeAtomicExpandPass(PassRegistry&);
 void initializeAttributorLegacyPassPass(PassRegistry&);
 void initializeAttributorCGSCCLegacyPassPass(PassRegistry &);
+void initializeBasicBlockSectionsInfoPass(PassRegistry &);
 void initializeBasicBlockSectionsPass(PassRegistry &);
 void initializeBDCELegacyPassPass(PassRegistry&);
 void initializeBarrierNoopPass(PassRegistry&);
 void initializeBasicAAWrapperPassPass(PassRegistry&);
 void initializeBlockExtractorLegacyPassPass(PassRegistry &);
 void initializeBlockFrequencyInfoWrapperPassPass(PassRegistry&);
 void initializeBoundsCheckingLegacyPassPass(PassRegistry&);
 void initializeBranchFolderPassPass(PassRegistry&);

llvm/lib/Analysis/BasicBlockSectionsInfo.cpp

+//===-- BasicBlockSectionsInfo.cpp ---=========----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Implementation of the basic block sections info analysis pass. It parses and
+// stores the basic block sections profile file (which is specified via the
+// `-basic-block-sections` flag).
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Analysis/BasicBlockSectionsInfo.h"
+#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/LineIterator.h"
+#include "llvm/Support/MemoryBuffer.h"
+
+using llvm::SmallSet;
+using llvm::SmallVector;
+using llvm::StringMap;
+using llvm::StringRef;
+using namespace llvm;
+
+char BasicBlockSectionsInfo::ID = 0;
+INITIALIZE_PASS(BasicBlockSectionsInfo, "bbsections-info-prepare",
+                "Prepares for basic block sections, by splitting functions "
+                "into clusters of basic blocks.",
+                false, false)
+
+std::pair<bool, SmallVector<BBClusterInfo, 4>>
+BasicBlockSectionsInfo::getBBClusterInfoForFunction(StringRef FuncName) const {
+  std::pair<bool, SmallVector<BBClusterInfo, 4>> cluster_info(false, {});
+  auto R = ProgramBBClusterInfo.find(getAliasName(FuncName));
+  if (R != ProgramBBClusterInfo.end()) {
+    cluster_info.second = R->second;
+    cluster_info.first = true;
+  }
+  return cluster_info;
+}
+
+// Basic Block Sections can be enabled for a subset of machine basic blocks.
+// This is done by passing a file containing names of functions for which basic
+// block sections are desired.  Additionally, machine basic block ids of the
+// functions can also be specified for a finer granularity. Moreover, a cluster
+// of basic blocks could be assigned to the same section.
+// A file with basic block sections for all of function main and three blocks
+// for function foo (of which 1 and 2 are placed in a cluster) looks like this:
+// ----------------------------
+// list.txt:
+// !main
+// !foo
+// !!1 2
+// !!4
+static Error getBBClusterInfo(const MemoryBuffer *MBuf,
+                              ProgramBBClusterInfoMapTy &ProgramBBClusterInfo,
+                              StringMap<StringRef> &FuncAliasMap) {
+  assert(MBuf);
+  line_iterator LineIt(*MBuf, /*SkipBlanks=*/true, /*CommentMarker=*/'#');
+
+  auto invalidProfileError = [&](auto Message) {
+    return make_error<StringError>(
+        Twine("Invalid profile " + MBuf->getBufferIdentifier() + " at line " +
+              Twine(LineIt.line_number()) + ": " + Message),
+        inconvertibleErrorCode());
+  };
+
+  auto FI = ProgramBBClusterInfo.end();
+
+  // Current cluster ID corresponding to this function.
+  unsigned CurrentCluster = 0;
+  // Current position in the current cluster.
+  unsigned CurrentPosition = 0;
+
+  // Temporary set to ensure every basic block ID appears once in the clusters
+  // of a function.
+  SmallSet<unsigned, 4> FuncBBIDs;
+
+  for (; !LineIt.is_at_eof(); ++LineIt) {
+    StringRef S(*LineIt);
+    if (S[0] == '@')
+      continue;
+    // Check for the leading "!"
+    if (!S.consume_front("!") || S.empty())
+      break;
+    // Check for second "!" which indicates a cluster of basic blocks.
+    if (S.consume_front("!")) {
+      if (FI == ProgramBBClusterInfo.end())
+        return invalidProfileError(
+            "Cluster list does not follow a function name specifier.");
+      SmallVector<StringRef, 4> BBIndexes;
+      S.split(BBIndexes, ' ');
+      // Reset current cluster position.
+      CurrentPosition = 0;
+      for (auto BBIndexStr : BBIndexes) {
+        unsigned long long BBIndex;
+        if (getAsUnsignedInteger(BBIndexStr, 10, BBIndex))
+          return invalidProfileError(Twine("Unsigned integer expected: '") +
+                                     BBIndexStr + "'.");
+        if (!FuncBBIDs.insert(BBIndex).second)
+          return invalidProfileError(Twine("Duplicate basic block id found '") +
+                                     BBIndexStr + "'.");
+        if (!BBIndex && CurrentPosition)
+          return invalidProfileError("Entry BB (0) does not begin a cluster.");
+
+        FI->second.emplace_back(BBClusterInfo{
+            ((unsigned)BBIndex), CurrentCluster, CurrentPosition++});
+      }
+      CurrentCluster++;
+    } else { // This is a function name specifier.
+      // Function aliases are separated using '/'. We use the first function
+      // name for the cluster info mapping and delegate all other aliases to
+      // this one.
+      SmallVector<StringRef, 4> Aliases;
+      S.split(Aliases, '/');
+      for (size_t i = 1; i < Aliases.size(); ++i)
+        FuncAliasMap.try_emplace(Aliases[i], Aliases.front());
+
+      // Prepare for parsing clusters of this function name.
+      // Start a new cluster map for this function name.
+      FI = ProgramBBClusterInfo.try_emplace(Aliases.front()).first;
+      CurrentCluster = 0;
+      FuncBBIDs.clear();
+    }
+  }
+  return Error::success();
+}
+
+void BasicBlockSectionsInfo::initializePass() {
+  if (!MBuf)
+    return;
+  if (auto Err = getBBClusterInfo(MBuf, ProgramBBClusterInfo, FuncAliasMap))
+    report_fatal_error(std::move(Err));
+}
+
+ImmutablePass *llvm::createBasicBlockSectionsInfoPass(const MemoryBuffer *Buf) {
+  return new BasicBlockSectionsInfo(Buf);
+}

llvm/lib/Analysis/CMakeLists.txt

add_llvm_component_library(LLVMAnalysis
   AliasAnalysis.cpp
   AliasAnalysisEvaluator.cpp
   AliasAnalysisSummary.cpp
   AliasSetTracker.cpp
   Analysis.cpp
   AssumeBundleQueries.cpp
   AssumptionCache.cpp
   BasicAliasAnalysis.cpp
+  BasicBlockSectionsInfo.cpp
   BlockFrequencyInfo.cpp
   BlockFrequencyInfoImpl.cpp
   BranchProbabilityInfo.cpp
   CFG.cpp
   CFGPrinter.cpp
   CFLAndersAliasAnalysis.cpp
   CFLSteensAliasAnalysis.cpp
   CGSCCPassManager.cpp

llvm/lib/CodeGen/BasicBlockSections.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/BasicBlockSectionsInfo.h"
 #include "llvm/CodeGen/BasicBlockSectionUtils.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/LineIterator.h"
-#include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Target/TargetMachine.h"
 
 using llvm::SmallSet;
 using llvm::SmallVector;
 using llvm::StringMap;
 using llvm::StringRef;
 using namespace llvm;
 

 cl::opt<bool> BBSectionsDetectSourceDrift(
     "bbsections-detect-source-drift",
     cl::desc("This checks if there is a fdo instr. profile hash "
              "mismatch for this function"),
     cl::init(true), cl::Hidden);
 
 namespace {
 
-// This struct represents the cluster information for a machine basic block.
-struct BBClusterInfo {
-  // MachineBasicBlock ID.
-  unsigned MBBNumber;
-  // Cluster ID this basic block belongs to.
-  unsigned ClusterID;
-  // Position of basic block within the cluster.
-  unsigned PositionInCluster;
-};
-
-using ProgramBBClusterInfoMapTy = StringMap<SmallVector<BBClusterInfo, 4>>;
-
 class BasicBlockSections : public MachineFunctionPass {
 public:
   static char ID;
 
-  // This contains the basic-block-sections profile.
-  const MemoryBuffer *MBuf = nullptr;
-
-  // This encapsulates the BB cluster information for the whole program.
-  //
-  // For every function name, it contains the cluster information for (all or
-  // some of) its basic blocks. The cluster information for every basic block
-  // includes its cluster ID along with the position of the basic block in that
-  // cluster.
-  ProgramBBClusterInfoMapTy ProgramBBClusterInfo;
-
-  // Some functions have alias names. We use this map to find the main alias
-  // name for which we have mapping in ProgramBBClusterInfo.
-  StringMap<StringRef> FuncAliasMap;
-
-  BasicBlockSections(const MemoryBuffer *Buf)
-      : MachineFunctionPass(ID), MBuf(Buf) {
-    initializeBasicBlockSectionsPass(*PassRegistry::getPassRegistry());
-  };
+  BasicBlockSectionsInfo *BBSectionsInfo = nullptr;
 
   BasicBlockSections() : MachineFunctionPass(ID) {
     initializeBasicBlockSectionsPass(*PassRegistry::getPassRegistry());
   }
 
   StringRef getPassName() const override {
     return "Basic Block Sections Analysis";
   }
 
   void getAnalysisUsage(AnalysisUsage &AU) const override;
 
-  /// Read profiles of basic blocks if available here.
-  bool doInitialization(Module &M) override;
-
   /// Identify basic blocks that need separate sections and prepare to emit them
   /// accordingly.
   bool runOnMachineFunction(MachineFunction &MF) override;
 };
 
 } // end anonymous namespace
 
 char BasicBlockSections::ID = 0;
for (auto &MBB : MF) {
     Cond.clear();
     MachineBasicBlock *TBB = nullptr, *FBB = nullptr; // For analyzeBranch.
     if (TII->analyzeBranch(MBB, TBB, FBB, Cond))
       continue;
     MBB.updateTerminator(FTMBB);
   }
 }
 
-// This function provides the BBCluster information associated with a function.
-// Returns true if a valid association exists and false otherwise.
-static bool getBBClusterInfoForFunction(
-    const MachineFunction &MF, const StringMap<StringRef> FuncAliasMap,
-    const ProgramBBClusterInfoMapTy &ProgramBBClusterInfo,
-    std::vector<Optional<BBClusterInfo>> &V) {
-  // Get the main alias name for the function.
-  auto FuncName = MF.getName();
-  auto R = FuncAliasMap.find(FuncName);
-  StringRef AliasName = R == FuncAliasMap.end() ? FuncName : R->second;
-
-  // Find the assoicated cluster information.
-  auto P = ProgramBBClusterInfo.find(AliasName);
-  if (P == ProgramBBClusterInfo.end())
-    return false;
-
-  if (P->second.empty()) {
-    // This indicates that sections are desired for all basic blocks of this
-    // function. We clear the BBClusterInfo vector to denote this.
-    V.clear();
-    return true;
-  }
-
-  V.resize(MF.getNumBlockIDs());
-  for (auto bbClusterInfo : P->second) {
-    // Bail out if the cluster information contains invalid MBB numbers.
-    if (bbClusterInfo.MBBNumber >= MF.getNumBlockIDs())
-      return false;
-    V[bbClusterInfo.MBBNumber] = bbClusterInfo;
-  }
-  return true;
-}
-
 // This function sorts basic blocks according to the cluster's information.
 // All explicitly specified clusters of basic blocks will be ordered
 // accordingly. All non-specified BBs go into a separate "Cold" section.
 // Additionally, if exception handling landing pads end up in more than one
 // clusters, they are moved into a single "Exception" section. Eventually,
 // clusters are ordered in increasing order of their IDs, with the "Exception"
 // and "Cold" succeeding all other clusters.
 // FuncBBClusterInfo represent the cluster information for basic blocks. If this
if (Existing) {
     for (auto &N : Tuple->operands())
       if (cast<MDString>(N.get())->getString() == MetadataName)
         return true;
   }
 
   return false;
 }
 
+// This function provides the BBCluster information associated with a function.
+// Returns true if a valid association exists and false otherwise.
+bool getBBClusterInfoForFunction(const MachineFunction &MF,
+                                 BasicBlockSectionsInfo *BBSectionsInfo,
+                                 std::vector<Optional<BBClusterInfo>> &V) {
+
+  // Find the assoicated cluster information.
+  std::pair<bool, SmallVector<BBClusterInfo, 4>> P =
+      BBSectionsInfo->getBBClusterInfoForFunction(MF.getName());
+  if (!P.first)
+    return false;
+
+  if (P.second.empty()) {
+    // This indicates that sections are desired for all basic blocks of this
+    // function. We clear the BBClusterInfo vector to denote this.
+    V.clear();
+    return true;
+  }
+
+  V.resize(MF.getNumBlockIDs());
+  for (auto bbClusterInfo : P.second) {
+    // Bail out if the cluster information contains invalid MBB numbers.
+    if (bbClusterInfo.MBBNumber >= MF.getNumBlockIDs())
+      return false;
+    V[bbClusterInfo.MBBNumber] = bbClusterInfo;
+  }
+  return true;
+}
+
 bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
   auto BBSectionsType = MF.getTarget().getBBSectionsType();
   assert(BBSectionsType != BasicBlockSection::None &&
          "BB Sections not enabled!");
 
   // Check for source drift.  If the source has changed since the profiles
   // were obtained, optimizing basic blocks might be sub-optimal.
   // This only applies to BasicBlockSection::List as it creates
bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
   // labels to match the profiles with the correct blocks.
   MF.RenumberBlocks();
 
   if (BBSectionsType == BasicBlockSection::Labels) {
     MF.setBBSectionsType(BBSectionsType);
     return true;
   }
 
+  BBSectionsInfo = &getAnalysis<BasicBlockSectionsInfo>();
+
   std::vector<Optional<BBClusterInfo>> FuncBBClusterInfo;
   if (BBSectionsType == BasicBlockSection::List &&
-      !getBBClusterInfoForFunction(MF, FuncAliasMap, ProgramBBClusterInfo,
-                                   FuncBBClusterInfo))
+      !getBBClusterInfoForFunction(MF, BBSectionsInfo, FuncBBClusterInfo))
     return true;
   MF.setBBSectionsType(BBSectionsType);
   assignSections(MF, FuncBBClusterInfo);
 
   // We make sure that the cluster including the entry basic block precedes all
   // other clusters.
   auto EntryBBSectionID = MF.front().getSectionID();
 
auto Comparator = [&](const MachineBasicBlock &X,
     return X.getNumber() < Y.getNumber();
   };
 
   sortBasicBlocksAndUpdateBranches(MF, Comparator);
   avoidZeroOffsetLandingPad(MF);
   return true;
 }
 
-// Basic Block Sections can be enabled for a subset of machine basic blocks.
-// This is done by passing a file containing names of functions for which basic
-// block sections are desired.  Additionally, machine basic block ids of the
-// functions can also be specified for a finer granularity. Moreover, a cluster
-// of basic blocks could be assigned to the same section.
-// A file with basic block sections for all of function main and three blocks
-// for function foo (of which 1 and 2 are placed in a cluster) looks like this:
-// ----------------------------
-// list.txt:
-// !main
-// !foo
-// !!1 2
-// !!4
-static Error getBBClusterInfo(const MemoryBuffer *MBuf,
-                              ProgramBBClusterInfoMapTy &ProgramBBClusterInfo,
-                              StringMap<StringRef> &FuncAliasMap) {
-  assert(MBuf);
-  line_iterator LineIt(*MBuf, /*SkipBlanks=*/true, /*CommentMarker=*/'#');
-
-  auto invalidProfileError = [&](auto Message) {
-    return make_error<StringError>(
-        Twine("Invalid profile " + MBuf->getBufferIdentifier() + " at line " +
-              Twine(LineIt.line_number()) + ": " + Message),
-        inconvertibleErrorCode());
-  };
-
-  auto FI = ProgramBBClusterInfo.end();
-
-  // Current cluster ID corresponding to this function.
-  unsigned CurrentCluster = 0;
-  // Current position in the current cluster.
-  unsigned CurrentPosition = 0;
-
-  // Temporary set to ensure every basic block ID appears once in the clusters
-  // of a function.
-  SmallSet<unsigned, 4> FuncBBIDs;
-
-  for (; !LineIt.is_at_eof(); ++LineIt) {
-    StringRef S(*LineIt);
-    if (S[0] == '@')
-      continue;
-    // Check for the leading "!"
-    if (!S.consume_front("!") || S.empty())
-      break;
-    // Check for second "!" which indicates a cluster of basic blocks.
-    if (S.consume_front("!")) {
-      if (FI == ProgramBBClusterInfo.end())
-        return invalidProfileError(
-            "Cluster list does not follow a function name specifier.");
-      SmallVector<StringRef, 4> BBIndexes;
-      S.split(BBIndexes, ' ');
-      // Reset current cluster position.
-      CurrentPosition = 0;
-      for (auto BBIndexStr : BBIndexes) {
-        unsigned long long BBIndex;
-        if (getAsUnsignedInteger(BBIndexStr, 10, BBIndex))
-          return invalidProfileError(Twine("Unsigned integer expected: '") +
-                                     BBIndexStr + "'.");
-        if (!FuncBBIDs.insert(BBIndex).second)
-          return invalidProfileError(Twine("Duplicate basic block id found '") +
-                                     BBIndexStr + "'.");
-        if (!BBIndex && CurrentPosition)
-          return invalidProfileError("Entry BB (0) does not begin a cluster.");
-
-        FI->second.emplace_back(BBClusterInfo{
-            ((unsigned)BBIndex), CurrentCluster, CurrentPosition++});
-      }
-      CurrentCluster++;
-    } else { // This is a function name specifier.
-      // Function aliases are separated using '/'. We use the first function
-      // name for the cluster info mapping and delegate all other aliases to
-      // this one.
-      SmallVector<StringRef, 4> Aliases;
-      S.split(Aliases, '/');
-      for (size_t i = 1; i < Aliases.size(); ++i)
-        FuncAliasMap.try_emplace(Aliases[i], Aliases.front());
-
-      // Prepare for parsing clusters of this function name.
-      // Start a new cluster map for this function name.
-      FI = ProgramBBClusterInfo.try_emplace(Aliases.front()).first;
-      CurrentCluster = 0;
-      FuncBBIDs.clear();
-    }
-  }
-  return Error::success();
-}
-
-bool BasicBlockSections::doInitialization(Module &M) {
-  if (!MBuf)
-    return false;
-  if (auto Err = getBBClusterInfo(MBuf, ProgramBBClusterInfo, FuncAliasMap))
-    report_fatal_error(std::move(Err));
-  return false;
-}
-
 void BasicBlockSections::getAnalysisUsage(AnalysisUsage &AU) const {
   AU.setPreservesAll();
+  AU.addRequired<BasicBlockSectionsInfo>();
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-MachineFunctionPass *
-llvm::createBasicBlockSectionsPass(const MemoryBuffer *Buf) {
-  return new BasicBlockSections(Buf);
+MachineFunctionPass *llvm::createBasicBlockSectionsPass() {
+  return new BasicBlockSections();
 }

llvm/lib/CodeGen/CodeGenPrepare.cpp

 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/Analysis/BasicBlockSectionsInfo.h"
 #include "llvm/Analysis/BlockFrequencyInfo.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
cl::desc("In profiling mode like sampleFDO, if a function doesn't have "
              "it may be a function newly added without ever being sampled. "
              "With the flag enabled, compiler can put such profile unknown "
              "functions into a special section, so runtime system can choose "
              "to handle it in a different way than .text section, to save "
              "RAM for example. "));
 
 static cl::opt<unsigned> FreqRatioToSkipMerge(
     "cgp-freq-ratio-to-skip-merge", cl::Hidden, cl::init(2),
     cl::desc("Skip merging empty blocks if (frequency of empty block) / "
              "(frequency of destination block) is greater than this ratio"));
 
 static cl::opt<bool> ForceSplitStore(
     "force-split-store", cl::Hidden, cl::init(false),
     cl::desc("Force store splitting no matter what the target query says."));
 
 static cl::opt<bool>
 EnableTypePromotionMerge("cgp-type-promotion-merge", cl::Hidden,

 class TypePromotionTransaction;
 
   class CodeGenPrepare : public FunctionPass {
     const TargetMachine *TM = nullptr;
     const TargetSubtargetInfo *SubtargetInfo;
     const TargetLowering *TLI = nullptr;
     const TargetRegisterInfo *TRI;
     const TargetTransformInfo *TTI = nullptr;
+    const BasicBlockSectionsInfo *BBSectionsInfo = nullptr;
     const TargetLibraryInfo *TLInfo;
     const LoopInfo *LI;
     std::unique_ptr<BlockFrequencyInfo> BFI;
     std::unique_ptr<BranchProbabilityInfo> BPI;
     ProfileSummaryInfo *PSI;
 
     /// As we scan instructions optimizing them, this is the next instruction
     /// to optimize. Transforms that can invalidate this should update it.
public:
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
       // FIXME: When we can selectively preserve passes, preserve the domtree.
       AU.addRequired<ProfileSummaryInfoWrapperPass>();
       AU.addRequired<TargetLibraryInfoWrapperPass>();
       AU.addRequired<TargetPassConfig>();
       AU.addRequired<TargetTransformInfoWrapperPass>();
       AU.addRequired<LoopInfoWrapperPass>();
+      AU.addRequired<BasicBlockSectionsInfo>();
     }
 
   private:
     template <typename F>
     void resetIteratorIfInvalidatedWhileCalling(BasicBlock *BB, F f) {
       // Substituting can cause recursive simplifications, which can invalidate
       // our iterator.  Use a WeakTrackingVH to hold onto it in case this
       // happens.

 
 INITIALIZE_PASS_BEGIN(CodeGenPrepare, DEBUG_TYPE,
                       "Optimize for code generation", false, false)
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(ProfileSummaryInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(BasicBlockSectionsInfo)
 INITIALIZE_PASS_END(CodeGenPrepare, DEBUG_TYPE,
                     "Optimize for code generation", false, false)
 
 FunctionPass *llvm::createCodeGenPreparePass() { return new CodeGenPrepare(); }
 
 bool CodeGenPrepare::runOnFunction(Function &F) {
   if (skipFunction(F))
     return false;
bool CodeGenPrepare::runOnFunction(Function &F) {
   TLI = SubtargetInfo->getTargetLowering();
   TRI = SubtargetInfo->getRegisterInfo();
   TLInfo = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(F);
   TTI = &getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
   LI = &getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
   BPI.reset(new BranchProbabilityInfo(F, *LI));
   BFI.reset(new BlockFrequencyInfo(F, *BPI, *LI));
   PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
+  BBSectionsInfo = getAnalysisIfAvailable<BasicBlockSectionsInfo>();
   OptSize = F.hasOptSize();
   if (ProfileGuidedSectionPrefix) {
     // The hot attribute overwrites profile count based hotness while profile
     // counts based hotness overwrite the cold attribute.
     // This is a conservative behabvior.
-    if (F.hasFnAttribute(Attribute::Hot) ||
+    if (BBSectionsInfo->getBBClusterInfoForFunction(F.getName()).first ||
+        F.hasFnAttribute(Attribute::Hot) ||
         PSI->isFunctionHotInCallGraph(&F, *BFI))
       F.setSectionPrefix("hot");
     // If PSI shows this function is not hot, we will placed the function
     // into unlikely section if (1) PSI shows this is a cold function, or
     // (2) the function has a attribute of cold.
     else if (PSI->isFunctionColdInCallGraph(&F, *BFI) ||
              F.hasFnAttribute(Attribute::Cold))
       F.setSectionPrefix("unlikely");

llvm/lib/CodeGen/TargetPassConfig.cpp

 //
 //===---------------------------------------------------------------------===//
 
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/BasicAliasAnalysis.h"
+#include "llvm/Analysis/BasicBlockSectionsInfo.h"
 #include "llvm/Analysis/CFLAndersAliasAnalysis.h"
 #include "llvm/Analysis/CFLSteensAliasAnalysis.h"
 #include "llvm/Analysis/CallGraphSCCPass.h"
 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
 #include "llvm/CodeGen/CSEConfigBase.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
if (AddOutliner)
       addPass(createMachineOutlinerPass(RunOnAllFunctions));
   }
 
   // Machine function splitter uses the basic block sections feature. Both
   // cannot be enabled at the same time. Basic block sections takes precedence.
   // FIXME: In principle, BasicBlockSection::Labels and splitting can used
   // together. Update this check once we have addressed any issues.
   if (TM->getBBSectionsType() != llvm::BasicBlockSection::None) {
-    addPass(llvm::createBasicBlockSectionsPass(TM->getBBSectionsFuncListBuf()));
+    if (TM->getBBSectionsType() == llvm::BasicBlockSection::List) {
+      addPass(llvm::createBasicBlockSectionsInfoPass(
+          TM->getBBSectionsFuncListBuf()));
+    }
+    addPass(llvm::createBasicBlockSectionsPass());
   } else if (TM->Options.EnableMachineFunctionSplitter ||
              EnableMachineFunctionSplitter) {
     addPass(createMachineFunctionSplitterPass());
   }
 
   if (!DisableCFIFixup && TM->Options.EnableCFIFixup)
     addPass(createCFIFixup());
 

llvm/test/CodeGen/X86/basic-block-sections-cold.ll

 6:                                                ; preds = %2, %4
   ret void
 }
 
 declare i32 @_Z3barv() #1
 
 declare i32 @_Z3foov() #1
 
-; LINUX-SECTIONS: .section        .text._Z3bazb,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3bazb,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb:
 ; Check that the basic block with id 1 doesn't get a section.
-; LINUX-SECTIONS-NOT: .section        .text._Z3bazb._Z3bazb.1,"ax",@progbits,unique
+; LINUX-SECTIONS-NOT: .section        .text{{.*}}._Z3bazb.1,"ax",@progbits,unique
 ; Check that a single cold section is started here and id 1 and 2 blocks are placed here.
 ; LINUX-SECTIONS: .section  .text.split._Z3bazb,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb.cold:
-; LINUX-SECTIONS-NOT: .section        .text._Z3bazb._Z3bazb.2,"ax",@progbits,unique
+; LINUX-SECTIONS-NOT: .section        .text.hot._Z3bazb._Z3bazb.2,"ax",@progbits,unique
 ; LINUX-SECTIONS: .LBB0_2:
 ; LINUX-SECTIONS: .size   _Z3bazb, .Lfunc_end{{[0-9]}}-_Z3bazb
 
 ; LINUX-SPLIT:      .section  .text.unlikely._Z3bazb,"ax",@progbits
 ; LINUX-SPLIT-NEXT: _Z3bazb.cold:
 ; LINUX-SPLIT-NEXT:   callq _Z3barv
 ; LINUX-SPLIT:      .LBB0_2:
 ; LINUX-SPLIT:      .LBB_END0_2:

llvm/test/CodeGen/X86/basic-block-sections-list.ll

11:                                               ; preds = %1
   store i32 %12, i32* %2, align 4
   br label %13
 
 13:                                               ; preds = %11, %9
   %14 = load i32, i32* %2, align 4
   ret i32 %14
 }
 
-; LINUX-SECTIONS: .section        .text._Z3foob,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3foob,"ax",@progbits
 ; LINUX-SECTIONS: _Z3foob:
-; LINUX-SECTIONS: .section        .text._Z3foob._Z3foob.__part.1,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3foob._Z3foob.__part.1,"ax",@progbits
 ; LINUX-SECTIONS: _Z3foob.__part.1:
-; LINUX-SECTIONS: .section        .text._Z3foob._Z3foob.__part.2,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3foob._Z3foob.__part.2,"ax",@progbits
 ; LINUX-SECTIONS: _Z3foob.__part.2:
-; LINUX-SECTIONS: .section        .text._Z3foob._Z3foob.__part.3,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3foob._Z3foob.__part.3,"ax",@progbits
 ; LINUX-SECTIONS: _Z3foob.__part.3:
 
 ; LINUX-SECTIONS-FUNCTION-SECTION: .section        .text._Z3zipb,"ax",@progbits
-; LINUX-SECIONS-NO-FUNCTION-SECTION-NOT: .section        .text._Z3zipb,"ax",@progbits
+; LINUX-SECIONS-NO-FUNCTION-SECTION-NOT: .section        .text{{.*}}._Z3zipb,"ax",@progbits
 ; LINUX-SECTIONS: _Z3zipb:
-; LINUX-SECTIONS-NOT: .section        .text._Z3zipb._Z3zipb.__part.{{[0-9]+}},"ax",@progbits
+; LINUX-SECTIONS-NOT: .section        .text{{.*}}._Z3zipb.__part.{{[0-9]+}},"ax",@progbits
 ; LINUX-SECTIONS-NOT: _Z3zipb.__part.{{[0-9]+}}:

llvm/test/CodeGen/X86/basic-block-sections-listbb.ll

 }
 
 declare i32 @_Z3barv() #1
 
 declare i32 @_Z3foov() #1
 
 ; Check that the correct block is found using the call insts for foo and bar.
 ;
-; LINUX-SECTIONS: .section        .text._Z3bazb,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3bazb,"ax",@progbits
 ; LINUX-SECTIONS: _Z3bazb:
 ; Check that the basic block with id 1 doesn't get a section.
-; LINUX-SECTIONS-NOT: .section        .text._Z3bazb._Z3bazb.__part.{{[0-9]+}},"ax",@progbits
+; LINUX-SECTIONS-NOT: .section        .text{{.*}}._Z3bazb.__part.{{[0-9]+}},"ax",@progbits
 ; LINUX-SECTIONS-LABEL: # %bb.1:
 ; LINUX-SECTIONS-NEXT:        callq   _Z3barv
-; LINUX-SECTIONS: .section        .text._Z3bazb.[[SECTION_LABEL:_Z3bazb.__part.[0-9]+]],"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3bazb.[[SECTION_LABEL:_Z3bazb.__part.[0-9]+]],"ax",@progbits
 ; LINUX-SECTIONS-NEXT: [[SECTION_LABEL]]:
 ; LINUX-SECTIONS-NEXT:        callq   _Z3foov
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL]], .LBB_END0_2-[[SECTION_LABEL]]
-; LINUX-SECTIONS: .section        .text._Z3bazb,"ax",@progbits
+; LINUX-SECTIONS: .section        .text.hot._Z3bazb,"ax",@progbits
 ; LINUX-SECTIONS: .Lfunc_end0:
 ; LINUX-SECTIONS-NEXT: .size _Z3bazb, .Lfunc_end0-_Z3bazb

llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll

3:                                                ; preds = %2
   ret i32 %4
 
 5:                                                ; preds = %2
   ret i32 1
 }
 
 !1 = !{!"instr_prof_hash_mismatch"}
 
-; SOURCE-DRIFT-NOT: .section .text
-; HASH-CHECK-DISABLED: .section .text
+; SOURCE-DRIFT-NOT: .section .text{{.*}}.foo
+; HASH-CHECK-DISABLED: .section .text.hot.foo

llvm/test/CodeGen/X86/opt-pipeline.ll

 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
 ; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
+; CHECK-NEXT: Basic Block Sections Info Analysis
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT: Default Regalloc Eviction Advisor
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT:     Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT:     FunctionPass Manager
 ; CHECK-NEXT:       Expand Atomic instructions
 ; CHECK-NEXT:       Lower AMX intrinsics
 ; CHECK-NEXT:       Lower AMX type for load/store