[llvm] Treat specifying a function in the bbsection profile without any directive as noop. (PR #167359)
Rahman Lavaee via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 10 12:46:54 PST 2025
https://github.com/rlavaee updated https://github.com/llvm/llvm-project/pull/167359
>From 1dd840d8520e320bd164d8b94d5a5782a2172533 Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Mon, 10 Nov 2025 17:27:58 +0000
Subject: [PATCH 1/5] Make specifying the function in bbsection profile a noop,
when no other directives are specified.
---
.../CodeGen/BasicBlockSectionsProfileReader.h | 16 ++++++----------
llvm/lib/CodeGen/BasicBlockSections.cpp | 4 +---
.../CodeGen/BasicBlockSectionsProfileReader.cpp | 13 ++++++-------
3 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
index 7b1a5f5019589..ee1f28377f7e4 100644
--- a/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
+++ b/llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
@@ -68,17 +68,13 @@ class BasicBlockSectionsProfileReader {
BasicBlockSectionsProfileReader() = default;
- // Returns true if basic block sections profile exist for function \p
- // FuncName.
+ // Returns true if function \p FuncName is hot based on the basic block
+ // section profile.
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>>
+ // Returns the cluster info for the function \p FuncName. Returns an empty
+ // vector if function has no cluster info.
+ SmallVector<BBClusterInfo>
getClusterInfoForFunction(StringRef FuncName) const;
// Returns the path clonings for the given function.
@@ -190,7 +186,7 @@ class BasicBlockSectionsProfileReaderWrapperPass : public ImmutablePass {
bool isFunctionHot(StringRef FuncName) const;
- std::pair<bool, SmallVector<BBClusterInfo>>
+ SmallVector<BBClusterInfo>
getClusterInfoForFunction(StringRef FuncName) const;
SmallVector<SmallVector<unsigned>>
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index e317e1c06741f..fdbf11d0444e2 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -314,11 +314,9 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
DenseMap<UniqueBBID, BBClusterInfo> FuncClusterInfo;
if (BBSectionsType == BasicBlockSection::List) {
- auto [HasProfile, ClusterInfo] =
+ auto ClusterInfo =
getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
.getClusterInfoForFunction(MF.getName());
- if (!HasProfile)
- return false;
for (auto &BBClusterInfo : ClusterInfo) {
FuncClusterInfo.try_emplace(BBClusterInfo.BBID, BBClusterInfo);
}
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index 485b44ae4c4aa..5a943f8e12303 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -58,22 +58,21 @@ BasicBlockSectionsProfileReader::parseUniqueBBID(StringRef S) const {
}
bool BasicBlockSectionsProfileReader::isFunctionHot(StringRef FuncName) const {
- return getClusterInfoForFunction(FuncName).first;
+ return !getClusterInfoForFunction(FuncName).empty();
}
-std::pair<bool, SmallVector<BBClusterInfo>>
+SmallVector<BBClusterInfo>
BasicBlockSectionsProfileReader::getClusterInfoForFunction(
StringRef FuncName) const {
auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
- return R != ProgramPathAndClusterInfo.end()
- ? std::pair(true, R->second.ClusterInfo)
- : std::pair(false, SmallVector<BBClusterInfo>());
+ return R != ProgramPathAndClusterInfo.end() ? R->second.ClusterInfo : SmallVector<BBClusterInfo>();
}
SmallVector<SmallVector<unsigned>>
BasicBlockSectionsProfileReader::getClonePathsForFunction(
StringRef FuncName) const {
- return ProgramPathAndClusterInfo.lookup(getAliasName(FuncName)).ClonePaths;
+ auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
+ return R != ProgramPathAndClusterInfo.end() ? R->second.ClonePaths : SmallVector<SmallVector<unsigned>>();
}
uint64_t BasicBlockSectionsProfileReader::getEdgeCount(
@@ -494,7 +493,7 @@ bool BasicBlockSectionsProfileReaderWrapperPass::isFunctionHot(
return BBSPR.isFunctionHot(FuncName);
}
-std::pair<bool, SmallVector<BBClusterInfo>>
+SmallVector<BBClusterInfo>
BasicBlockSectionsProfileReaderWrapperPass::getClusterInfoForFunction(
StringRef FuncName) const {
return BBSPR.getClusterInfoForFunction(FuncName);
>From b741e4b8bc09e4b10cdc8fb94663e8d28b9ec33f Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Mon, 10 Nov 2025 17:28:12 +0000
Subject: [PATCH 2/5] clang-format.
---
llvm/lib/CodeGen/BasicBlockSections.cpp | 5 ++---
llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp | 7 +++++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index fdbf11d0444e2..d6210d1d729c1 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -314,9 +314,8 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
DenseMap<UniqueBBID, BBClusterInfo> FuncClusterInfo;
if (BBSectionsType == BasicBlockSection::List) {
- auto ClusterInfo =
- getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
- .getClusterInfoForFunction(MF.getName());
+ auto ClusterInfo = getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
+ .getClusterInfoForFunction(MF.getName());
for (auto &BBClusterInfo : ClusterInfo) {
FuncClusterInfo.try_emplace(BBClusterInfo.BBID, BBClusterInfo);
}
diff --git a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
index 5a943f8e12303..c234c0f1b0b34 100644
--- a/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
@@ -65,14 +65,17 @@ SmallVector<BBClusterInfo>
BasicBlockSectionsProfileReader::getClusterInfoForFunction(
StringRef FuncName) const {
auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
- return R != ProgramPathAndClusterInfo.end() ? R->second.ClusterInfo : SmallVector<BBClusterInfo>();
+ return R != ProgramPathAndClusterInfo.end() ? R->second.ClusterInfo
+ : SmallVector<BBClusterInfo>();
}
SmallVector<SmallVector<unsigned>>
BasicBlockSectionsProfileReader::getClonePathsForFunction(
StringRef FuncName) const {
auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName));
- return R != ProgramPathAndClusterInfo.end() ? R->second.ClonePaths : SmallVector<SmallVector<unsigned>>();
+ return R != ProgramPathAndClusterInfo.end()
+ ? R->second.ClonePaths
+ : SmallVector<SmallVector<unsigned>>();
}
uint64_t BasicBlockSectionsProfileReader::getEdgeCount(
>From 6a0c665b75a3e68b58e5019c23345bbc2572afe2 Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Mon, 10 Nov 2025 17:31:58 +0000
Subject: [PATCH 3/5] Change the test.
---
.../CodeGen/X86/basic-block-sections-list.ll | 62 +++----------------
1 file changed, 8 insertions(+), 54 deletions(-)
diff --git a/llvm/test/CodeGen/X86/basic-block-sections-list.ll b/llvm/test/CodeGen/X86/basic-block-sections-list.ll
index 45ef452f4f5c1..d652a540f3e9c 100644
--- a/llvm/test/CodeGen/X86/basic-block-sections-list.ll
+++ b/llvm/test/CodeGen/X86/basic-block-sections-list.ll
@@ -1,17 +1,13 @@
-;; Check the basic block sections list option.
-;; version 0 profile:
-; RUN: echo '!_Z3foob' > %t1
+;; Check that specifying the function in the basic block sections profile
+;; without any other directives is a noop.
;;
-;; version 1 profile:
-; RUN: echo 'v1' > %t2
-; RUN: echo 'f _Z3foob' >> %t2
+;; Specify the bb sections profile:
+; RUN: echo 'v1' > %t
+; RUN: echo 'f _Z3foob' >> %t
;;
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t1 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-FUNCTION-SECTION
-; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t1 -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=%t1 -unique-basic-block-section-names --bbsections-guided-section-prefix=false | FileCheck %s -check-prefix=LINUX-SECTIONS-NO-GUIDED-PREFIX
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t2 -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS --check-prefix=LINUX-SECTIONS-FUNCTION-SECTION
-; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t2 -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=%t2 -unique-basic-block-section-names --bbsections-guided-section-prefix=false | FileCheck %s -check-prefix=LINUX-SECTIONS-NO-GUIDED-PREFIX
+; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=%t > %bbsections
+; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections > %orig
+; RUN: diff -u %orig %bbsections
define i32 @_Z3foob(i1 zeroext %0) nounwind {
%2 = alloca i32, align 4
@@ -41,45 +37,3 @@ define i32 @_Z3foob(i1 zeroext %0) nounwind {
declare i32 @_Z3barv() #1
declare i32 @_Z3bazv() #1
-
-define i32 @_Z3zipb(i1 zeroext %0) nounwind {
- %2 = alloca i32, align 4
- %3 = alloca i8, align 1
- %4 = zext i1 %0 to i8
- store i8 %4, ptr %3, align 1
- %5 = load i8, ptr %3, align 1
- %6 = trunc i8 %5 to i1
- %7 = zext i1 %6 to i32
- %8 = icmp sgt i32 %7, 0
- br i1 %8, label %9, label %11
-
-9: ; preds = %1
- %10 = call i32 @_Z3barv()
- store i32 %10, ptr %2, align 4
- br label %13
-
-11: ; preds = %1
- %12 = call i32 @_Z3bazv()
- store i32 %12, ptr %2, align 4
- br label %13
-
-13: ; preds = %11, %9
- %14 = load i32, ptr %2, align 4
- ret i32 %14
-}
-
-; LINUX-SECTIONS-NO-GUIDED-PREFIX: .section .text._Z3foob,"ax", at progbits
-; LINUX-SECTIONS: .section .text.hot._Z3foob,"ax", at progbits
-; LINUX-SECTIONS: _Z3foob:
-; LINUX-SECTIONS: .section .text.hot._Z3foob._Z3foob.__part.1,"ax", at progbits
-; LINUX-SECTIONS: _Z3foob.__part.1:
-; LINUX-SECTIONS: .section .text.hot._Z3foob._Z3foob.__part.2,"ax", at progbits
-; LINUX-SECTIONS: _Z3foob.__part.2:
-; LINUX-SECTIONS: .section .text.hot._Z3foob._Z3foob.__part.3,"ax", at progbits
-; LINUX-SECTIONS: _Z3foob.__part.3:
-
-; LINUX-SECTIONS-FUNCTION-SECTION: .section .text._Z3zipb,"ax", at progbits
-; LINUX-SECTIONS-NO-FUNCTION-SECTION-NOT: .section .text{{.*}}._Z3zipb,"ax", at progbits
-; LINUX-SECTIONS: _Z3zipb:
-; LINUX-SECTIONS-NOT: .section .text{{.*}}._Z3zipb.__part.{{[0-9]+}},"ax", at progbits
-; LINUX-SECTIONS-NOT: _Z3zipb.__part.{{[0-9]+}}:
>From ae05930a513723bb19998135d9e545701e0f9a75 Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Mon, 10 Nov 2025 17:55:00 +0000
Subject: [PATCH 4/5] Fix the source drift test.
---
llvm/lib/CodeGen/BasicBlockSections.cpp | 17 ++++++++---------
.../X86/basic-block-sections-source-drift.ll | 8 +++++---
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index d6210d1d729c1..92a40ed995993 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -183,8 +183,7 @@ updateBranches(MachineFunction &MF,
// clusters are ordered in increasing order of their IDs, with the "Exception"
// and "Cold" succeeding all other clusters.
// FuncClusterInfo 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.
+// maps from BBID of basic blocks to their cluster information.
static void
assignSections(MachineFunction &MF,
const DenseMap<UniqueBBID, BBClusterInfo> &FuncClusterInfo) {
@@ -197,10 +196,8 @@ assignSections(MachineFunction &MF,
for (auto &MBB : 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 FuncClusterInfo is empty).
- if (MF.getTarget().getBBSectionsType() == llvm::BasicBlockSection::All ||
- FuncClusterInfo.empty()) {
+ // associated with its cluster.
+ if (MF.getTarget().getBBSectionsType() == llvm::BasicBlockSection::All) {
// 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
@@ -308,19 +305,21 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
if (BBSectionsType == BasicBlockSection::List &&
hasInstrProfHashMismatch(MF))
return false;
- // Renumber blocks before sorting them. This is useful for accessing the
- // original layout positions and finding the original fallthroughs.
- MF.RenumberBlocks();
DenseMap<UniqueBBID, BBClusterInfo> FuncClusterInfo;
if (BBSectionsType == BasicBlockSection::List) {
auto ClusterInfo = getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
.getClusterInfoForFunction(MF.getName());
+ if (ClusterInfo.empty()) return false;
for (auto &BBClusterInfo : ClusterInfo) {
FuncClusterInfo.try_emplace(BBClusterInfo.BBID, BBClusterInfo);
}
}
+ // Renumber blocks before sorting them. This is useful for accessing the
+ // original layout positions and finding the original fallthroughs.
+ MF.RenumberBlocks();
+
MF.setBBSectionsType(BBSectionsType);
assignSections(MF, FuncClusterInfo);
diff --git a/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll b/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll
index d481b147662dc..6e0db20ca0492 100644
--- a/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll
+++ b/llvm/test/CodeGen/X86/basic-block-sections-source-drift.ll
@@ -1,6 +1,8 @@
-; RUN: echo "!foo" > %t.order.txt
-; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t.order.txt | FileCheck --check-prefix=SOURCE-DRIFT %s
-; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t.order.txt -bbsections-detect-source-drift=false | FileCheck --check-prefix=HASH-CHECK-DISABLED %s
+; RUN: echo "v1" > %t
+; RUN: echo "f foo" >> %t
+; RUN: echo "c 0" >> %t
+; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t | FileCheck --check-prefix=SOURCE-DRIFT %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -basic-block-sections=%t -bbsections-detect-source-drift=false | FileCheck --check-prefix=HASH-CHECK-DISABLED %s
define dso_local i32 @foo(i1 zeroext %0, i1 zeroext %1) !annotation !1 {
br i1 %0, label %5, label %3
>From bef4e78b51be2cd1a85f5e327153c75cecf1e9c0 Mon Sep 17 00:00:00 2001
From: Rahman Lavaee <rahmanl at google.com>
Date: Mon, 10 Nov 2025 17:58:47 +0000
Subject: [PATCH 5/5] clang-format.
---
llvm/lib/CodeGen/BasicBlockSections.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/BasicBlockSections.cpp b/llvm/lib/CodeGen/BasicBlockSections.cpp
index 92a40ed995993..52e2909bec072 100644
--- a/llvm/lib/CodeGen/BasicBlockSections.cpp
+++ b/llvm/lib/CodeGen/BasicBlockSections.cpp
@@ -310,7 +310,8 @@ bool BasicBlockSections::handleBBSections(MachineFunction &MF) {
if (BBSectionsType == BasicBlockSection::List) {
auto ClusterInfo = getAnalysis<BasicBlockSectionsProfileReaderWrapperPass>()
.getClusterInfoForFunction(MF.getName());
- if (ClusterInfo.empty()) return false;
+ if (ClusterInfo.empty())
+ return false;
for (auto &BBClusterInfo : ClusterInfo) {
FuncClusterInfo.try_emplace(BBClusterInfo.BBID, BBClusterInfo);
}
More information about the llvm-commits
mailing list