bd1976llvm created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.
Alternative to https://reviews.llvm.org/D68101
Prevents globals with an explicit section from being mergeable by default (as
in D68101 <https://reviews.llvm.org/D68101>).
Adds a command line option to allow the explicit section to be mergeable; but,
an error will be emitted if broken output would be created.
Users can use the command line option if the default behaviour results in a
performance regression.
This is a partial fix for https://bugs.llvm.org/show_bug.cgi?id=43457.
Repository:
rC Clang
https://reviews.llvm.org/D68147
Files:
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/CodeGen/CommandFlags.inc
llvm/include/llvm/Target/TargetMachine.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/Target/TargetLoweringObjectFile.cpp
llvm/test/CodeGen/X86/explict-section-mergeable.ll
llvm/test/CodeGen/X86/section_mergeable_size.ll
Index: llvm/test/CodeGen/X86/section_mergeable_size.ll
===================================================================
--- llvm/test/CodeGen/X86/section_mergeable_size.ll
+++ llvm/test/CodeGen/X86/section_mergeable_size.ll
@@ -1,3 +1,3 @@
-; RUN: llc -mtriple x86_64-linux-gnu < %s | FileCheck %s
+; RUN: llc -mtriple x86_64-linux-gnu -mergeable-explicit-sections < %s | FileCheck %s
@a = internal unnamed_addr constant [1 x [1 x i32]] zeroinitializer, section ".init.rodata", align 4
; CHECK: .init.rodata,"aM",{{[@%]}}progbits,4
Index: llvm/test/CodeGen/X86/explict-section-mergeable.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/X86/explict-section-mergeable.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s -mtriple=x86_64-linux-gnu | FileCheck %s --check-prefix=DEFAULT
+
+;; show that the implicitly generated sections are as expected
+@implicit1 = unnamed_addr constant [2 x i16] [i16 1, i16 1]
+; DEFAULT-CHECK: .section .rodata.cst4,"aM",@progbits,4
+@implicit2 = unnamed_addr constant [2 x i32] [i32 1, i32 1]
+; DEFAULT-CHECK: .section .rodata.cst8,"aM",@progbits,8
+@implicit3 = unnamed_addr constant [2 x i16] [i16 1, i16 0]
+; DEFAULT-CHECK: .section .rodata.str2.2,"aMS",@progbits,2
+
+;; Check that an explicitly created section is not SHF_MERGE and that
+;; it contains all three symbols.
+@explicit1 = unnamed_addr constant [2 x i16] [i16 1, i16 1] "rodata-section"=".explicit"
+@explicit2 = unnamed_addr constant [2 x i32] [i32 1, i32 1] "rodata-section"=".explicit"
+@explicit3 = unnamed_addr constant [2 x i16] [i16 1, i16 0] "rodata-section"=".explicit"
+; DEFAULT: .section .explicit,"a",@progbits
+; DEFAULT-NOT: .section
+; DEFAULT: explicit1:
+; DEFAULT-NOT: .section
+; DEFAULT: explicit2:
+; DEFAULT-NOT: .section
+; DEFAULT: explicit3:
+
+; RUN: not llc < %s -mtriple=x86_64-linux-gnu -mergeable-explicit-sections 2>&1 | FileCheck %s --check-prefix=MERGE
+
+; MERGE: Symbol 'explicit2' from module '<stdin>' required a section with entry-size=8 but was placed in section '.explicit' with entry-size=4: Explicit assignment by pragma or attribute of incompatible symbols to a section?
+
Index: llvm/lib/Target/TargetLoweringObjectFile.cpp
===================================================================
--- llvm/lib/Target/TargetLoweringObjectFile.cpp
+++ llvm/lib/Target/TargetLoweringObjectFile.cpp
@@ -135,6 +135,26 @@
const MCSymbol *Sym) const {
}
+static bool inExplicitSection(const GlobalObject *GO, SectionKind Kind) {
+ if (GO->hasSection())
+ return true;
+
+ if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
+ auto Attrs = GVar->getAttributes();
+ if ((Attrs.hasAttribute("bss-section") && Kind.isBSS()) ||
+ (Attrs.hasAttribute("data-section") && Kind.isData()) ||
+ (Attrs.hasAttribute("rodata-section") && Kind.isReadOnly())) {
+ return true;
+ }
+ }
+
+ if (auto *F = dyn_cast<Function>(GO)) {
+ if (F->hasFnAttribute("implicit-section-name"))
+ return true;
+ }
+
+ return false;
+}
/// getKindForGlobal - This is a top-level target-independent classifier for
/// a global object. Given a global variable and information from the TM, this
@@ -187,6 +207,16 @@
if (!GVar->hasGlobalUnnamedAddr())
return SectionKind::getReadOnly();
+ // If two globals with differing sizes end up in the same mergeable section that
+ // section can be assigned an incorrect entry size. To avoid this we do not put
+ // globals into a mergeable section if they have been explicitly assigned to a section.
+ // TODO: This is a pessimistic solution as we lose the benifits of mergeable sections. A
+ // better solution might be to bin globals of the same size into different mergeable
+ // sections with the same name.
+ if (inExplicitSection(GO, SectionKind::getReadOnly()) &&
+ !TM.getMergeableExplicitSections())
+ return SectionKind::getReadOnly();
+
// If initializer is a null-terminated string, put it in a "cstring"
// section of the right width.
if (ArrayType *ATy = dyn_cast<ArrayType>(C->getType())) {
@@ -245,24 +275,8 @@
/// available externally) globals.
MCSection *TargetLoweringObjectFile::SectionForGlobal(
const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const {
- // Select section name.
- if (GO->hasSection())
+ if (inExplicitSection(GO, Kind))
return getExplicitSectionGlobal(GO, Kind, TM);
-
- if (auto *GVar = dyn_cast<GlobalVariable>(GO)) {
- auto Attrs = GVar->getAttributes();
- if ((Attrs.hasAttribute("bss-section") && Kind.isBSS()) ||
- (Attrs.hasAttribute("data-section") && Kind.isData()) ||
- (Attrs.hasAttribute("rodata-section") && Kind.isReadOnly())) {
- return getExplicitSectionGlobal(GO, Kind, TM);
- }
- }
-
- if (auto *F = dyn_cast<Function>(GO)) {
- if (F->hasFnAttribute("implicit-section-name"))
- return getExplicitSectionGlobal(GO, Kind, TM);
- }
-
// Use default section depending on the 'type' of global
return SelectSectionForGlobal(GO, Kind, TM);
}
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===================================================================
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -603,6 +603,21 @@
// This should not be possible due to UniqueID code above.
assert(Section->getAssociatedSymbol() == AssociatedSymbol &&
"Associated symbol mismatch between sections");
+
+ // If two globals with differing sizes end up in the same mergeable section
+ // that section can be assigned an incorrect entry size. Error rather than
+ // creating broken output.
+ if ((Section->getEntrySize() != 0) &&
+ (Section->getEntrySize() != getEntrySizeForKind(Kind)))
+ report_fatal_error(
+ "Symbol '" + GO->getName() + "' from module '" +
+ (GO->getParent() ? GO->getParent()->getSourceFileName() : "unknown") +
+ "' required a section with entry-size=" +
+ Twine(getEntrySizeForKind(Kind)) + " but was placed in section '" +
+ SectionName + "' with entry-size=" + Twine(Section->getEntrySize()) +
+ ": Explicit assignment by pragma or attribute of incompatible symbols "
+ "to a section?");
+
return Section;
}
Index: llvm/include/llvm/Target/TargetOptions.h
===================================================================
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -114,6 +114,7 @@
EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
DisableIntegratedAS(false), RelaxELFRelocations(false),
FunctionSections(false), DataSections(false),
+ MergeableExplicitSections(false),
UniqueSectionNames(true), TrapUnreachable(false),
NoTrapAfterNoreturn(false), EmulatedTLS(false),
ExplicitEmulatedTLS(false), EnableIPRA(false),
@@ -222,6 +223,10 @@
/// Emit data into separate sections.
unsigned DataSections : 1;
+ /// Emit globals with explicit sections into mergeable sections; May result
+ /// in an error if incompatible symbols are assigned to a section.
+ unsigned MergeableExplicitSections : 1;
+
unsigned UniqueSectionNames : 1;
/// Emit target-specific trap instruction for 'unreachable' IR instructions.
Index: llvm/include/llvm/Target/TargetMachine.h
===================================================================
--- llvm/include/llvm/Target/TargetMachine.h
+++ llvm/include/llvm/Target/TargetMachine.h
@@ -254,6 +254,10 @@
return Options.FunctionSections;
}
+ bool getMergeableExplicitSections() const {
+ return Options.MergeableExplicitSections;
+ }
+
/// Get a \c TargetIRAnalysis appropriate for the target.
///
/// This is used to construct the new pass manager's target IR analysis pass,
Index: llvm/include/llvm/CodeGen/CommandFlags.inc
===================================================================
--- llvm/include/llvm/CodeGen/CommandFlags.inc
+++ llvm/include/llvm/CodeGen/CommandFlags.inc
@@ -233,6 +233,10 @@
cl::desc("Emit data into separate sections"),
cl::init(false));
+static cl::opt<bool> MergeableExplicitSections ("mergeable-explicit-sections",
+ cl::desc("Attempts mark explicit sections as mergeable. May result in an error if incompatible symbols are assigned to a section."),
+ cl::init(false));
+
static cl::opt<bool>
FunctionSections("function-sections",
cl::desc("Emit functions into separate sections"),
@@ -298,6 +302,7 @@
Options.UseInitArray = !UseCtors;
Options.RelaxELFRelocations = RelaxELFRelocations;
Options.DataSections = DataSections;
+ Options.MergeableExplicitSections = MergeableExplicitSections;
Options.FunctionSections = FunctionSections;
Options.UniqueSectionNames = UniqueSectionNames;
Options.EmulatedTLS = EmulatedTLS;
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -949,6 +949,8 @@
OPT_fno_function_sections, false);
Opts.DataSections = Args.hasFlag(OPT_fdata_sections,
OPT_fno_data_sections, false);
+ Opts.MergeableExplicitSections = Args.hasFlag(
+ OPT_fmergeable_explicit_sections, OPT_fmergeable_explicit_sections, false);
Opts.StackSizeSection =
Args.hasFlag(OPT_fstack_size_section, OPT_fno_stack_size_section, false);
Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -433,6 +433,11 @@
CmdArgs.push_back("-plugin-opt=-data-sections");
}
+ if (Args.hasFlag(options::OPT_fmergeable_explicit_sections, options::OPT_fno_mergeable_explicit_sections,
+ false)) {
+ CmdArgs.push_back("-plugin-opt=-mergeable-explicit-sections");
+ }
+
if (Arg *A = getLastProfileSampleUseArg(Args)) {
StringRef FName = A->getValue();
if (!llvm::sys::fs::exists(FName))
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4289,6 +4289,11 @@
CmdArgs.push_back("-fdata-sections");
}
+ if (Args.hasFlag(options::OPT_fmergeable_explicit_sections, options::OPT_fno_mergeable_explicit_sections,
+ false)) {
+ CmdArgs.push_back("-fmergeable-explicit-sections");
+ }
+
if (!Args.hasFlag(options::OPT_funique_section_names,
options::OPT_fno_unique_section_names, true))
CmdArgs.push_back("-fno-unique-section-names");
Index: clang/lib/CodeGen/BackendUtil.cpp
===================================================================
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -478,6 +478,7 @@
Options.StackAlignmentOverride = CodeGenOpts.StackAlignment;
Options.FunctionSections = CodeGenOpts.FunctionSections;
Options.DataSections = CodeGenOpts.DataSections;
+ Options.MergeableExplicitSections = CodeGenOpts.MergeableExplicitSections;
Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
Options.ExplicitEmulatedTLS = CodeGenOpts.ExplicitEmulatedTLS;
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1867,6 +1867,10 @@
Flags<[CC1Option]>, HelpText<"Place each data in its own section (ELF Only)">;
def fno_data_sections : Flag <["-"], "fno-data-sections">, Group<f_Group>,
Flags<[CC1Option]>;
+def fmergeable_explicit_sections : Flag <["-"], "fmergeable-explicit-sections">, Group<f_Group>,
+ Flags<[CC1Option]>, HelpText<"Attempts mark explicit sections as mergeable. May result in an error if incompatible symbols are assigned to a section.">;
+def fno_mergeable_explicit_sections : Flag <["-"], "fno-mergeable-explicit-sections">, Group<f_Group>,
+ Flags<[CC1Option]>;
def fstack_size_section : Flag<["-"], "fstack-size-section">, Group<f_Group>, Flags<[CC1Option]>,
HelpText<"Emit section containing metadata on function stack sizes">;
def fno_stack_size_section : Flag<["-"], "fno-stack-size-section">, Group<f_Group>, Flags<[CC1Option]>,
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -46,6 +46,7 @@
CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
///< aliases to base ctors when possible.
CODEGENOPT(DataSections , 1, 0) ///< Set when -fdata-sections is enabled.
+CODEGENOPT(MergeableExplicitSections, 1, 0) ///< Set when -fmergeable-explict-sections is enabled.
CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names.
ENUM_CODEGENOPT(FramePointer, FramePointerKind, 2, FramePointerKind::None) /// frame-pointer: all,non-leaf,none
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
