Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] -fseparate-named-sections option #91028

Merged
merged 6 commits into from
May 7, 2024

Conversation

petrhosek
Copy link
Member

When set, the compiler will use separate unique sections for global symbols in named special sections (e.g. symbols that are annotated with attribute((section(...)))). Doing so enables linker GC to collect unused symbols without having to use a different section per-symbol.

When set, the compiler will use separate unique sections for global
symbols in named special sections (e.g. symbols that are annotated with
__attribute__((section(...)))). Doing so enables linker GC to collect
unused symbols without having to use a different section per-symbol.
@petrhosek
Copy link
Member Author

This is an alternative approach to address the issue described in https://discourse.llvm.org/t/rfc-support-for-memory-regions-in-elf/78570 which doesn't require a custom section type.

@MaskRay
Copy link
Member

MaskRay commented May 4, 2024

This is an alternative approach to address the issue described in discourse.llvm.org/t/rfc-support-for-memory-regions-in-elf/78570 which doesn't require a custom section type.

This RFC is about SHT_LLVM_MEMORY while this patch seems orthogonal?

I agree that the option is useful in some cases. If ld -r is used, --unique will be needed to not destroy granularity.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels May 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 4, 2024

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Petr Hosek (petrhosek)

Changes

When set, the compiler will use separate unique sections for global symbols in named special sections (e.g. symbols that are annotated with attribute((section(...)))). Doing so enables linker GC to collect unused symbols without having to use a different section per-symbol.


Full diff: https://github.com/llvm/llvm-project/pull/91028.diff

11 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+5)
  • (modified) clang/include/clang/Basic/CodeGenOptions.def (+1)
  • (modified) clang/include/clang/Driver/Options.td (+5)
  • (modified) clang/lib/CodeGen/BackendUtil.cpp (+1)
  • (added) clang/test/CodeGen/fseparate-named-sections.c (+28)
  • (modified) llvm/include/llvm/CodeGen/CommandFlags.h (+2)
  • (modified) llvm/include/llvm/Target/TargetMachine.h (+4)
  • (modified) llvm/include/llvm/Target/TargetOptions.h (+11-9)
  • (modified) llvm/lib/CodeGen/CommandFlags.cpp (+8)
  • (modified) llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (+13-3)
  • (added) llvm/test/CodeGen/X86/elf-separate-named-sections.ll (+36)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2b3bafa1c30548..1802267a5ac609 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -221,6 +221,11 @@ New Compiler Flags
 - ``-fexperimental-modules-reduced-bmi`` enables the Reduced BMI for C++20 named modules.
   See the document of standard C++ modules for details.
 
+- ``-fseparate-named-sections`` uses separate unique sections for global
+  symbols in named special sections (i.e. symbols annotated with
+  ``__attribute__((section(...)))``. This enabled linker GC to collect unused
+  symbols without having to use a per-symbol section.
+
 Deprecated Compiler Flags
 -------------------------
 
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 340b08dd7e2a33..b964e45574782c 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -57,6 +57,7 @@ CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names.
 CODEGENOPT(UniqueBasicBlockSectionNames, 1, 1) ///< Set for -funique-basic-block-section-names,
                                                ///< Produce unique section names with
                                                ///< basic block sections.
+CODEGENOPT(SeparateNamedSections, 1, 0) ///< Set for -fseparate-named-sections.
 CODEGENOPT(EnableAIXExtendedAltivecABI, 1, 0) ///< Set for -mabi=vec-extabi. Enables the extended Altivec ABI on AIX.
 CODEGENOPT(XCOFFReadOnlyPointers, 1, 0) ///< Set for -mxcoff-roptr.
 CODEGENOPT(AllTocData, 1, 0) ///< AIX -mtocdata
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 9f86808145d9ab..deb1c2253c1ed2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4136,6 +4136,11 @@ defm unique_section_names : BoolFOption<"unique-section-names",
   NegFlag<SetFalse, [], [ClangOption, CC1Option],
           "Don't use unique names for text and data sections">,
   PosFlag<SetTrue>>;
+defm separate_named_sections : BoolFOption<"separate-named-sections",
+  CodeGenOpts<"SeparateNamedSections">, DefaultFalse,
+  PosFlag<SetTrue, [], [ClangOption, CC1Option],
+          "Use separate unique sections for named sections (ELF Only)">,
+  NegFlag<SetFalse>>;
 
 defm split_machine_functions: BoolFOption<"split-machine-functions",
   CodeGenOpts<"SplitMachineFunctions">, DefaultFalse,
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 22c3f8642ad8eb..119ec4704002cf 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -423,6 +423,7 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
   Options.UniqueBasicBlockSectionNames =
       CodeGenOpts.UniqueBasicBlockSectionNames;
+  Options.SeparateNamedSections = CodeGenOpts.SeparateNamedSections;
   Options.TLSSize = CodeGenOpts.TLSSize;
   Options.EnableTLSDESC = CodeGenOpts.EnableTLSDESC;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
diff --git a/clang/test/CodeGen/fseparate-named-sections.c b/clang/test/CodeGen/fseparate-named-sections.c
new file mode 100644
index 00000000000000..7a247dbd085cbe
--- /dev/null
+++ b/clang/test/CodeGen/fseparate-named-sections.c
@@ -0,0 +1,28 @@
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -o - < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -S -fseparate-named-sections -o - < %s | FileCheck %s --check-prefix=SEPARATE
+
+__attribute__((section("custom_text"))) void f(void) {}
+__attribute__((section("custom_text"))) void g(void) {}
+
+// CHECK: .section custom_text,"ax",@progbits{{$}}
+// CHECK: f:
+// CHECK: g:
+
+// SEPARATE: .section custom_text,"ax",@progbits,unique,1{{$}}
+// SEPARATE: f:
+// SEPARATE: .section custom_text,"ax",@progbits,unique,2{{$}}
+// SEPARATE: g:
+
+__attribute__((section("custom_data"))) int i = 0;
+__attribute__((section("custom_data"))) int j = 0;
+
+// CHECK: .section custom_data,"aw",@progbits{{$}}
+// CHECK: i:
+// CHECK: j:
+
+// SEPARATE: .section custom_data,"aw",@progbits,unique,3{{$}}
+// SEPARATE: i:
+// SEPARATE: .section custom_data,"aw",@progbits,unique,4{{$}}
+// SEPARATE: j:
diff --git a/llvm/include/llvm/CodeGen/CommandFlags.h b/llvm/include/llvm/CodeGen/CommandFlags.h
index 244dabd38cf65b..d5448d781363d4 100644
--- a/llvm/include/llvm/CodeGen/CommandFlags.h
+++ b/llvm/include/llvm/CodeGen/CommandFlags.h
@@ -122,6 +122,8 @@ bool getUniqueSectionNames();
 
 bool getUniqueBasicBlockSectionNames();
 
+bool getSeparateNamedSections();
+
 llvm::EABI getEABIVersion();
 
 llvm::DebuggerKind getDebuggerTuningOpt();
diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
index 48ea3cfe02775b..1ba99730ca7024 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -288,6 +288,10 @@ class TargetMachine {
     return Options.UniqueBasicBlockSectionNames;
   }
 
+  bool getSeparateNamedSections() const {
+    return Options.SeparateNamedSections;
+  }
+
   /// Return true if data objects should be emitted into their own section,
   /// corresponds to -fdata-sections.
   bool getDataSections() const {
diff --git a/llvm/include/llvm/Target/TargetOptions.h b/llvm/include/llvm/Target/TargetOptions.h
index d37e9d9576ba75..25d1f3f81fc142 100644
--- a/llvm/include/llvm/Target/TargetOptions.h
+++ b/llvm/include/llvm/Target/TargetOptions.h
@@ -144,15 +144,15 @@ namespace llvm {
           DisableIntegratedAS(false), FunctionSections(false),
           DataSections(false), IgnoreXCOFFVisibility(false),
           XCOFFTracebackTable(true), UniqueSectionNames(true),
-          UniqueBasicBlockSectionNames(false), TrapUnreachable(false),
-          NoTrapAfterNoreturn(false), TLSSize(0), EmulatedTLS(false),
-          EnableTLSDESC(false), EnableIPRA(false), EmitStackSizeSection(false),
-          EnableMachineOutliner(false), EnableMachineFunctionSplitter(false),
-          SupportsDefaultOutlining(false), EmitAddrsig(false), BBAddrMap(false),
-          EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
-          EnableDebugEntryValues(false), ValueTrackingVariableLocations(false),
-          ForceDwarfFrameSection(false), XRayFunctionIndex(true),
-          DebugStrictDwarf(false), Hotpatch(false),
+          UniqueBasicBlockSectionNames(false), SeparateNamedSections(false),
+          TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
+          EmulatedTLS(false), EnableTLSDESC(false), EnableIPRA(false),
+          EmitStackSizeSection(false), EnableMachineOutliner(false),
+          EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
+          EmitAddrsig(false), BBAddrMap(false), EmitCallSiteInfo(false),
+          SupportsDebugEntryValues(false), EnableDebugEntryValues(false),
+          ValueTrackingVariableLocations(false), ForceDwarfFrameSection(false),
+          XRayFunctionIndex(true), DebugStrictDwarf(false), Hotpatch(false),
           PPCGenScalarMASSEntries(false), JMCInstrument(false),
           EnableCFIFixup(false), MisExpect(false), XCOFFReadOnlyPointers(false),
           FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
@@ -277,6 +277,8 @@ namespace llvm {
     /// Use unique names for basic block sections.
     unsigned UniqueBasicBlockSectionNames : 1;
 
+    unsigned SeparateNamedSections : 1;
+
     /// Emit target-specific trap instruction for 'unreachable' IR instructions.
     unsigned TrapUnreachable : 1;
 
diff --git a/llvm/lib/CodeGen/CommandFlags.cpp b/llvm/lib/CodeGen/CommandFlags.cpp
index 14ac4b2102c2fa..677460a2d8e401 100644
--- a/llvm/lib/CodeGen/CommandFlags.cpp
+++ b/llvm/lib/CodeGen/CommandFlags.cpp
@@ -96,6 +96,7 @@ CGOPT_EXP(bool, EmulatedTLS)
 CGOPT_EXP(bool, EnableTLSDESC)
 CGOPT(bool, UniqueSectionNames)
 CGOPT(bool, UniqueBasicBlockSectionNames)
+CGOPT(bool, SeparateNamedSections)
 CGOPT(EABI, EABIVersion)
 CGOPT(DebuggerKind, DebuggerTuningOpt)
 CGOPT(bool, EnableStackSizeSection)
@@ -419,6 +420,12 @@ codegen::RegisterCodeGenFlags::RegisterCodeGenFlags() {
       cl::init(false));
   CGBINDOPT(UniqueBasicBlockSectionNames);
 
+  static cl::opt<bool> SeparateNamedSections(
+      "separate-named-sections",
+      cl::desc("Use separate unique sections for named sections"),
+      cl::init(false));
+  CGBINDOPT(SeparateNamedSections);
+
   static cl::opt<EABI> EABIVersion(
       "meabi", cl::desc("Set EABI type (default depends on triple):"),
       cl::init(EABI::Default),
@@ -569,6 +576,7 @@ codegen::InitTargetOptionsFromCodeGenFlags(const Triple &TheTriple) {
   Options.BBSections = getBBSectionsMode(Options);
   Options.UniqueSectionNames = getUniqueSectionNames();
   Options.UniqueBasicBlockSectionNames = getUniqueBasicBlockSectionNames();
+  Options.SeparateNamedSections = getSeparateNamedSections();
   Options.TLSSize = getTLSSize();
   Options.EmulatedTLS =
       getExplicitEmulatedTLS().value_or(TheTriple.hasDefaultEmulatedTLS());
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 2a77a683a901f1..1217559e1e2276 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -75,6 +75,10 @@ static cl::opt<bool> JumpTableInFunctionSection(
     "jumptable-in-function-section", cl::Hidden, cl::init(false),
     cl::desc("Putting Jump Table in function section"));
 
+static cl::opt<bool> UniqueExplicitSections(
+    "unique-explicit-sections", cl::Hidden, cl::init(true),
+    cl::desc("Putting symbols with explicit section into unique sections"));
+
 static void GetObjCImageInfo(Module &M, unsigned &Version, unsigned &Flags,
                              StringRef &Section) {
   SmallVector<Module::ModuleFlagEntry, 8> ModuleFlags;
@@ -733,16 +737,22 @@ calcUniqueIDUpdateFlagsAndSize(const GlobalObject *GO, StringRef SectionName,
       Ctx.isELFGenericMergeableSection(SectionName);
   // If this is the first ocurrence of this section name, treat it as the
   // generic section
-  if (!SymbolMergeable && !SeenSectionNameBefore)
-    return MCContext::GenericSectionID;
+  if (!SymbolMergeable && !SeenSectionNameBefore) {
+    if (TM.getSeparateNamedSections())
+      return NextUniqueID++;
+    else
+      return MCContext::GenericSectionID;
+  }
 
   // Symbols must be placed into sections with compatible entry sizes. Generate
   // unique sections for symbols that have not been assigned to compatible
   // sections.
   const auto PreviousID =
       Ctx.getELFUniqueIDForEntsize(SectionName, Flags, EntrySize);
-  if (PreviousID)
+  if (PreviousID && (!TM.getSeparateNamedSections() ||
+                     *PreviousID == MCContext::GenericSectionID)) {
     return *PreviousID;
+  }
 
   // If the user has specified the same section name as would be created
   // implicitly for this symbol e.g. .rodata.str1.1, then we don't need
diff --git a/llvm/test/CodeGen/X86/elf-separate-named-sections.ll b/llvm/test/CodeGen/X86/elf-separate-named-sections.ll
new file mode 100644
index 00000000000000..0631b604afab8e
--- /dev/null
+++ b/llvm/test/CodeGen/X86/elf-separate-named-sections.ll
@@ -0,0 +1,36 @@
+; Test that global values with explicit sections are placed into unique sections.
+
+; RUN: llc < %s 2>&1 | FileCheck %s
+; RUN: llc -separate-named-sections < %s 2>&1 | FileCheck %s --check-prefix=SEPARATE
+target triple="x86_64-unknown-unknown-elf"
+
+define i32 @f() section "custom_text" {
+    entry:
+    ret i32 0
+}
+
+define i32 @g() section "custom_text" {
+    entry:
+    ret i32 0
+}
+
+; CHECK: .section custom_text,"ax",@progbits{{$}}
+; CHECK: f:
+; CHECK: g:
+
+; SEPARATE: .section custom_text,"ax",@progbits,unique,1{{$}}
+; SEPARATE: f:
+; SEPARATE: .section custom_text,"ax",@progbits,unique,2{{$}}
+; SEPARATE: g:
+
+@i = global i32 0, section "custom_data", align 8
+@j = global i32 0, section "custom_data", align 8
+
+; CHECK: .section custom_data,"aw",@progbits{{$}}
+; CHECK: i:
+; CHECK: j:
+
+; SEPARATE: .section custom_data,"aw",@progbits,unique,3{{$}}
+; SEPARATE: i:
+; SEPARATE: .section custom_data,"aw",@progbits,unique,4{{$}}
+; SEPARATE: j:


// Symbols must be placed into sections with compatible entry sizes. Generate
// unique sections for symbols that have not been assigned to compatible
// sections.
const auto PreviousID =
Ctx.getELFUniqueIDForEntsize(SectionName, Flags, EntrySize);
if (PreviousID)
if (PreviousID && (!TM.getSeparateNamedSections() ||
*PreviousID == MCContext::GenericSectionID)) {
return *PreviousID;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop braces for one-line single statement

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -0,0 +1,36 @@
; Test that global values with explicit sections are placed into unique sections.

; RUN: llc < %s 2>&1 | FileCheck %s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop unused 2>&1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@petrhosek
Copy link
Member Author

This is an alternative approach to address the issue described in discourse.llvm.org/t/rfc-support-for-memory-regions-in-elf/78570 which doesn't require a custom section type.

This RFC is about SHT_LLVM_MEMORY while this patch seems orthogonal?

We might still consider implementing SHT_LLVM_MEMORY since it'd enable new use cases, but this PR addresses one of the issues described in that RFC without requiring significant changes in either LLVM or the existing code.

I agree that the option is useful in some cases. If ld -r is used, --unique will be needed to not destroy granularity.

Copy link
Collaborator

@smithp35 smithp35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! Just a couple of small comment suggestions from me.

@@ -221,6 +221,11 @@ New Compiler Flags
- ``-fexperimental-modules-reduced-bmi`` enables the Reduced BMI for C++20 named modules.
See the document of standard C++ modules for details.

- ``-fseparate-named-sections`` uses separate unique sections for global
symbols in named special sections (i.e. symbols annotated with
``__attribute__((section(...)))``. This enabled linker GC to collect unused
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be "This enables linker GC to collect ..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -277,6 +277,8 @@ namespace llvm {
/// Use unique names for basic block sections.
unsigned UniqueBasicBlockSectionNames : 1;

unsigned SeparateNamedSections : 1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all of the other flags have Doxygen comments. Worth something like:
/// Emit named sections with the same name into different sections.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@petrhosek petrhosek merged commit 8bcb073 into llvm:main May 7, 2024
4 of 5 checks passed
petrhosek added a commit to petrhosek/llvm-project that referenced this pull request May 9, 2024
petrhosek added a commit that referenced this pull request May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants