[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

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

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/CodeGen/BasicBlockSectionsProfileReader.h

+//===-- BasicBlockSectionsProfileReader.h - BB sections profile reader 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 namespace llvm;
+
+namespace llvm {
+
+// 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>>;
+
+class BasicBlockSectionsProfileReader : public ImmutablePass {
+public:
+  static char ID;
+
+  BasicBlockSectionsProfileReader(const MemoryBuffer *Buf)
+      : ImmutablePass(ID), MBuf(Buf) {
+    initializeBasicBlockSectionsProfileReaderPass(
+        *PassRegistry::getPassRegistry());
+  };
+
+  BasicBlockSectionsProfileReader() : ImmutablePass(ID) {
+    initializeBasicBlockSectionsProfileReaderPass(
+        *PassRegistry::getPassRegistry());
+  }
+
+  StringRef getPassName() const override {
+    return "Basic Block Sections Profile Reader";
+  }
+
+  // Returns true if basic block sections profile exist for function \p
+  // FuncName.
+  bool isFunctionHot(StringRef FuncName) const;
+
+  // Returns a pair with first element representing whether basic block sections
+  // profile exist for the function \p FuncName, and the second element
+  // representing the basic block sections profile (cluster info) for this
+  // function. If the first element is true and the second element is empty, it
+  // means unique basic block sections are desired for all basic blocks of the
+  // function.
+  std::pair<bool, SmallVector<BBClusterInfo>>
+  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;
+};
+
+// Creates a BasicBlockSectionsProfileReader pass to parse the basic block
+// sections profile. \p Buf is a memory buffer that contains the list of
+// functions and basic block ids to selectively enable basic block sections.
+ImmutablePass *
+createBasicBlockSectionsProfileReaderPass(const MemoryBuffer *Buf);
+
+} // namespace llvm
+#endif // LLVM_ANALYSIS_BASICBLOCKSECTIONSINFO_H

llvm/include/llvm/CodeGen/Passes.h

   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);
+  /// basic blocks and is enabled with -fbasic-block-sections.
+  MachineFunctionPass *createBasicBlockSectionsPass();
 
   /// createMachineFunctionSplitterPass - This pass splits machine functions
   /// using profile information.

llvm/include/llvm/InitializePasses.h

 void initializeAtomicExpandPass(PassRegistry&);
 void initializeAttributorLegacyPassPass(PassRegistry&);
 void initializeAttributorCGSCCLegacyPassPass(PassRegistry &);
+void initializeBasicBlockSectionsProfileReaderPass(PassRegistry &);
 void initializeBasicBlockSectionsPass(PassRegistry &);
 void initializeBDCELegacyPassPass(PassRegistry&);
 void initializeBarrierNoopPass(PassRegistry&);

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/CodeGen/BasicBlockSectionsProfileReader.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;
 
 // Placing the cold clusters in a separate section mitigates against poor

 
 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());
-  };
+  BasicBlockSectionsProfileReader *BBSectionsProfileReader = nullptr;
 
   BasicBlockSections() : MachineFunctionPass(ID) {
     initializeBasicBlockSectionsPass(*PassRegistry::getPassRegistry());

 
   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;

 
 // 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,
+bool getBBClusterInfoForFunction(
+    const MachineFunction &MF,
+    BasicBlockSectionsProfileReader *BBSectionsProfileReader,
     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())
+  std::pair<bool, SmallVector<BBClusterInfo, 4>> P =
+      BBSectionsProfileReader->getBBClusterInfoForFunction(MF.getName());
+  if (!P.first)
     return false;
 
-  if (P->second.empty()) {
+  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();

   }
 
   V.resize(MF.getNumBlockIDs());
-  for (auto bbClusterInfo : P->second) {
+  for (auto bbClusterInfo : P.second) {
     // Bail out if the cluster information contains invalid MBB numbers.
     if (bbClusterInfo.MBBNumber >= MF.getNumBlockIDs())
       return false;

     return true;
   }
 
+  BBSectionsProfileReader = &getAnalysis<BasicBlockSectionsProfileReader>();
+
   std::vector<Optional<BBClusterInfo>> FuncBBClusterInfo;
   if (BBSectionsType == BasicBlockSection::List &&
-      !getBBClusterInfoForFunction(MF, FuncAliasMap, ProgramBBClusterInfo,
+      !getBBClusterInfoForFunction(MF, BBSectionsProfileReader,
                                    FuncBBClusterInfo))
     return true;
   MF.setBBSectionsType(BBSectionsType);

   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<BasicBlockSectionsProfileReader>();
   MachineFunctionPass::getAnalysisUsage(AU);
 }
 
-MachineFunctionPass *
-llvm::createBasicBlockSectionsPass(const MemoryBuffer *Buf) {
-  return new BasicBlockSections(Buf);
+MachineFunctionPass *llvm::createBasicBlockSectionsPass() {
+  return new BasicBlockSections();
 }

llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp

+//===-- BasicBlockSectionsProfileReader.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 profile reader pass. It parses
+// and stores the basic block sections profile file (which is specified via the
+// `-basic-block-sections` flag).
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/BasicBlockSectionsProfileReader.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 namespace llvm;
+
+char BasicBlockSectionsProfileReader::ID = 0;
+INITIALIZE_PASS(BasicBlockSectionsProfileReader, "bbsections-profile-reader",
+                "Reads and parses a basic block sections profile.", false,
+                false)
+
+bool BasicBlockSectionsProfileReader::isFunctionHot(StringRef FuncName) const {
+  return getBBClusterInfoForFunction(FuncName).first;
+}
+
+std::pair<bool, SmallVector<BBClusterInfo>>
+BasicBlockSectionsProfileReader::getBBClusterInfoForFunction(
+    StringRef FuncName) const {
+  std::pair<bool, SmallVector<BBClusterInfo>> 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 BasicBlockSectionsProfileReader::initializePass() {
+  if (!MBuf)
+    return;
+  if (auto Err = getBBClusterInfo(MBuf, ProgramBBClusterInfo, FuncAliasMap))
+    report_fatal_error(std::move(Err));
+}
+
+ImmutablePass *
+llvm::createBasicBlockSectionsProfileReaderPass(const MemoryBuffer *Buf) {
+  return new BasicBlockSectionsProfileReader(Buf);
+}

llvm/lib/CodeGen/CMakeLists.txt

   BranchRelaxation.cpp
   BreakFalseDeps.cpp
   BasicBlockSections.cpp
+  BasicBlockSectionsProfileReader.cpp
   CalcSpillWeights.cpp
   CallingConvLower.cpp
   CFGuardLongjmp.cpp

llvm/lib/CodeGen/CodeGenPrepare.cpp

 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/CodeGen/Analysis.h"
+#include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
 #include "llvm/CodeGen/ISDOpcodes.h"
 #include "llvm/CodeGen/SelectionDAGNodes.h"
 #include "llvm/CodeGen/TargetLowering.h"

              "to handle it in a different way than .text section, to save "
              "RAM for example. "));
 
+static cl::opt<bool> BBSectionsGuidedSectionPrefix(
+    "bbsections-guided-section-prefix", cl::Hidden, cl::init(true),
+    cl::desc("Use the basic-block-sections profile to determine the text "
+             "section prefix for hot functions. Functions with "
+             "basic-block-sections profile will be placed in `.text.hot` "
+             "regardless of their FDO profile info. Other functions won't be "
+             "impacted, i.e., their prefixes will be decided by FDO/sampleFDO "
+             "profiles."));
+
 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) / "

     const TargetLowering *TLI = nullptr;
     const TargetRegisterInfo *TRI;
     const TargetTransformInfo *TTI = nullptr;
+    const BasicBlockSectionsProfileReader *BBSectionsProfileReader = nullptr;
     const TargetLibraryInfo *TLInfo;
     const LoopInfo *LI;
     std::unique_ptr<BlockFrequencyInfo> BFI;

       AU.addRequired<TargetPassConfig>();
       AU.addRequired<TargetTransformInfoWrapperPass>();
       AU.addRequired<LoopInfoWrapperPass>();
+      AU.addRequired<BasicBlockSectionsProfileReader>();
     }
 
   private:

 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetPassConfig)
 INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(BasicBlockSectionsProfileReader)
 INITIALIZE_PASS_END(CodeGenPrepare, DEBUG_TYPE,
                     "Optimize for code generation", false, false)
 

   BPI.reset(new BranchProbabilityInfo(F, *LI));
   BFI.reset(new BlockFrequencyInfo(F, *BPI, *LI));
   PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
+  BBSectionsProfileReader =
+      getAnalysisIfAvailable<BasicBlockSectionsProfileReader>();
   OptSize = F.hasOptSize();
-  if (ProfileGuidedSectionPrefix) {
+  // Use the basic-block-sections profile to promote hot functions to .text.hot if requested.
+  if (BBSectionsGuidedSectionPrefix && BBSectionsProfileReader &&
+      BBSectionsProfileReader->isFunctionHot(F.getName())) {
+    F.setSectionPrefix("hot");
+  } else if (ProfileGuidedSectionPrefix) {
     // The hot attribute overwrites profile count based hotness while profile
     // counts based hotness overwrite the cold attribute.
     // This is a conservative behabvior.

llvm/lib/CodeGen/TargetPassConfig.cpp

 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
+#include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
 #include "llvm/CodeGen/CSEConfigBase.h"
 #include "llvm/CodeGen/MachineFunctionPass.h"
 #include "llvm/CodeGen/MachinePassRegistry.h"

   // 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::createBasicBlockSectionsProfileReaderPass(
+          TM->getBBSectionsFuncListBuf()));
+    }
+    addPass(llvm::createBasicBlockSectionsPass());
   } else if (TM->Options.EnableMachineFunctionSplitter ||
              EnableMachineFunctionSplitter) {
     addPass(createMachineFunctionSplitterPass());

llvm/test/CodeGen/AArch64/O3-pipeline.ll

 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
+; CHECK-NEXT: Basic Block Sections Profile Reader
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT: Default Regalloc Eviction Advisor
 ; CHECK-NEXT:   ModulePass Manager

llvm/test/CodeGen/AMDGPU/llc-pipeline.ll

 ; GCN-O1-NEXT:External Alias Analysis
 ; GCN-O1-NEXT:Type-Based Alias Analysis
 ; GCN-O1-NEXT:Scoped NoAlias Alias Analysis
+; GCN-O1-NEXT:Basic Block Sections Profile Reader
 ; GCN-O1-NEXT:Argument Register Usage Information Storage
 ; GCN-O1-NEXT:Create Garbage Collector Module Metadata
 ; GCN-O1-NEXT:Machine Branch Probability Analysis

 ; GCN-O1-OPTS-NEXT:Scoped NoAlias Alias Analysis
 ; GCN-O1-OPTS-NEXT:Argument Register Usage Information Storage
 ; GCN-O1-OPTS-NEXT:Create Garbage Collector Module Metadata
+; GCN-O1-OPTS-NEXT:Basic Block Sections Profile Reader
 ; GCN-O1-OPTS-NEXT:Machine Branch Probability Analysis
 ; GCN-O1-OPTS-NEXT:Register Usage Information Storage
 ; GCN-O1-OPTS-NEXT:Default Regalloc Eviction Advisor

 ; GCN-O2-NEXT:Scoped NoAlias Alias Analysis
 ; GCN-O2-NEXT:Argument Register Usage Information Storage
 ; GCN-O2-NEXT:Create Garbage Collector Module Metadata
+; GCN-O2-NEXT:Basic Block Sections Profile Reader
 ; GCN-O2-NEXT:Machine Branch Probability Analysis
 ; GCN-O2-NEXT:Register Usage Information Storage
 ; GCN-O2-NEXT:Default Regalloc Eviction Advisor

 ; GCN-O3-NEXT:Scoped NoAlias Alias Analysis
 ; GCN-O3-NEXT:Argument Register Usage Information Storage
 ; GCN-O3-NEXT:Create Garbage Collector Module Metadata
+; GCN-O3-NEXT:Basic Block Sections Profile Reader
 ; GCN-O3-NEXT:Machine Branch Probability Analysis
 ; GCN-O3-NEXT:Register Usage Information Storage
 ; GCN-O3-NEXT:Default Regalloc Eviction Advisor

llvm/test/CodeGen/PowerPC/O3-pipeline.ll

 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
+; CHECK-NEXT: Basic Block Sections Profile Reader
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT: Default Regalloc Eviction Advisor
 ; CHECK-NEXT:   ModulePass Manager

llvm/test/CodeGen/RISCV/O3-pipeline.ll

 ; CHECK-NEXT: Assumption Cache Tracker
 ; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
+; CHECK-NEXT: Basic Block Sections Profile Reader
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT: Default Regalloc Eviction Advisor
 ; CHECK-NEXT:   ModulePass Manager

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

 
 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
 

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

 ; Check the basic block sections list option.
 ; RUN: echo '!_Z3foob' > %t
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-FUNCTION-SECTION
-;  llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-NO-FUNCTION-SECTION
+; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-NO-FUNCTION-SECTION
+; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t -unique-basic-block-section-names --bbsections-guided-section-prefix=false | FileCheck %s -check-prefix=LINUX-SECTIONS-NO-GUIDED-PREFIX
 
 define i32 @_Z3foob(i1 zeroext %0) nounwind {
   %2 = alloca i32, align 4

   ret i32 %14
 }
 
-; LINUX-SECTIONS: .section        .text._Z3foob,"ax",@progbits
+; LINUX-SECTIONS-NO-GUIDED-PREFIX: .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-SECTIONS-FUNCTION-SECTION: .section     .text._Z3zipb,"ax",@progbits
+; LINUX-SECTIONS-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

 
 ; 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

 
 !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: Assumption Cache Tracker
 ; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
+; CHECK-NEXT: Basic Block Sections Profile Reader
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT: Default Regalloc Eviction Advisor
 ; CHECK-NEXT:   ModulePass Manager