[llvm] [BasicBlockSections] Introduce the path cloning profile format to BasicBlockSectionsProfileReader. (PR #67214)
Rahman Lavaee via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 11 22:41:22 PDT 2023
https://github.com/rlavaee updated https://github.com/llvm/llvm-project/pull/67214
>From b2a9bf487badd1255e3234c4419e8df4f6ef0af9 Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Fri, 22 Sep 2023 22:03:35 +0000
Subject: [PATCH 1/3] [BasicBlockSections] Introduce the path cloning profile
format to BasicBlockSectionsProfileReader.
Following up on prior RFC (https://lists.llvm.org/pipermail/llvm-dev/2020-September/145357.html) we can now improve above our highly-optimized basic-block-sections binary (e.g., 2% for clang) by applying path cloning. Cloning can improve performance by reducing taken branches.
This patch prepares the profile format for applying cloning actions.
The basic block cloning profile format extends the basic block sections profile in two ways.
1. Specifies the cloning paths with a 'p' specifier. For example, `p 1 4 5` specifies that blocks with BB ids 4 and 5 must be cloned along the edge 1 --> 4.
2. For each cloned block, it will appear in the cluster info as `<bb_id>.<clone_id>` where `clone_id` is the id associated with this clone.
For example, the following profile specifies one cloned block (2) and determines its cluster position as well.
```
f foo
p 1 2
c 0 1 2.1 3 2 5
```
This patch keeps backward-compatibility (retains the behavior for old profile formats). This feature is only introduced for profile version >= 1.
---
.../CodeGen/BasicBlockSectionsProfileReader.h | 61 +++++++--
llvm/lib/CodeGen/BasicBlockSections.cpp | 70 ++++-------
.../BasicBlockSectionsProfileReader.cpp | 116 +++++++++++++-----
.../basic-block-sections-clusters-error.ll | 35 ++++--
4 files changed, 190 insertions(+), 92 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index ad26eee642ead4f..f04eab8172253d9 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -28,17 +28,53 @@
namespace llvm {
-// The cluster information for a machine basic block.
-struct BBClusterInfo {
- // Unique ID for this basic block.
+// This structure represents a unique ID for every block specified in the
+// profile.
+struct ProfileBBID {
unsigned BBID;
+ unsigned CloneID;
+};
+
+// Provides DenseMapInfo for ProfileBBID.
+template <> struct DenseMapInfo<ProfileBBID> {
+ static inline ProfileBBID getEmptyKey() {
+ unsigned EmptyKey = DenseMapInfo<unsigned>::getEmptyKey();
+ return ProfileBBID{EmptyKey, EmptyKey};
+ }
+ static inline ProfileBBID getTombstoneKey() {
+ unsigned TombstoneKey = DenseMapInfo<unsigned>::getTombstoneKey();
+ return ProfileBBID{TombstoneKey, TombstoneKey};
+ }
+ static unsigned getHashValue(const ProfileBBID &Val) {
+ std::pair<unsigned, unsigned> PairVal =
+ std::make_pair(Val.BBID, Val.CloneID);
+ return DenseMapInfo<std::pair<unsigned, unsigned>>::getHashValue(PairVal);
+ }
+ static bool isEqual(const ProfileBBID &LHS, const ProfileBBID &RHS) {
+ return DenseMapInfo<unsigned>::isEqual(LHS.BBID, RHS.BBID) &&
+ DenseMapInfo<unsigned>::isEqual(LHS.CloneID, RHS.CloneID);
+ }
+};
+
+// This struct represents the cluster information for a machine basic block,
+// which is specifed by a unique ID.
+template <typename BBIDType> struct BBProfile {
+ // Basic block ID.
+ BBIDType BasicBlockID;
// Cluster ID this basic block belongs to.
unsigned ClusterID;
// Position of basic block within the cluster.
unsigned PositionInCluster;
};
-using ProgramBBClusterInfoMapTy = StringMap<SmallVector<BBClusterInfo>>;
+// This represents the profile for one function.
+struct RawFunctionProfile {
+ // BB Cluster information specified by `ProfileBBID`s (before cloning).
+ SmallVector<BBProfile<ProfileBBID>> RawBBProfiles;
+ // Paths to clone. A path a -> b -> c -> d implies cloning b, c, and d along
+ // the edge a -> b.
+ SmallVector<SmallVector<unsigned>> ClonePaths;
+};
class BasicBlockSectionsProfileReader : public ImmutablePass {
public:
@@ -70,8 +106,8 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
// 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;
+ std::pair<bool, RawFunctionProfile>
+ getRawProfileForFunction(StringRef FuncName) const;
// Initializes the FunctionNameToDIFilename map for the current module and
// then reads the profile for matching functions.
@@ -91,6 +127,9 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
inconvertibleErrorCode());
}
+ // Parses a `ProfileBBID` from `S`.
+ Expected<ProfileBBID> parseProfileBBID(StringRef S) const;
+
// Reads the basic block sections profile for functions in this module.
Error ReadProfile();
@@ -113,11 +152,11 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
// 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;
+ // For every function name, it contains the cloning and 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.
+ StringMap<RawFunctionProfile> RawProgramProfile;
// Some functions have alias names. We use this map to find the main alias
// name for which we have mapping in ProgramBBClusterInfo.
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index de7c17082fa4bb9..5e61a96c696aff0 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -168,31 +168,6 @@ updateBranches(MachineFunction &MF,
}
}
-// 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,
- BasicBlockSectionsProfileReader *BBSectionsProfileReader,
- DenseMap<unsigned, BBClusterInfo> &V) {
-
- // Find the assoicated cluster information.
- std::pair<bool, SmallVector<BBClusterInfo, 4>> P =
- BBSectionsProfileReader->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;
- }
-
- for (const BBClusterInfo &BBCI : P.second)
- V[BBCI.BBID] = BBCI;
- 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.
@@ -200,12 +175,12 @@ bool getBBClusterInfoForFunction(
// 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. It
+// BBProfilesByBBID represents the cluster information for basic blocks. It
// maps from BBID of basic blocks to their cluster information. If this is
// empty, it means unique sections for all basic blocks in the function.
-static void
-assignSections(MachineFunction &MF,
- const DenseMap<unsigned, BBClusterInfo> &FuncBBClusterInfo) {
+static void assignSections(
+ MachineFunction &MF,
+ const DenseMap<unsigned, BBProfile<unsigned>> &BBProfilesByBBID) {
assert(MF.hasBBSections() && "BB Sections is not set for function.");
// This variable stores the section ID of the cluster containing eh_pads (if
// all eh_pads are one cluster). If more than one cluster contain eh_pads, we
@@ -216,17 +191,17 @@ assignSections(MachineFunction &MF,
// With the 'all' option, every basic block is placed in a unique section.
// With the 'list' option, every basic block is placed in a section
// associated with its cluster, unless we want individual unique sections
- // for every basic block in this function (if FuncBBClusterInfo is empty).
+ // for every basic block in this function (if BBProfilesByBBID is empty).
if (MF.getTarget().getBBSectionsType() == llvm::BasicBlockSection::All ||
- FuncBBClusterInfo.empty()) {
+ BBProfilesByBBID.empty()) {
// If unique sections are desired for all basic blocks of the function, we
// set every basic block's section ID equal to its original position in
// the layout (which is equal to its number). This ensures that basic
// blocks are ordered canonically.
MBB.setSectionID(MBB.getNumber());
} else {
- auto I = FuncBBClusterInfo.find(*MBB.getBBID());
- if (I != FuncBBClusterInfo.end()) {
+ auto I = BBProfilesByBBID.find(*MBB.getBBID());
+ if (I != BBProfilesByBBID.end()) {
MBB.setSectionID(I->second.ClusterID);
} else {
// BB goes into the special cold section if it is not specified in the
@@ -333,16 +308,25 @@ bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
return true;
}
- BBSectionsProfileReader = &getAnalysis<BasicBlockSectionsProfileReader>();
+ DenseMap<unsigned, BBProfile<unsigned>> BBProfilesByBBID;
+ if (BBSectionsType == BasicBlockSection::List) {
+ auto [HasProfile, RawProfile] =
+ getAnalysis<BasicBlockSectionsProfileReader>().getRawProfileForFunction(
+ MF.getName());
+ if (!HasProfile)
+ return true;
+ // TODO: Apply the path cloning profile.
+ for (const BBProfile<ProfileBBID> &BBP : RawProfile.RawBBProfiles) {
+ assert(!BBP.BasicBlockID.CloneID && "Path cloning is not supported yet.");
+ BBProfilesByBBID.try_emplace(BBP.BasicBlockID.BBID,
+ BBProfile<unsigned>{BBP.BasicBlockID.BBID,
+ BBP.ClusterID,
+ BBP.PositionInCluster});
+ }
+ }
- // Map from BBID of blocks to their cluster information.
- DenseMap<unsigned, BBClusterInfo> FuncBBClusterInfo;
- if (BBSectionsType == BasicBlockSection::List &&
- !getBBClusterInfoForFunction(MF, BBSectionsProfileReader,
- FuncBBClusterInfo))
- return true;
MF.setBBSectionsType(BBSectionsType);
- assignSections(MF, FuncBBClusterInfo);
+ assignSections(MF, BBProfilesByBBID);
// We make sure that the cluster including the entry basic block precedes all
// other clusters.
@@ -376,8 +360,8 @@ bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
// If the two basic block are in the same section, the order is decided by
// their position within the section.
if (XSectionID.Type == MBBSectionID::SectionType::Default)
- return FuncBBClusterInfo.lookup(*X.getBBID()).PositionInCluster <
- FuncBBClusterInfo.lookup(*Y.getBBID()).PositionInCluster;
+ return BBProfilesByBBID.lookup(*X.getBBID()).PositionInCluster <
+ BBProfilesByBBID.lookup(*Y.getBBID()).PositionInCluster;
return X.getNumber() < Y.getNumber();
};
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index ef5f1251f5324e4..eb5cbda696a7de3 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/CodeGen/BasicBlockSectionsProfileReader.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
@@ -34,17 +35,36 @@ INITIALIZE_PASS(BasicBlockSectionsProfileReader, "bbsections-profile-reader",
"Reads and parses a basic block sections profile.", false,
false)
+Expected<ProfileBBID>
+BasicBlockSectionsProfileReader::parseProfileBBID(StringRef S) const {
+ SmallVector<StringRef, 2> Parts;
+ S.split(Parts, '.');
+ if (Parts.size() > 2)
+ return createProfileParseError(Twine("unable to parse basic block id: '") +
+ S + "'");
+ unsigned long long BBID;
+ if (getAsUnsignedInteger(Parts[0], 10, BBID))
+ return createProfileParseError(
+ Twine("unable to parse BB id: '" + Parts[0]) +
+ "': unsigned integer expected");
+ unsigned long long CloneID = 0;
+ if (Parts.size() > 1 && getAsUnsignedInteger(Parts[1], 10, CloneID))
+ return createProfileParseError(Twine("unable to parse clone id: '") +
+ Parts[1] + "': unsigned integer expected");
+ return ProfileBBID{static_cast<unsigned>(BBID),
+ static_cast<unsigned>(CloneID)};
+}
+
bool BasicBlockSectionsProfileReader::isFunctionHot(StringRef FuncName) const {
- return getBBClusterInfoForFunction(FuncName).first;
+ return getRawProfileForFunction(FuncName).first;
}
-std::pair<bool, SmallVector<BBClusterInfo>>
-BasicBlockSectionsProfileReader::getBBClusterInfoForFunction(
+std::pair<bool, RawFunctionProfile>
+BasicBlockSectionsProfileReader::getRawProfileForFunction(
StringRef FuncName) const {
- auto R = ProgramBBClusterInfo.find(getAliasName(FuncName));
- return R != ProgramBBClusterInfo.end()
- ? std::pair(true, R->second)
- : std::pair(false, SmallVector<BBClusterInfo>{});
+ auto R = RawProgramProfile.find(getAliasName(FuncName));
+ return R != RawProgramProfile.end() ? std::pair(true, R->second)
+ : std::pair(false, RawFunctionProfile());
}
// Reads the version 1 basic block sections profile. Profile for each function
@@ -61,8 +81,24 @@ BasicBlockSectionsProfileReader::getBBClusterInfoForFunction(
// aliases. Basic block clusters are specified by 'c' and specify the cluster of
// basic blocks, and the internal order in which they must be placed in the same
// section.
+// This profile can also specify cloning paths which instruct the compiler to
+// clone basic blocks along a path. The cloned blocks are then specified in the
+// cluster information.
+// The following profile lists two cloning paths (starting with 'p') for
+// function bar and places the total 11 blocks within two clusters. Each cloned
+// block is identified by its original block id, along with its clone id. A
+// block cloned multiple times (2 in this example) appears with distinct clone
+// ids (2.1 and 2.2).
+// ---------------------------
+//
+// f main
+// f bar
+// p 1 2 3
+// p 4 2 5
+// c 2 3 5 6 7
+// c 1 2.1 3.1 4 2.2 5.1
Error BasicBlockSectionsProfileReader::ReadV1Profile() {
- auto FI = ProgramBBClusterInfo.end();
+ auto FI = RawProgramProfile.end();
// Current cluster ID corresponding to this function.
unsigned CurrentCluster = 0;
@@ -71,7 +107,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
// Temporary set to ensure every basic block ID appears once in the clusters
// of a function.
- SmallSet<unsigned, 4> FuncBBIDs;
+ DenseSet<ProfileBBID> FuncBBIDs;
// Debug-info-based module filename for the current function. Empty string
// means no filename.
@@ -106,7 +142,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
if (!FunctionFound) {
// Skip the following profile by setting the profile iterator (FI) to
// the past-the-end element.
- FI = ProgramBBClusterInfo.end();
+ FI = RawProgramProfile.end();
DIFilename = "";
continue;
}
@@ -115,7 +151,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
// Prepare for parsing clusters of this function name.
// Start a new cluster map for this function name.
- auto R = ProgramBBClusterInfo.try_emplace(Values.front());
+ auto R = RawProgramProfile.try_emplace(Values.front());
// Report error when multiple profiles have been specified for the same
// function.
if (!R.second)
@@ -132,27 +168,44 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
case 'c': // Basic block cluster specifier.
// Skip the profile when we the profile iterator (FI) refers to the
// past-the-end element.
- if (FI == ProgramBBClusterInfo.end())
+ if (FI == RawProgramProfile.end())
break;
// Reset current cluster position.
CurrentPosition = 0;
- for (auto BBIDStr : Values) {
- unsigned long long BBID;
- if (getAsUnsignedInteger(BBIDStr, 10, BBID))
- return createProfileParseError(Twine("unsigned integer expected: '") +
- BBIDStr + "'");
- if (!FuncBBIDs.insert(BBID).second)
+ for (auto BasicBlockIDStr : Values) {
+ auto BasicBlockID = parseProfileBBID(BasicBlockIDStr);
+ if (!BasicBlockID)
+ return BasicBlockID.takeError();
+ if (!FuncBBIDs.insert(*BasicBlockID).second)
return createProfileParseError(
- Twine("duplicate basic block id found '") + BBIDStr + "'");
- if (BBID == 0 && CurrentPosition)
+ Twine("duplicate basic block id found '") + BasicBlockIDStr +
+ "'");
+
+ if (!BasicBlockID->BBID && CurrentPosition)
return createProfileParseError(
- "entry BB (0) does not begin a cluster");
+ "entry BB (0) does not begin a cluster.");
- FI->second.emplace_back(
- BBClusterInfo{((unsigned)BBID), CurrentCluster, CurrentPosition++});
+ FI->second.RawBBProfiles.emplace_back(BBProfile<ProfileBBID>{
+ *std::move(BasicBlockID), CurrentCluster, CurrentPosition++});
}
CurrentCluster++;
continue;
+ case 'p': { // Basic block cloning path specifier.
+ SmallSet<unsigned, 5> BBsInPath;
+ FI->second.ClonePaths.push_back({});
+ for (size_t I = 0; I < Values.size(); ++I) {
+ auto BBIDStr = Values[I];
+ unsigned long long BBID = 0;
+ if (getAsUnsignedInteger(BBIDStr, 10, BBID))
+ return createProfileParseError(Twine("unsigned integer expected: '") +
+ BBIDStr + "'");
+ if (I != 0 && !BBsInPath.insert(BBID).second)
+ return createProfileParseError(
+ Twine("duplicate cloned block in path: '") + BBIDStr + "'");
+ FI->second.ClonePaths.back().push_back(BBID);
+ }
+ continue;
+ }
default:
return createProfileParseError(Twine("invalid specifier: '") +
Twine(Specifier) + "'");
@@ -162,8 +215,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
}
Error BasicBlockSectionsProfileReader::ReadV0Profile() {
- auto FI = ProgramBBClusterInfo.end();
-
+ auto FI = RawProgramProfile.end();
// Current cluster ID corresponding to this function.
unsigned CurrentCluster = 0;
// Current position in the current cluster.
@@ -184,7 +236,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
if (S.consume_front("!")) {
// Skip the profile when we the profile iterator (FI) refers to the
// past-the-end element.
- if (FI == ProgramBBClusterInfo.end())
+ if (FI == RawProgramProfile.end())
continue;
SmallVector<StringRef, 4> BBIDs;
S.split(BBIDs, ' ');
@@ -202,8 +254,10 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
return createProfileParseError(
"entry BB (0) does not begin a cluster");
- FI->second.emplace_back(
- BBClusterInfo{((unsigned)BBID), CurrentCluster, CurrentPosition++});
+ FI->second.RawBBProfiles.emplace_back(
+ BBProfile<ProfileBBID>({{static_cast<unsigned>(BBID), 0},
+ CurrentCluster,
+ CurrentPosition++}));
}
CurrentCluster++;
} else {
@@ -237,7 +291,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
if (!FunctionFound) {
// Skip the following profile by setting the profile iterator (FI) to
// the past-the-end element.
- FI = ProgramBBClusterInfo.end();
+ FI = RawProgramProfile.end();
continue;
}
for (size_t i = 1; i < Aliases.size(); ++i)
@@ -245,7 +299,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
// Prepare for parsing clusters of this function name.
// Start a new cluster map for this function name.
- auto R = ProgramBBClusterInfo.try_emplace(Aliases.front());
+ auto R = RawProgramProfile.try_emplace(Aliases.front());
// Report error when multiple profiles have been specified for the same
// function.
if (!R.second)
@@ -261,7 +315,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
// 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
+// 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.
// Optionally, a debug-info filename can be specified for each function to allow
diff --git a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll
index 5577601c02cfd79..597d8f6707ecca3 100644
--- a/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll
+++ b/llvm/test/CodeGen/X86/basic-block-sections-clusters-error.ll
@@ -32,13 +32,35 @@
; RUN: echo '!dummy1' >> %t8
; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t8 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR8
; CHECK-ERROR8: LLVM ERROR: invalid profile {{.*}} at line 2: invalid specifier: '!'
-; RUN: echo 'v1' > %t0
-; RUN: echo 'm dummy1/module1 dummy1/module2'
+; RUN: echo 'v1' > %t9
+; RUN: echo 'm dummy1/module1 dummy1/module2' >> %t9
; RUN: echo 'f dummy1' >> %t9
-; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t8 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR8
-; CHECK-ERROR9: LLVM ERROR: invalid profile {{.*}} at line 2: invalid module name value: 'dummy1/module dummy1/module2'
-
-
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t9 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR9
+; CHECK-ERROR9: LLVM ERROR: invalid profile {{.*}} at line 2: invalid module name value: 'dummy1/module1 dummy1/module2'
+;;
+;; Error handling for version 1, cloning paths.
+; RUN: echo 'v1' > %t10
+; RUN: echo 'f dummy1' >> %t10
+; RUN: echo 'c 0 1.1.1' >> %t10
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t10 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR10
+; CHECK-ERROR10: LLVM ERROR: invalid profile {{.*}} at line 3: unable to parse basic block id: '1.1.1'
+; RUN: echo 'v1' > %t11
+; RUN: echo 'f dummy1' >> %t11
+; RUN: echo 'c 0 1.a' >> %t11
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t11 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR11
+; CHECK-ERROR11: LLVM ERROR: invalid profile {{.*}} at line 3: unable to parse clone id: 'a'
+; RUN: echo 'v1' > %t12
+; RUN: echo 'f dummy1' >> %t12
+; RUN: echo 'c 0 1' >> %t12
+; RUN: echo 'p 1 2.1' >> %t12
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t12 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR12
+; CHECK-ERROR12: LLVM ERROR: invalid profile {{.*}} at line 4: unsigned integer expected: '2.1'
+; RUN: echo 'v1' > %t13
+; RUN: echo 'f dummy1' >> %t13
+; RUN: echo 'c 0 1' >> %t13
+; RUN: echo 'p 1 2 3 2' >> %t13
+; RUN: not --crash llc < %s -O0 -mtriple=x86_64 -function-sections -basic-block-sections=%t13 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR13
+; CHECK-ERROR13: LLVM ERROR: invalid profile {{.*}} at line 4: duplicate cloned block in path: '2'
define i32 @dummy1(i32 %x, i32 %y, i32 %z) {
entry:
@@ -63,4 +85,3 @@ define i32 @dummy2(i32 %x, i32 %y, i32 %z) !dbg !4 {
!3 = !{i32 2, !"Debug Info Version", i32 3}
!4 = distinct !DISubprogram(name: "dummy1", scope: !1, unit: !0)
-
>From 53362597b3b47aceb920e52f5da79866c631243d Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Sat, 30 Sep 2023 00:03:45 +0000
Subject: [PATCH 2/3] Show ascii art of the CFG for cloning.
---
.../CodeGen/BasicBlockSectionsProfileReader.h | 63 ++++++++-------
llvm/lib/CodeGen/BasicBlockSections.cpp | 40 +++++-----
.../BasicBlockSectionsProfileReader.cpp | 79 ++++++++++++-------
3 files changed, 107 insertions(+), 75 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index f04eab8172253d9..61d06f19e618cc2 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -29,12 +29,39 @@
namespace llvm {
// This structure represents a unique ID for every block specified in the
-// profile.
+// input profile.
struct ProfileBBID {
+ // Basic block id associated with `MachineBasicBlock::BBID`.
unsigned BBID;
+ // The clone id associated with the block. This is zero for the original
+ // block. For the cloned ones, it is equal to 1 + index of the associated
+ // path in `FunctionPathAndClusterInfo::ClonePaths`.
unsigned CloneID;
};
+// This struct represents the cluster information for a machine basic block,
+// which is specifed by a unique ID. This templated struct is used for both the
+// raw input profile (as `BBClusterInfo<ProfileBBID>`) and the processed profile
+// after applying the clonings (as `BBClusterInfo<unsigned>`).
+template <typename BBIDType> struct BBClusterInfo {
+ // Basic block ID.
+ BBIDType BasicBlockID;
+ // Cluster ID this basic block belongs to.
+ unsigned ClusterID;
+ // Position of basic block within the cluster.
+ unsigned PositionInCluster;
+};
+
+// This represents the raw input profile for one function.
+struct FunctionPathAndClusterInfo {
+ // BB Cluster information specified by `ProfileBBID`s (before cloning).
+ SmallVector<BBClusterInfo<ProfileBBID>> ClusterInfo;
+ // Paths to clone. A path a -> b -> c -> d implies cloning b, c, and d along
+ // the edge a -> b (a is not cloned). The index of the path in this vector
+ // determines the `ProfileBBID::CloneID` of the cloned blocks in that path.
+ SmallVector<SmallVector<unsigned>> ClonePaths;
+};
+
// Provides DenseMapInfo for ProfileBBID.
template <> struct DenseMapInfo<ProfileBBID> {
static inline ProfileBBID getEmptyKey() {
@@ -56,26 +83,6 @@ template <> struct DenseMapInfo<ProfileBBID> {
}
};
-// This struct represents the cluster information for a machine basic block,
-// which is specifed by a unique ID.
-template <typename BBIDType> struct BBProfile {
- // Basic block ID.
- BBIDType BasicBlockID;
- // Cluster ID this basic block belongs to.
- unsigned ClusterID;
- // Position of basic block within the cluster.
- unsigned PositionInCluster;
-};
-
-// This represents the profile for one function.
-struct RawFunctionProfile {
- // BB Cluster information specified by `ProfileBBID`s (before cloning).
- SmallVector<BBProfile<ProfileBBID>> RawBBProfiles;
- // Paths to clone. A path a -> b -> c -> d implies cloning b, c, and d along
- // the edge a -> b.
- SmallVector<SmallVector<unsigned>> ClonePaths;
-};
-
class BasicBlockSectionsProfileReader : public ImmutablePass {
public:
static char ID;
@@ -106,11 +113,11 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
// 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, RawFunctionProfile>
- getRawProfileForFunction(StringRef FuncName) const;
+ std::pair<bool, FunctionPathAndClusterInfo>
+ getPathAndClusterInfoForFunction(StringRef FuncName) const;
// Initializes the FunctionNameToDIFilename map for the current module and
- // then reads the profile for matching functions.
+ // then reads the profile for the matching functions.
bool doInitialization(Module &M) override;
private:
@@ -127,7 +134,7 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
inconvertibleErrorCode());
}
- // Parses a `ProfileBBID` from `S`.
+ // Parses a `ProfileBBID` from `S`. `S` should be in the form "<bbid>" (representing an original block) or "<bbid>.<cloneid>" (representing a cloned block) where bbid is a non-negative integer and cloneid is a positive integer.
Expected<ProfileBBID> parseProfileBBID(StringRef S) const;
// Reads the basic block sections profile for functions in this module.
@@ -150,16 +157,16 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
// empty string if no debug info is available.
StringMap<SmallString<128>> FunctionNameToDIFilename;
- // This encapsulates the BB cluster information for the whole program.
+ // This contains the BB cluster information for the whole program.
//
// For every function name, it contains the cloning and 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.
- StringMap<RawFunctionProfile> RawProgramProfile;
+ StringMap<FunctionPathAndClusterInfo> ProgramPathAndClusterInfo;
// Some functions have alias names. We use this map to find the main alias
- // name for which we have mapping in ProgramBBClusterInfo.
+ // name which appears in ProgramPathAndClusterInfo as a key.
StringMap<StringRef> FuncAliasMap;
};
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index 5e61a96c696aff0..70bee84e65cbf33 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -175,12 +175,12 @@ updateBranches(MachineFunction &MF,
// 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.
-// BBProfilesByBBID represents the cluster information for basic blocks. It
+// ClusterInfoByBBID represents the cluster information for basic blocks. It
// maps from BBID of basic blocks to their cluster information. If this is
// empty, it means unique sections for all basic blocks in the function.
static void assignSections(
MachineFunction &MF,
- const DenseMap<unsigned, BBProfile<unsigned>> &BBProfilesByBBID) {
+ const DenseMap<unsigned, BBClusterInfo<unsigned>> &ClusterInfoByBBID) {
assert(MF.hasBBSections() && "BB Sections is not set for function.");
// This variable stores the section ID of the cluster containing eh_pads (if
// all eh_pads are one cluster). If more than one cluster contain eh_pads, we
@@ -191,17 +191,17 @@ static void assignSections(
// With the 'all' option, every basic block is placed in a unique section.
// With the 'list' option, every basic block is placed in a section
// associated with its cluster, unless we want individual unique sections
- // for every basic block in this function (if BBProfilesByBBID is empty).
+ // for every basic block in this function (if ClusterInfoByBBID is empty).
if (MF.getTarget().getBBSectionsType() == llvm::BasicBlockSection::All ||
- BBProfilesByBBID.empty()) {
+ ClusterInfoByBBID.empty()) {
// If unique sections are desired for all basic blocks of the function, we
// set every basic block's section ID equal to its original position in
// the layout (which is equal to its number). This ensures that basic
// blocks are ordered canonically.
MBB.setSectionID(MBB.getNumber());
} else {
- auto I = BBProfilesByBBID.find(*MBB.getBBID());
- if (I != BBProfilesByBBID.end()) {
+ auto I = ClusterInfoByBBID.find(*MBB.getBBID());
+ if (I != ClusterInfoByBBID.end()) {
MBB.setSectionID(I->second.ClusterID);
} else {
// BB goes into the special cold section if it is not specified in the
@@ -308,25 +308,27 @@ bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
return true;
}
- DenseMap<unsigned, BBProfile<unsigned>> BBProfilesByBBID;
+ DenseMap<unsigned, BBClusterInfo<unsigned>> ClusterInfoByBBID;
if (BBSectionsType == BasicBlockSection::List) {
- auto [HasProfile, RawProfile] =
- getAnalysis<BasicBlockSectionsProfileReader>().getRawProfileForFunction(
+ auto [HasProfile, PathAndClusterInfo] =
+ getAnalysis<BasicBlockSectionsProfileReader>().getPathAndClusterInfoForFunction(
MF.getName());
if (!HasProfile)
return true;
- // TODO: Apply the path cloning profile.
- for (const BBProfile<ProfileBBID> &BBP : RawProfile.RawBBProfiles) {
- assert(!BBP.BasicBlockID.CloneID && "Path cloning is not supported yet.");
- BBProfilesByBBID.try_emplace(BBP.BasicBlockID.BBID,
- BBProfile<unsigned>{BBP.BasicBlockID.BBID,
- BBP.ClusterID,
- BBP.PositionInCluster});
+ for (const BBClusterInfo<ProfileBBID> &BBP : PathAndClusterInfo.ClusterInfo) {
+ // TODO: Apply the path cloning profile.
+ assert(!BBP.BasicBlockID.CloneID && "Path cloning is not supported yet");
+ const auto [I, Inserted] = ClusterInfoByBBID.try_emplace(
+ BBP.BasicBlockID.BBID,
+ BBClusterInfo<unsigned>{BBP.BasicBlockID.BBID, BBP.ClusterID,
+ BBP.PositionInCluster});
+ (void)I;
+ assert(Inserted && "Duplicate BBID found in profile");
}
}
MF.setBBSectionsType(BBSectionsType);
- assignSections(MF, BBProfilesByBBID);
+ assignSections(MF, ClusterInfoByBBID);
// We make sure that the cluster including the entry basic block precedes all
// other clusters.
@@ -360,8 +362,8 @@ bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
// If the two basic block are in the same section, the order is decided by
// their position within the section.
if (XSectionID.Type == MBBSectionID::SectionType::Default)
- return BBProfilesByBBID.lookup(*X.getBBID()).PositionInCluster <
- BBProfilesByBBID.lookup(*Y.getBBID()).PositionInCluster;
+ return ClusterInfoByBBID.lookup(*X.getBBID()).PositionInCluster <
+ ClusterInfoByBBID.lookup(*Y.getBBID()).PositionInCluster;
return X.getNumber() < Y.getNumber();
};
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index eb5cbda696a7de3..0b48d2ab0e00a67 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -56,15 +56,15 @@ BasicBlockSectionsProfileReader::parseProfileBBID(StringRef S) const {
}
bool BasicBlockSectionsProfileReader::isFunctionHot(StringRef FuncName) const {
- return getRawProfileForFunction(FuncName).first;
+ return getPathAndClusterInfoForFunction(FuncName).first;
}
-std::pair<bool, RawFunctionProfile>
-BasicBlockSectionsProfileReader::getRawProfileForFunction(
+std::pair<bool, FunctionPathAndClusterInfo>
+BasicBlockSectionsProfileReader::getPathAndClusterInfoForFunction(
StringRef FuncName) const {
- auto R = RawProgramProfile.find(getAliasName(FuncName));
- return R != RawProgramProfile.end() ? std::pair(true, R->second)
- : std::pair(false, RawFunctionProfile());
+ auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
+ return R != ProgramPathAndClusterInfo.end() ? std::pair(true, R->second)
+ : std::pair(false, FunctionPathAndClusterInfo());
}
// Reads the version 1 basic block sections profile. Profile for each function
@@ -85,20 +85,42 @@ BasicBlockSectionsProfileReader::getRawProfileForFunction(
// clone basic blocks along a path. The cloned blocks are then specified in the
// cluster information.
// The following profile lists two cloning paths (starting with 'p') for
-// function bar and places the total 11 blocks within two clusters. Each cloned
-// block is identified by its original block id, along with its clone id. A
-// block cloned multiple times (2 in this example) appears with distinct clone
-// ids (2.1 and 2.2).
-// ---------------------------
+// function bar and places the total 9 blocks within two clusters. The blocks in each path are cloned along the path from the first block (the first block is not cloned).
+// For instance, path 1 (1 -> 3 -> 4) specifies that 3 and 4 must be cloned along the edge 1->3. In the clusters, each cloned
+// block is identified by its original block id, along with its clone id. For instance, the cloned blocks from path 1 are represented by 3.1 and 4.1.
+// A block cloned multiple times appears with distinct clone ids. The CFG for bar
+// is shown below before and after cloning with its final clusters labeled.
//
// f main
// f bar
-// p 1 2 3
-// p 4 2 5
-// c 2 3 5 6 7
-// c 1 2.1 3.1 4 2.2 5.1
+// p 1 3 4 # cloning path 1
+// p 4 2 # cloning path 2
+// c 1 3.1 4.1 6 # basic block cluster 1
+// c 0 2 3 4 2.1 5 # basic block cluster 2
+// ****************************************************************************
+// function bar before and after cloning with basic block clusters shown.
+// ****************************************************************************
+// .... ..............
+// 0 -------+ : 0 :---->: 1 ---> 3.1 :
+// | | : | : :........ | :
+// v v : v : : v :
+// +--> 2 --> 5 1 ~~~~~~> +---: 2 : : 4.1: clsuter 1
+// | | | | : | : : | :
+// | v | | : v ....... : v :
+// | 3 <------+ | : 3 <--+ : : 6 :
+// | | | : | | : :....:
+// | v | : v | :
+// +--- 4 ---> 6 | : 4 | :
+// | : | | :
+// | : v | :
+// | :2.1---+ : cluster 2
+// | : | ......:
+// | : v :
+// +-->: 5 :
+// ....
+// ****************************************************************************
Error BasicBlockSectionsProfileReader::ReadV1Profile() {
- auto FI = RawProgramProfile.end();
+ auto FI = ProgramPathAndClusterInfo.end();
// Current cluster ID corresponding to this function.
unsigned CurrentCluster = 0;
@@ -121,7 +143,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
S.split(Values, ' ');
switch (Specifier) {
case '@':
- break;
+ continue;
case 'm': // Module name speicifer.
if (Values.size() != 1) {
return createProfileParseError(Twine("invalid module name value: '") +
@@ -142,7 +164,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
if (!FunctionFound) {
// Skip the following profile by setting the profile iterator (FI) to
// the past-the-end element.
- FI = RawProgramProfile.end();
+ FI = ProgramPathAndClusterInfo.end();
DIFilename = "";
continue;
}
@@ -151,7 +173,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
// Prepare for parsing clusters of this function name.
// Start a new cluster map for this function name.
- auto R = RawProgramProfile.try_emplace(Values.front());
+ auto R = ProgramPathAndClusterInfo.try_emplace(Values.front());
// Report error when multiple profiles have been specified for the same
// function.
if (!R.second)
@@ -168,8 +190,8 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
case 'c': // Basic block cluster specifier.
// Skip the profile when we the profile iterator (FI) refers to the
// past-the-end element.
- if (FI == RawProgramProfile.end())
- break;
+ if (FI == ProgramPathAndClusterInfo.end())
+ continue;
// Reset current cluster position.
CurrentPosition = 0;
for (auto BasicBlockIDStr : Values) {
@@ -185,7 +207,7 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
return createProfileParseError(
"entry BB (0) does not begin a cluster.");
- FI->second.RawBBProfiles.emplace_back(BBProfile<ProfileBBID>{
+ FI->second.ClusterInfo.emplace_back(BBClusterInfo<ProfileBBID>{
*std::move(BasicBlockID), CurrentCluster, CurrentPosition++});
}
CurrentCluster++;
@@ -210,12 +232,13 @@ Error BasicBlockSectionsProfileReader::ReadV1Profile() {
return createProfileParseError(Twine("invalid specifier: '") +
Twine(Specifier) + "'");
}
+ llvm_unreachable("should not break from this switch statement");
}
return Error::success();
}
Error BasicBlockSectionsProfileReader::ReadV0Profile() {
- auto FI = RawProgramProfile.end();
+ auto FI = ProgramPathAndClusterInfo.end();
// Current cluster ID corresponding to this function.
unsigned CurrentCluster = 0;
// Current position in the current cluster.
@@ -236,7 +259,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
if (S.consume_front("!")) {
// Skip the profile when we the profile iterator (FI) refers to the
// past-the-end element.
- if (FI == RawProgramProfile.end())
+ if (FI == ProgramPathAndClusterInfo.end())
continue;
SmallVector<StringRef, 4> BBIDs;
S.split(BBIDs, ' ');
@@ -254,8 +277,8 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
return createProfileParseError(
"entry BB (0) does not begin a cluster");
- FI->second.RawBBProfiles.emplace_back(
- BBProfile<ProfileBBID>({{static_cast<unsigned>(BBID), 0},
+ FI->second.ClusterInfo.emplace_back(
+ BBClusterInfo<ProfileBBID>({{static_cast<unsigned>(BBID), 0},
CurrentCluster,
CurrentPosition++}));
}
@@ -291,7 +314,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
if (!FunctionFound) {
// Skip the following profile by setting the profile iterator (FI) to
// the past-the-end element.
- FI = RawProgramProfile.end();
+ FI = ProgramPathAndClusterInfo.end();
continue;
}
for (size_t i = 1; i < Aliases.size(); ++i)
@@ -299,7 +322,7 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
// Prepare for parsing clusters of this function name.
// Start a new cluster map for this function name.
- auto R = RawProgramProfile.try_emplace(Aliases.front());
+ auto R = ProgramPathAndClusterInfo.try_emplace(Aliases.front());
// Report error when multiple profiles have been specified for the same
// function.
if (!R.second)
>From e9afb2dc1357cba30e08ec0b724e4acac8dab6fa Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Wed, 11 Oct 2023 23:12:27 +0000
Subject: [PATCH 3/3] Address review comments.
Changing variable names and comments.
---
.../CodeGen/BasicBlockSectionsProfileReader.h | 5 ++++-
llvm/lib/CodeGen/BasicBlockSections.cpp | 9 +++++----
.../BasicBlockSectionsProfileReader.cpp | 20 +++++++++++--------
3 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index 61d06f19e618cc2..6e01dfd11ee6dad 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -134,7 +134,10 @@ class BasicBlockSectionsProfileReader : public ImmutablePass {
inconvertibleErrorCode());
}
- // Parses a `ProfileBBID` from `S`. `S` should be in the form "<bbid>" (representing an original block) or "<bbid>.<cloneid>" (representing a cloned block) where bbid is a non-negative integer and cloneid is a positive integer.
+ // Parses a `ProfileBBID` from `S`. `S` must be in the form "<bbid>"
+ // (representing an original block) or "<bbid>.<cloneid>" (representing a
+ // cloned block) where bbid is a non-negative integer and cloneid is a
+ // positive integer.
Expected<ProfileBBID> parseProfileBBID(StringRef S) const;
// Reads the basic block sections profile for functions in this module.
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index 70bee84e65cbf33..632fd68d88b5c64 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -311,17 +311,18 @@ bool BasicBlockSections::runOnMachineFunction(MachineFunction &MF) {
DenseMap<unsigned, BBClusterInfo<unsigned>> ClusterInfoByBBID;
if (BBSectionsType == BasicBlockSection::List) {
auto [HasProfile, PathAndClusterInfo] =
- getAnalysis<BasicBlockSectionsProfileReader>().getPathAndClusterInfoForFunction(
- MF.getName());
+ getAnalysis<BasicBlockSectionsProfileReader>()
+ .getPathAndClusterInfoForFunction(MF.getName());
if (!HasProfile)
return true;
- for (const BBClusterInfo<ProfileBBID> &BBP : PathAndClusterInfo.ClusterInfo) {
+ for (const BBClusterInfo<ProfileBBID> &BBP :
+ PathAndClusterInfo.ClusterInfo) {
// TODO: Apply the path cloning profile.
assert(!BBP.BasicBlockID.CloneID && "Path cloning is not supported yet");
const auto [I, Inserted] = ClusterInfoByBBID.try_emplace(
BBP.BasicBlockID.BBID,
BBClusterInfo<unsigned>{BBP.BasicBlockID.BBID, BBP.ClusterID,
- BBP.PositionInCluster});
+ BBP.PositionInCluster});
(void)I;
assert(Inserted && "Duplicate BBID found in profile");
}
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index 0b48d2ab0e00a67..6bb412a6c7534a6 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -63,8 +63,9 @@ std::pair<bool, FunctionPathAndClusterInfo>
BasicBlockSectionsProfileReader::getPathAndClusterInfoForFunction(
StringRef FuncName) const {
auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
- return R != ProgramPathAndClusterInfo.end() ? std::pair(true, R->second)
- : std::pair(false, FunctionPathAndClusterInfo());
+ return R != ProgramPathAndClusterInfo.end()
+ ? std::pair(true, R->second)
+ : std::pair(false, FunctionPathAndClusterInfo());
}
// Reads the version 1 basic block sections profile. Profile for each function
@@ -85,10 +86,13 @@ BasicBlockSectionsProfileReader::getPathAndClusterInfoForFunction(
// clone basic blocks along a path. The cloned blocks are then specified in the
// cluster information.
// The following profile lists two cloning paths (starting with 'p') for
-// function bar and places the total 9 blocks within two clusters. The blocks in each path are cloned along the path from the first block (the first block is not cloned).
-// For instance, path 1 (1 -> 3 -> 4) specifies that 3 and 4 must be cloned along the edge 1->3. In the clusters, each cloned
-// block is identified by its original block id, along with its clone id. For instance, the cloned blocks from path 1 are represented by 3.1 and 4.1.
-// A block cloned multiple times appears with distinct clone ids. The CFG for bar
+// function bar and places the total 9 blocks within two clusters. The first two
+// blocks of a cloning path specify the edge along which the path is cloned. For
+// instance, path 1 (1 -> 3 -> 4) instructs that 3 and 4 must be cloned along
+// the edge 1->3. Within the given clusters, each cloned block is identified by
+// "<original block id>.<clone id>". For instance, 3.1 represents the first
+// clone of block 3. Original blocks are specified just with their block ids. A
+// block cloned multiple times appears with distinct clone ids. The CFG for bar
// is shown below before and after cloning with its final clusters labeled.
//
// f main
@@ -279,8 +283,8 @@ Error BasicBlockSectionsProfileReader::ReadV0Profile() {
FI->second.ClusterInfo.emplace_back(
BBClusterInfo<ProfileBBID>({{static_cast<unsigned>(BBID), 0},
- CurrentCluster,
- CurrentPosition++}));
+ CurrentCluster,
+ CurrentPosition++}));
}
CurrentCluster++;
} else {
More information about the llvm-commits
mailing list