From f9533772256fc288d1b43d2b700020fd77dde635 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Wed, 29 Jan 2025 16:10:23 -0800 Subject: [PATCH 1/2] [IR][AsmPrinter][ELF]Allow setting section prefix for and use the prefix when emitting global objects --- llvm/include/llvm/IR/Function.h | 6 ----- llvm/include/llvm/IR/GlobalObject.h | 6 +++++ llvm/include/llvm/IR/MDBuilder.h | 4 +-- .../CodeGen/TargetLoweringObjectFileImpl.cpp | 9 +++---- llvm/lib/IR/Function.cpp | 16 ----------- llvm/lib/IR/Globals.cpp | 19 +++++++++++++ llvm/lib/IR/MDBuilder.cpp | 6 ++--- llvm/test/CodeGen/X86/data-section-prefix.ll | 27 +++++++++++++++++++ .../CodeGenPrepare/X86/section-samplepgo.ll | 4 +-- .../Transforms/CodeGenPrepare/X86/section.ll | 4 +-- .../Transforms/HotColdSplit/coldentrycount.ll | 4 +-- .../section-accurate-samplepgo.ll | 6 ++--- 12 files changed, 70 insertions(+), 41 deletions(-) create mode 100644 llvm/test/CodeGen/X86/data-section-prefix.ll diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h index e7afcbd31420c..0ec3b132cc1aa 100644 --- a/llvm/include/llvm/IR/Function.h +++ b/llvm/include/llvm/IR/Function.h @@ -334,12 +334,6 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node { /// sample PGO, to enable the same inlines as the profiled optimized binary. DenseSet getImportGUIDs() const; - /// Set the section prefix for this function. - void setSectionPrefix(StringRef Prefix); - - /// Get the section prefix for this function. - std::optional getSectionPrefix() const; - /// hasGC/getGC/setGC/clearGC - The name of the garbage collection algorithm /// to use during code generation. bool hasGC() const { diff --git a/llvm/include/llvm/IR/GlobalObject.h b/llvm/include/llvm/IR/GlobalObject.h index 08edc13d81f88..73c1b87d67717 100644 --- a/llvm/include/llvm/IR/GlobalObject.h +++ b/llvm/include/llvm/IR/GlobalObject.h @@ -124,6 +124,12 @@ class GlobalObject : public GlobalValue { /// appropriate default object file section. void setSection(StringRef S); + /// Set the section prefix for this global object. + void setSectionPrefix(StringRef Prefix); + + /// Get the section prefix for this global object. + std::optional getSectionPrefix() const; + bool hasComdat() const { return getComdat() != nullptr; } const Comdat *getComdat() const { return ObjComdat; } Comdat *getComdat() { return ObjComdat; } diff --git a/llvm/include/llvm/IR/MDBuilder.h b/llvm/include/llvm/IR/MDBuilder.h index e02ec8f5a3d8b..ce4e1da656049 100644 --- a/llvm/include/llvm/IR/MDBuilder.h +++ b/llvm/include/llvm/IR/MDBuilder.h @@ -89,8 +89,8 @@ class MDBuilder { MDNode *createFunctionEntryCount(uint64_t Count, bool Synthetic, const DenseSet *Imports); - /// Return metadata containing the section prefix for a function. - MDNode *createFunctionSectionPrefix(StringRef Prefix); + /// Return metadata containing the section prefix for a global object. + MDNode *createGlobalObjectSectionPrefix(StringRef Prefix); /// Return metadata containing the pseudo probe descriptor for a function. MDNode *createPseudoProbeDesc(uint64_t GUID, uint64_t Hash, StringRef FName); diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp index 6ab6d18213ba4..ac28ea59c155d 100644 --- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp +++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp @@ -668,11 +668,10 @@ getELFSectionNameForGlobal(const GlobalObject *GO, SectionKind Kind, } bool HasPrefix = false; - if (const auto *F = dyn_cast(GO)) { - if (std::optional Prefix = F->getSectionPrefix()) { - raw_svector_ostream(Name) << '.' << *Prefix; - HasPrefix = true; - } + + if (std::optional Prefix = GO->getSectionPrefix()) { + raw_svector_ostream(Name) << '.' << *Prefix; + HasPrefix = true; } if (UniqueSectionName) { diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp index 9c5dd5aeb92e9..018228fb938d6 100644 --- a/llvm/lib/IR/Function.cpp +++ b/llvm/lib/IR/Function.cpp @@ -1164,22 +1164,6 @@ DenseSet Function::getImportGUIDs() const { return R; } -void Function::setSectionPrefix(StringRef Prefix) { - MDBuilder MDB(getContext()); - setMetadata(LLVMContext::MD_section_prefix, - MDB.createFunctionSectionPrefix(Prefix)); -} - -std::optional Function::getSectionPrefix() const { - if (MDNode *MD = getMetadata(LLVMContext::MD_section_prefix)) { - assert(cast(MD->getOperand(0))->getString() == - "function_section_prefix" && - "Metadata not match"); - return cast(MD->getOperand(1))->getString(); - } - return std::nullopt; -} - bool Function::nullPointerIsDefined() const { return hasFnAttribute(Attribute::NullPointerIsValid); } diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp index db5e1cb57b1ba..148f8e26535d2 100644 --- a/llvm/lib/IR/Globals.cpp +++ b/llvm/lib/IR/Globals.cpp @@ -18,6 +18,7 @@ #include "llvm/IR/GlobalAlias.h" #include "llvm/IR/GlobalValue.h" #include "llvm/IR/GlobalVariable.h" +#include "llvm/IR/MDBuilder.h" #include "llvm/IR/Module.h" #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" @@ -286,6 +287,24 @@ void GlobalObject::setSection(StringRef S) { setGlobalObjectFlag(HasSectionHashEntryBit, !S.empty()); } +void GlobalObject::setSectionPrefix(StringRef Prefix) { + MDBuilder MDB(getContext()); + setMetadata(LLVMContext::MD_section_prefix, + MDB.createGlobalObjectSectionPrefix(Prefix)); +} + +std::optional GlobalObject::getSectionPrefix() const { + if (MDNode *MD = getMetadata(LLVMContext::MD_section_prefix)) { + [[maybe_unused]] StringRef MDName = + cast(MD->getOperand(0))->getString(); + assert((MDName == "section_prefix" || + (isa(this) && MDName == "function_section_prefix")) && + "Metadata not match"); + return cast(MD->getOperand(1))->getString(); + } + return std::nullopt; +} + bool GlobalValue::isNobuiltinFnDef() const { const Function *F = dyn_cast(this); if (!F || F->empty()) diff --git a/llvm/lib/IR/MDBuilder.cpp b/llvm/lib/IR/MDBuilder.cpp index 26c8ab9fc36c8..b6aa8844a7eaf 100644 --- a/llvm/lib/IR/MDBuilder.cpp +++ b/llvm/lib/IR/MDBuilder.cpp @@ -87,9 +87,9 @@ MDNode *MDBuilder::createFunctionEntryCount( return MDNode::get(Context, Ops); } -MDNode *MDBuilder::createFunctionSectionPrefix(StringRef Prefix) { - return MDNode::get( - Context, {createString("function_section_prefix"), createString(Prefix)}); +MDNode *MDBuilder::createGlobalObjectSectionPrefix(StringRef Prefix) { + return MDNode::get(Context, + {createString("section_prefix"), createString(Prefix)}); } MDNode *MDBuilder::createRange(const APInt &Lo, const APInt &Hi) { diff --git a/llvm/test/CodeGen/X86/data-section-prefix.ll b/llvm/test/CodeGen/X86/data-section-prefix.ll new file mode 100644 index 0000000000000..4812fc70758fb --- /dev/null +++ b/llvm/test/CodeGen/X86/data-section-prefix.ll @@ -0,0 +1,27 @@ +; RUN: llc -mtriple x86_64-linux-gnu -data-sections %s -o - | FileCheck %s --check-prefix=ELF +; RUN: llc -mtriple x86_64-linux-gnu -unique-section-names=0 -data-sections %s -o - | FileCheck %s --check-prefix=ELF-NOUNIQ + +; RUN: llc -mtriple x86_64-windows-msvc -data-sections %s -o - | FileCheck %s --check-prefix=COFF-MSVC + +; ELF: .section .data.hot.foo, +; ELF: .section .data.bar, +; ELF: .section .bss.unlikely.baz, +; ELF: .section .bss.quz, + +; ELF-NOUNIQ: .section .data.hot.,"aw",@progbits,unique,1 +; ELF-NOUNIQ: .section .data,"aw",@progbits,unique,2 +; ELF-NOUNIQ: .section .bss.unlikely.,"aw",@nobits,unique,3 +; ELF-NOUNIQ: .section .bss,"aw",@nobits,unique,4 + +; COFF-MSVC: .section .data,"dw",one_only,foo +; COFF-MSVC: .section .data,"dw",one_only,bar +; COFF-MSVC: .section .bss,"bw",one_only,baz +; COFF-MSVC: .section .bss,"bw",one_only,quz + +@foo = global i32 1, !section_prefix !0 +@bar = global i32 2 +@baz = global i32 0, !section_prefix !1 +@quz = global i32 0 + +!0 = !{!"section_prefix", !"hot"} +!1 = !{!"section_prefix", !"unlikely"} diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/section-samplepgo.ll b/llvm/test/Transforms/CodeGenPrepare/X86/section-samplepgo.ll index 58af88d8cf365..48d02e5cebc69 100644 --- a/llvm/test/Transforms/CodeGenPrepare/X86/section-samplepgo.ll +++ b/llvm/test/Transforms/CodeGenPrepare/X86/section-samplepgo.ll @@ -34,8 +34,8 @@ define void @cold_func() !prof !16 { ret void } -; CHECK: ![[HOT_ID]] = !{!"function_section_prefix", !"hot"} -; CHECK: ![[COLD_ID]] = !{!"function_section_prefix", !"unlikely"} +; CHECK: ![[HOT_ID]] = !{!"section_prefix", !"hot"} +; CHECK: ![[COLD_ID]] = !{!"section_prefix", !"unlikely"} !llvm.module.flags = !{!1} !1 = !{i32 1, !"ProfileSummary", !2} !2 = !{!3, !4, !5, !6, !7, !8, !9, !10} diff --git a/llvm/test/Transforms/CodeGenPrepare/X86/section.ll b/llvm/test/Transforms/CodeGenPrepare/X86/section.ll index 6dad1122e4294..4baa0b5baa4be 100644 --- a/llvm/test/Transforms/CodeGenPrepare/X86/section.ll +++ b/llvm/test/Transforms/CodeGenPrepare/X86/section.ll @@ -66,8 +66,8 @@ define void @cold_func3() !prof !16 { ret void } -; CHECK: ![[HOT_ID]] = !{!"function_section_prefix", !"hot"} -; CHECK: ![[COLD_ID]] = !{!"function_section_prefix", !"unlikely"} +; CHECK: ![[HOT_ID]] = !{!"section_prefix", !"hot"} +; CHECK: ![[COLD_ID]] = !{!"section_prefix", !"unlikely"} !llvm.module.flags = !{!1} !1 = !{i32 1, !"ProfileSummary", !2} !2 = !{!3, !4, !5, !6, !7, !8, !9, !10} diff --git a/llvm/test/Transforms/HotColdSplit/coldentrycount.ll b/llvm/test/Transforms/HotColdSplit/coldentrycount.ll index 6e5ef1aa25392..1e8825e651ec4 100644 --- a/llvm/test/Transforms/HotColdSplit/coldentrycount.ll +++ b/llvm/test/Transforms/HotColdSplit/coldentrycount.ll @@ -27,9 +27,9 @@ declare void @sink() cold ; CHECK: define {{.*}} @fun.cold.1{{.*}} ![[PROF:[0-9]+]] {{.*}}section_prefix ![[UNLIKELY:[0-9]+]] ; CHECK: ![[HOTPROF]] = !{!"function_entry_count", i64 100} -; CHECK: ![[LIKELY]] = !{!"function_section_prefix", !"hot"} +; CHECK: ![[LIKELY]] = !{!"section_prefix", !"hot"} ; CHECK: ![[PROF]] = !{!"function_entry_count", i64 0} -; CHECK: ![[UNLIKELY]] = !{!"function_section_prefix", !"unlikely"} +; CHECK: ![[UNLIKELY]] = !{!"section_prefix", !"unlikely"} !llvm.module.flags = !{!0} !0 = !{i32 1, !"ProfileSummary", !1} diff --git a/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll b/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll index ef2ddbc33cee4..af4b875818f6f 100644 --- a/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll +++ b/llvm/test/Transforms/SampleProfile/section-accurate-samplepgo.ll @@ -36,11 +36,11 @@ attributes #1 = { "use-sample-profile" } ; CHECK: ![[NOPROFILE_ID]] = !{!"function_entry_count", i64 -1} ; CHECK: ![[ZERO_ID]] = !{!"function_entry_count", i64 0} -; CHECK: ![[COLD_ID]] = !{!"function_section_prefix", !"unlikely"} +; CHECK: ![[COLD_ID]] = !{!"section_prefix", !"unlikely"} ; UNKNOWN: ![[NOPROFILE_ID]] = !{!"function_entry_count", i64 -1} -; UNKNOWN: ![[UNKNOWN_ID]] = !{!"function_section_prefix", !"unknown"} +; UNKNOWN: ![[UNKNOWN_ID]] = !{!"section_prefix", !"unknown"} ; ACCURATE: ![[ZERO_ID]] = !{!"function_entry_count", i64 0} -; ACCURATE: ![[COLD_ID]] = !{!"function_section_prefix", !"unlikely"} +; ACCURATE: ![[COLD_ID]] = !{!"section_prefix", !"unlikely"} !llvm.module.flags = !{!1} !1 = !{i32 1, !"ProfileSummary", !2} !2 = !{!3, !4, !5, !6, !7, !8, !9, !10} From 630959c91591fc1e394735fdf05dc36e92ac9b22 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Mon, 3 Feb 2025 14:44:45 -0800 Subject: [PATCH 2/2] refactor --- llvm/lib/CodeGen/StaticDataSplitter.cpp | 127 +++++++++++------------- 1 file changed, 58 insertions(+), 69 deletions(-) diff --git a/llvm/lib/CodeGen/StaticDataSplitter.cpp b/llvm/lib/CodeGen/StaticDataSplitter.cpp index 25f02fde8a4b8..211b3b65107c8 100644 --- a/llvm/lib/CodeGen/StaticDataSplitter.cpp +++ b/llvm/lib/CodeGen/StaticDataSplitter.cpp @@ -41,25 +41,17 @@ STATISTIC(NumUnknownJumpTables, "Number of jump tables with unknown hotness. Option " "-static-data-default-hotness specifies the hotness."); -static cl::opt StaticDataDefaultHotness( - "static-data-default-hotness", cl::Hidden, - cl::desc("This option specifies the hotness of static data when profile " - "information is unavailable"), - cl::init(MachineFunctionDataHotness::Hot), - cl::values(clEnumValN(MachineFunctionDataHotness::Hot, "hot", "Hot"), - clEnumValN(MachineFunctionDataHotness::Cold, "cold", "Cold"))); - class StaticDataSplitter : public MachineFunctionPass { const MachineBranchProbabilityInfo *MBPI = nullptr; const MachineBlockFrequencyInfo *MBFI = nullptr; const ProfileSummaryInfo *PSI = nullptr; - // Returns true iff any jump table is hot-cold categorized. - bool splitJumpTables(MachineFunction &MF); + void updateStats(bool ProfileAvailable, const MachineJumpTableInfo *MJTI); + void updateJumpTableStats(bool ProfileAvailable, + const MachineJumpTableInfo &MJTI); - // Same as above but works on functions with profile information. - bool splitJumpTablesWithProfiles(const MachineFunction &MF, - MachineJumpTableInfo &MJTI); + // Use profiles to partition static data. + bool partitionStaticDataWithProfiles(MachineFunction &MF); public: static char ID; @@ -85,13 +77,22 @@ bool StaticDataSplitter::runOnMachineFunction(MachineFunction &MF) { MBFI = &getAnalysis().getMBFI(); PSI = &getAnalysis().getPSI(); - return splitJumpTables(MF); + const bool ProfileAvailable = PSI && PSI->hasProfileSummary() && MBFI && + MF.getFunction().hasProfileData(); + bool Changed = false; + + if (ProfileAvailable) + Changed |= partitionStaticDataWithProfiles(MF); + + updateStats(ProfileAvailable, MF.getJumpTableInfo()); + return Changed; } -bool StaticDataSplitter::splitJumpTablesWithProfiles( - const MachineFunction &MF, MachineJumpTableInfo &MJTI) { +bool StaticDataSplitter::partitionStaticDataWithProfiles(MachineFunction &MF) { int NumChangedJumpTables = 0; + MachineJumpTableInfo *MJTI = MF.getJumpTableInfo(); + // Jump table could be used by either terminating instructions or // non-terminating ones, so we walk all instructions and use // `MachineOperand::isJTI()` to identify jump table operands. @@ -100,70 +101,58 @@ bool StaticDataSplitter::splitJumpTablesWithProfiles( for (const auto &MBB : MF) { for (const MachineInstr &I : MBB) { for (const MachineOperand &Op : I.operands()) { - if (!Op.isJTI()) - continue; - const int JTI = Op.getIndex(); - // This is not a source block of jump table. - if (JTI == -1) - continue; - - auto Hotness = MachineFunctionDataHotness::Hot; - - // Hotness is based on source basic block hotness. - // TODO: PSI APIs are about instruction hotness. Introduce API for data - // access hotness. - if (PSI->isColdBlock(&MBB, MBFI)) - Hotness = MachineFunctionDataHotness::Cold; - - if (MJTI.updateJumpTableEntryHotness(JTI, Hotness)) - ++NumChangedJumpTables; + if (Op.isJTI()) { + assert(MJTI != nullptr && "Jump table info is not available."); + const int JTI = Op.getIndex(); + // This is not a source block of jump table. + if (JTI == -1) + continue; + + auto Hotness = MachineFunctionDataHotness::Hot; + + // Hotness is based on source basic block hotness. + // TODO: PSI APIs are about instruction hotness. Introduce API for + // data access hotness. + if (PSI->isColdBlock(&MBB, MBFI)) + Hotness = MachineFunctionDataHotness::Cold; + + if (MJTI->updateJumpTableEntryHotness(JTI, Hotness)) + ++NumChangedJumpTables; + } } } } return NumChangedJumpTables > 0; } -bool StaticDataSplitter::splitJumpTables(MachineFunction &MF) { - MachineJumpTableInfo *MJTI = MF.getJumpTableInfo(); - if (!MJTI || MJTI->getJumpTables().empty()) - return false; +void StaticDataSplitter::updateJumpTableStats( + bool ProfileAvailable, const MachineJumpTableInfo &MJTI) { - const bool ProfileAvailable = PSI && PSI->hasProfileSummary() && MBFI && - MF.getFunction().hasProfileData(); - auto statOnExit = llvm::make_scope_exit([&] { - if (!AreStatisticsEnabled()) - return; - - if (!ProfileAvailable) { - NumUnknownJumpTables += MJTI->getJumpTables().size(); - return; - } + if (!ProfileAvailable) { + NumUnknownJumpTables += MJTI.getJumpTables().size(); + return; + } - for (size_t JTI = 0; JTI < MJTI->getJumpTables().size(); JTI++) { - auto Hotness = MJTI->getJumpTables()[JTI].Hotness; - if (Hotness == MachineFunctionDataHotness::Hot) { - ++NumHotJumpTables; - } else { - assert(Hotness == MachineFunctionDataHotness::Cold && - "A jump table is either hot or cold when profile information is " - "available."); - ++NumColdJumpTables; - } + for (size_t JTI = 0; JTI < MJTI.getJumpTables().size(); JTI++) { + auto Hotness = MJTI.getJumpTables()[JTI].Hotness; + if (Hotness == MachineFunctionDataHotness::Hot) { + ++NumHotJumpTables; + } else { + assert(Hotness == MachineFunctionDataHotness::Cold && + "A jump table is either hot or cold when profile information is " + "available."); + ++NumColdJumpTables; } - }); - - // Place jump tables according to block hotness if function has profile data. - if (ProfileAvailable) - return splitJumpTablesWithProfiles(MF, *MJTI); + } +} - // If function profile is unavailable (e.g., module not instrumented, or new - // code paths lacking samples), -static-data-default-hotness specifies the - // hotness. - for (size_t JTI = 0; JTI < MJTI->getJumpTables().size(); JTI++) - MF.getJumpTableInfo()->updateJumpTableEntryHotness( - JTI, StaticDataDefaultHotness); +void StaticDataSplitter::updateStats(bool ProfileAvailable, + const MachineJumpTableInfo *MJTI) { + if (!AreStatisticsEnabled()) + return; - return true; + if (MJTI) + updateJumpTableStats(ProfileAvailable, *MJTI); } char StaticDataSplitter::ID = 0;