Skip to content

[X86] Set SHF_X86_64_LARGE for globals with explicit well-known large section name #74381

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

Merged
merged 2 commits into from
Dec 5, 2023

Conversation

aeubanks
Copy link
Contributor

@aeubanks aeubanks commented Dec 4, 2023

Globals marked with the .lbss/.ldata/.lrodata should automatically be treated as large.
Do this regardless of the code model for consistency when mixing object files compiled with different code models.

Basically the other half of #70748.

Example in the wild:
https://codebrowser.dev/qt5/qtbase/src/testlib/qtestcase.cpp.html#1664

… section name

Globals marked with the .lbss/.ldata/.lrodata should automatically be treated as large.
Do this regardless of the code model for consistency when mixing object files compiled with different code models.

Basically the opposite of llvm#70748.

Example in the wild:
https://codebrowser.dev/qt5/qtbase/src/testlib/qtestcase.cpp.html#1664
@aeubanks aeubanks requested review from MaskRay and rnk December 4, 2023 22:48
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2023

@llvm/pr-subscribers-backend-x86

Author: Arthur Eubanks (aeubanks)

Changes

Globals marked with the .lbss/.ldata/.lrodata should automatically be treated as large.
Do this regardless of the code model for consistency when mixing object files compiled with different code models.

Basically the opposite of #70748.

Example in the wild:
https://codebrowser.dev/qt5/qtbase/src/testlib/qtestcase.cpp.html#1664


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

2 Files Affected:

  • (modified) llvm/lib/Target/TargetMachine.cpp (+9-7)
  • (modified) llvm/test/CodeGen/X86/code-model-elf-sections.ll (+20)
diff --git a/llvm/lib/Target/TargetMachine.cpp b/llvm/lib/Target/TargetMachine.cpp
index f7096b708b39d..58a785434f659 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -43,9 +43,6 @@ bool TargetMachine::isLargeGlobalObject(const GlobalObject *GO) const {
   if (getTargetTriple().getArch() != Triple::x86_64)
     return false;
 
-  if (getCodeModel() != CodeModel::Medium && getCodeModel() != CodeModel::Large)
-    return false;
-
   if (isa<Function>(GO))
     return getCodeModel() == CodeModel::Large;
 
@@ -55,10 +52,10 @@ bool TargetMachine::isLargeGlobalObject(const GlobalObject *GO) const {
     return false;
 
   // Allowing large metadata sections in the presence of an explicit section is
-  // useful, even if GCC does not allow them. However, we should not mark
-  // certain well-known prefixes as large, because it would make the whole
-  // output section large and cause the linker to move it, which is almost
-  // always undesired.
+  // useful, even if GCC does not allow them. However, we should properly mark
+  // well-known section name prefixes as small/large, because otherwise the
+  // output section may have the wrong section flags and the linker will lay it
+  // out in an unexpected way.
   StringRef Name = GV->getSection();
   auto IsPrefix = [&](StringRef Prefix) {
     StringRef S = Name;
@@ -66,6 +63,11 @@ bool TargetMachine::isLargeGlobalObject(const GlobalObject *GO) const {
   };
   if (IsPrefix(".bss") || IsPrefix(".data") || IsPrefix(".rodata"))
     return false;
+  if (IsPrefix(".lbss") || IsPrefix(".ldata") || IsPrefix(".lrodata"))
+    return true;
+
+  if (getCodeModel() != CodeModel::Medium && getCodeModel() != CodeModel::Large)
+    return false;
 
   const DataLayout &DL = GV->getParent()->getDataLayout();
   uint64_t Size = DL.getTypeSizeInBits(GV->getValueType()) / 8;
diff --git a/llvm/test/CodeGen/X86/code-model-elf-sections.ll b/llvm/test/CodeGen/X86/code-model-elf-sections.ll
index 0b99a70c4044a..f72880e172bdc 100644
--- a/llvm/test/CodeGen/X86/code-model-elf-sections.ll
+++ b/llvm/test/CodeGen/X86/code-model-elf-sections.ll
@@ -21,9 +21,14 @@
 ; SMALL: .data {{.*}} WA {{.*}}
 ; SMALL: .data.x {{.*}} WA {{.*}}
 ; SMALL: .data0 {{.*}} WA {{.*}}
+; SMALL: .ldata {{.*}} WAl {{.*}}
+; SMALL: .ldata.x {{.*}} WAl {{.*}}
+; SMALL: .ldata0 {{.*}} WA {{.*}}
 ; SMALL: foo {{.*}} WA {{.*}}
 ; SMALL: .bss {{.*}} WA {{.*}}
+; SMALL: .lbss {{.*}} WAl {{.*}}
 ; SMALL: .rodata {{.*}} A {{.*}}
+; SMALL: .lrodata {{.*}} Al {{.*}}
 ; SMALL: .data.rel.ro {{.*}} WA {{.*}}
 ; SMALL: .tbss {{.*}} WAT {{.*}}
 ; SMALL: .tdata {{.*}} WAT {{.*}}
@@ -31,9 +36,14 @@
 ; SMALL-DS: .data {{.*}} WA {{.*}}
 ; SMALL-DS: .data.x {{.*}} WA {{.*}}
 ; SMALL-DS: .data0 {{.*}} WA {{.*}}
+; SMALL-DS: .ldata {{.*}} WAl {{.*}}
+; SMALL-DS: .ldata.x {{.*}} WAl {{.*}}
+; SMALL-DS: .ldata0 {{.*}} WA {{.*}}
 ; SMALL-DS: .data.data {{.*}} WA {{.*}}
 ; SMALL-DS: foo {{.*}} WA {{.*}}
+; SMALL-DS: .lbss {{.*}} WAl {{.*}}
 ; SMALL-DS: .bss.bss {{.*}} WA {{.*}}
+; SMALL-DS: .lrodata {{.*}} Al {{.*}}
 ; SMALL-DS: .rodata.rodata {{.*}} A {{.*}}
 ; SMALL-DS: .data.rel.ro.relro {{.*}} WA {{.*}}
 ; SMALL-DS: .tbss.tbss {{.*}} WAT {{.*}}
@@ -43,6 +53,8 @@
 ; LARGE: .data.x {{.*}} WA {{.*}}
 ; LARGE: .data0 {{.*}} WAl {{.*}}
 ; LARGE: .ldata {{.*}} WAl {{.*}}
+; LARGE: .ldata.x {{.*}} WAl {{.*}}
+; LARGE: .ldata0 {{.*}} WAl {{.*}}
 ; LARGE: foo {{.*}} WAl {{.*}}
 ; LARGE: .bss {{.*}} WA {{.*}}
 ; LARGE: .lbss {{.*}} WAl {{.*}}
@@ -55,6 +67,9 @@
 ; LARGE-DS: .data {{.*}} WA {{.*}}
 ; LARGE-DS: .data.x {{.*}} WA {{.*}}
 ; LARGE-DS: .data0 {{.*}} WAl {{.*}}
+; LARGE-DS: .ldata {{.*}} WAl {{.*}}
+; LARGE-DS: .ldata.x {{.*}} WAl {{.*}}
+; LARGE-DS: .ldata0 {{.*}} WAl {{.*}}
 ; LARGE-DS: .ldata.data {{.*}} WAl {{.*}}
 ; LARGE-DS: foo {{.*}} WAl {{.*}}
 ; LARGE-DS: .bss {{.*}} WA {{.*}}
@@ -71,11 +86,16 @@ target triple = "x86_64--linux"
 @data_with_explicit_section = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section ".data"
 @data_with_explicit_section2 = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section ".data.x"
 @data_with_explicit_section0 = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section ".data0"
+@ldata_with_explicit_section = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section ".ldata"
+@ldata_with_explicit_section2 = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section ".ldata.x"
+@ldata_with_explicit_section0 = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section ".ldata0"
 @data = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0]
 @foo_with_explicit_section = internal global [10 x i64] [i64 1, i64 2, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0, i64 0], section "foo"
 @bss_with_explicit_section = internal global [10 x i64] zeroinitializer, section ".bss"
+@lbss_with_explicit_section = internal global [10 x i64] zeroinitializer, section ".lbss"
 @bss = internal global [10 x i64] zeroinitializer
 @rodata_with_explicit_section = internal constant [10 x i64] zeroinitializer, section ".rodata"
+@lrodata_with_explicit_section = internal constant [10 x i64] zeroinitializer, section ".lrodata"
 @rodata = internal constant [10 x i64] zeroinitializer
 @relro = internal constant [10 x ptr] [ptr @func, ptr @func, ptr @func, ptr @func, ptr @func, ptr @func, ptr @func, ptr @func, ptr @func, ptr @func]
 @tbss = internal thread_local global [10 x i64] zeroinitializer

@aeubanks aeubanks merged commit 323451a into llvm:main Dec 5, 2023
@aeubanks aeubanks deleted the explicitlsection branch December 5, 2023 16:26
aeubanks added a commit that referenced this pull request Dec 13, 2023
…wn large section name (#74381)"

This reverts commit 323451a.

Code with these section names in the wild doesn't compile because
support for large globals in the small code model is not complete yet.
aeubanks added a commit that referenced this pull request Dec 21, 2023
…own large section name (#74381)"

This reverts commit 19fff85.

Now that explicit large globals are handled properly in the small code model.
Zentrik added a commit to Zentrik/llvm-project that referenced this pull request Jun 30, 2024
… well-known large section name (llvm#74381)""

This reverts commit 7433b1c.
Zentrik added a commit to Zentrik/llvm-project that referenced this pull request Jun 30, 2024
…well-known large section name (llvm#74381)""

This reverts commit 19fff85.
Zentrik added a commit to Zentrik/llvm-project that referenced this pull request Jun 30, 2024
Zentrik added a commit to Zentrik/llvm-project that referenced this pull request Jul 7, 2024
… well-known large section name (llvm#74381)""

This reverts commit 7433b1c.

Revert "Revert "[X86] Set SHF_X86_64_LARGE for globals with explicit well-known large section name (llvm#74381)""

This reverts commit 19fff85.

Revert "[X86] Set SHF_X86_64_LARGE for globals with explicit well-known large section name (llvm#74381)"

This reverts commit 323451a.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants