From 6cf97d01d96034581ed16ba7b4525ee698990b91 Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Mon, 10 Nov 2025 17:27:58 +0000 Subject: [PATCH 1/6] 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> + // Returns the cluster info for the function \p FuncName. Returns an empty + // vector if function has no cluster info. + SmallVector 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> + SmallVector getClusterInfoForFunction(StringRef FuncName) const; SmallVector> 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 FuncClusterInfo; if (BBSectionsType == BasicBlockSection::List) { - auto [HasProfile, ClusterInfo] = + auto ClusterInfo = getAnalysis() .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> +SmallVector 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()); + return R != ProgramPathAndClusterInfo.end() ? R->second.ClusterInfo : SmallVector(); } SmallVector> 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>(); } uint64_t BasicBlockSectionsProfileReader::getEdgeCount( @@ -494,7 +493,7 @@ bool BasicBlockSectionsProfileReaderWrapperPass::isFunctionHot( return BBSPR.isFunctionHot(FuncName); } -std::pair> +SmallVector BasicBlockSectionsProfileReaderWrapperPass::getClusterInfoForFunction( StringRef FuncName) const { return BBSPR.getClusterInfoForFunction(FuncName); From 777d6893ceadfbe905c3a8691853834852481e0a Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Mon, 10 Nov 2025 17:28:12 +0000 Subject: [PATCH 2/6] 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 FuncClusterInfo; if (BBSectionsType == BasicBlockSection::List) { - auto ClusterInfo = - getAnalysis() - .getClusterInfoForFunction(MF.getName()); + auto ClusterInfo = getAnalysis() + .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 BasicBlockSectionsProfileReader::getClusterInfoForFunction( StringRef FuncName) const { auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName)); - return R != ProgramPathAndClusterInfo.end() ? R->second.ClusterInfo : SmallVector(); + return R != ProgramPathAndClusterInfo.end() ? R->second.ClusterInfo + : SmallVector(); } SmallVector> BasicBlockSectionsProfileReader::getClonePathsForFunction( StringRef FuncName) const { auto R = ProgramPathAndClusterInfo.find(getAliasName(FuncName)); - return R != ProgramPathAndClusterInfo.end() ? R->second.ClonePaths : SmallVector>(); + return R != ProgramPathAndClusterInfo.end() + ? R->second.ClonePaths + : SmallVector>(); } uint64_t BasicBlockSectionsProfileReader::getEdgeCount( From efbb80216de1cb8c09ca928e2743b8adc6f8f88c Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Mon, 10 Nov 2025 17:31:58 +0000 Subject: [PATCH 3/6] 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",@progbits -; LINUX-SECTIONS: .section .text.hot._Z3foob,"ax",@progbits -; LINUX-SECTIONS: _Z3foob: -; LINUX-SECTIONS: .section .text.hot._Z3foob._Z3foob.__part.1,"ax",@progbits -; LINUX-SECTIONS: _Z3foob.__part.1: -; LINUX-SECTIONS: .section .text.hot._Z3foob._Z3foob.__part.2,"ax",@progbits -; LINUX-SECTIONS: _Z3foob.__part.2: -; 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-SECTIONS-NO-FUNCTION-SECTION-NOT: .section .text{{.*}}._Z3zipb,"ax",@progbits -; LINUX-SECTIONS: _Z3zipb: -; LINUX-SECTIONS-NOT: .section .text{{.*}}._Z3zipb.__part.{{[0-9]+}},"ax",@progbits -; LINUX-SECTIONS-NOT: _Z3zipb.__part.{{[0-9]+}}: From adac2e6cd39abf740ec76c3291a17d4173e7cc1c Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Mon, 10 Nov 2025 17:55:00 +0000 Subject: [PATCH 4/6] 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 &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 FuncClusterInfo; if (BBSectionsType == BasicBlockSection::List) { auto ClusterInfo = getAnalysis() .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 8af4ed537741acf4a5dc1126a62c034e4f661151 Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Mon, 10 Nov 2025 17:58:47 +0000 Subject: [PATCH 5/6] 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() .getClusterInfoForFunction(MF.getName()); - if (ClusterInfo.empty()) return false; + if (ClusterInfo.empty()) + return false; for (auto &BBClusterInfo : ClusterInfo) { FuncClusterInfo.try_emplace(BBClusterInfo.BBID, BBClusterInfo); } From 564c38ef8dfff9bba689f5a564b66b2f895a6476 Mon Sep 17 00:00:00 2001 From: Rahman Lavaee Date: Mon, 10 Nov 2025 21:36:37 +0000 Subject: [PATCH 6/6] Fix clang test. --- clang/test/CodeGen/Inputs/basic-block-sections.funcnames | 4 +++- clang/test/CodeGen/basic-block-sections.c | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/clang/test/CodeGen/Inputs/basic-block-sections.funcnames b/clang/test/CodeGen/Inputs/basic-block-sections.funcnames index 329cea9a0adfb..2452ee345fe2f 100644 --- a/clang/test/CodeGen/Inputs/basic-block-sections.funcnames +++ b/clang/test/CodeGen/Inputs/basic-block-sections.funcnames @@ -1 +1,3 @@ -!world +v1 +f world +c 0 diff --git a/clang/test/CodeGen/basic-block-sections.c b/clang/test/CodeGen/basic-block-sections.c index a61b8dd4ac376..0c21a4cb1442c 100644 --- a/clang/test/CodeGen/basic-block-sections.c +++ b/clang/test/CodeGen/basic-block-sections.c @@ -30,8 +30,10 @@ int another(int a) { // // BB_WORLD: .section .text.world,"ax",@progbits{{$}} // BB_WORLD: world: -// BB_WORLD: .section .text.world,"ax",@progbits,unique -// BB_WORLD: world.__part.1: +// BB_ALL: .section .text.world,"ax",@progbits,unique +// BB_ALL: world.__part.1: +// BB_LIST: .section .text.split.world,"ax",@progbits +// BB_LIST: world.cold: // BB_ALL: .section .text.another,"ax",@progbits // BB_ALL: another.__part.1: // BB_LIST-NOT: .section .text.another,"ax",@progbits