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

[X86] Change how we treat functions with explicit sections as small/large #88172

Merged
merged 2 commits into from
Apr 16, 2024

Conversation

aeubanks
Copy link
Contributor

@aeubanks aeubanks commented Apr 9, 2024

Following #78348, we should treat functions with an explicit section as small, unless the section name is (or has the prefix) ".ltext".

Clang emits global initializers into a ".text.startup" section on Linux. If we mix small/medium code model object files with large code model object files, we'll end up mixing sections with and without the large section flag.

Reland of #87838 with a check for non-ELF platforms in TargetMachine::isLargeGlobalValue(), otherwise MCJIT on Windows tests fail.

…arge

Following llvm#78348, we should treat functions with an explicit section as
small, unless the section name is (or has the prefix) ".ltext".

Clang emits global initializers into a ".text.startup" section on Linux.
If we mix small/medium code model object files with large code model
object files, we'll end up mixing sections with and without the large
section flag.

Reland of llvm#87838 with a check for non-ELF platforms in
TargetMachine::isLargeGlobalValue(), otherwise MCJIT on Windows tests
fail.
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2024

@llvm/pr-subscribers-backend-x86

Author: Arthur Eubanks (aeubanks)

Changes

Following #78348, we should treat functions with an explicit section as small, unless the section name is (or has the prefix) ".ltext".

Clang emits global initializers into a ".text.startup" section on Linux. If we mix small/medium code model object files with large code model object files, we'll end up mixing sections with and without the large section flag.

Reland of #87838 with a check for non-ELF platforms in TargetMachine::isLargeGlobalValue(), otherwise MCJIT on Windows tests fail.


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

2 Files Affected:

  • (modified) llvm/lib/Target/TargetMachine.cpp (+17-6)
  • (modified) llvm/test/CodeGen/X86/code-model-elf-text-sections.ll (+23)
diff --git a/llvm/lib/Target/TargetMachine.cpp b/llvm/lib/Target/TargetMachine.cpp
index a7fe329b064ee16..4ae6a468df21f36 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -43,6 +43,9 @@ bool TargetMachine::isLargeGlobalValue(const GlobalValue *GVal) const {
   if (getTargetTriple().getArch() != Triple::x86_64)
     return false;
 
+  if (!getTargetTriple().isOSBinFormatELF())
+    return getCodeModel() == CodeModel::Large;
+
   auto *GO = GVal->getAliaseeObject();
 
   // Be conservative if we can't find an underlying GlobalObject.
@@ -51,9 +54,20 @@ bool TargetMachine::isLargeGlobalValue(const GlobalValue *GVal) const {
 
   auto *GV = dyn_cast<GlobalVariable>(GO);
 
+  auto IsPrefix = [](StringRef Name, StringRef Prefix) {
+    return Name.consume_front(Prefix) && (Name.empty() || Name[0] == '.');
+  };
+
   // Functions/GlobalIFuncs are only large under the large code model.
-  if (!GV)
+  if (!GV) {
+    // Handle explicit sections as we do for GlobalVariables with an explicit
+    // section, see comments below.
+    if (GO->hasSection()) {
+      StringRef Name = GO->getSection();
+      return IsPrefix(Name, ".ltext");
+    }
     return getCodeModel() == CodeModel::Large;
+  }
 
   if (GV->isThreadLocal())
     return false;
@@ -73,11 +87,8 @@ bool TargetMachine::isLargeGlobalValue(const GlobalValue *GVal) const {
   // data sections. The code model attribute overrides this above.
   if (GV->hasSection()) {
     StringRef Name = GV->getSection();
-    auto IsPrefix = [&](StringRef Prefix) {
-      StringRef S = Name;
-      return S.consume_front(Prefix) && (S.empty() || S[0] == '.');
-    };
-    return IsPrefix(".lbss") || IsPrefix(".ldata") || IsPrefix(".lrodata");
+    return IsPrefix(Name, ".lbss") || IsPrefix(Name, ".ldata") ||
+           IsPrefix(Name, ".lrodata");
   }
 
   // Respect large data threshold for medium and large code models.
diff --git a/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll b/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll
index 016c9a4d7b8390e..66a6fd376754275 100644
--- a/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll
+++ b/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll
@@ -13,9 +13,20 @@
 ; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=LARGE-DS
 
 ; SMALL: .text {{.*}} AX {{.*}}
+; SMALL: .ltext {{.*}} AXl {{.*}}
+; SMALL: .ltext.2 {{.*}} AXl {{.*}}
+; SMALL: .foo {{.*}} AX {{.*}}
 ; SMALL-DS: .text.func {{.*}} AX {{.*}}
+; SMALL-DS: .ltext {{.*}} AXl {{.*}}
+; SMALL-DS: .ltext.2 {{.*}} AXl {{.*}}
+; SMALL-DS: .foo {{.*}} AX {{.*}}
 ; LARGE: .ltext {{.*}} AXl {{.*}}
+; LARGE: .ltext.2 {{.*}} AXl {{.*}}
+; LARGE: .foo {{.*}} AX {{.*}}
 ; LARGE-DS: .ltext.func {{.*}} AXl {{.*}}
+; LARGE-DS: .ltext {{.*}} AXl {{.*}}
+; LARGE-DS: .ltext.2 {{.*}} AXl {{.*}}
+; LARGE-DS: .foo {{.*}} AX {{.*}}
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64--linux"
@@ -23,3 +34,15 @@ target triple = "x86_64--linux"
 define void @func() {
   ret void
 }
+
+define void @ltext() section ".ltext" {
+  ret void
+}
+
+define void @ltext2() section ".ltext.2" {
+  ret void
+}
+
+define void @foo() section ".foo" {
+  ret void
+}

@aeubanks aeubanks requested review from MaskRay and rnk April 10, 2024 06:30
@aeubanks
Copy link
Contributor Author

ping

@@ -43,6 +43,9 @@ bool TargetMachine::isLargeGlobalValue(const GlobalValue *GVal) const {
if (getTargetTriple().getArch() != Triple::x86_64)
return false;

if (!getTargetTriple().isOSBinFormatELF())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a comment about what's going on here. Essentially, all the logic below is ELF-specific. The large code model is supported with other object file formats mainly for the benefit of JIT compilation, and for that we fall back to the global code model option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@aeubanks aeubanks merged commit 281d716 into llvm:main Apr 16, 2024
3 of 4 checks passed
@aeubanks aeubanks deleted the large-text-explicit-section2 branch April 16, 2024 19:51
aeubanks added a commit that referenced this pull request Apr 17, 2024
This matches other types of relocations, e.g. to constant pool. And
makes things more consistent with PIC large code model.

Some users of the large code model may not place small data in the lower
2GB of the address space (e.g.
ClangBuiltLinux/linux#2016), so just
unconditionally use 64-bit relocations in the large code model.

So now functions in a section not marked large will use 64-bit
relocations to reference everything when using the large code model.

This also fixes some lldb tests broken by #88172
(https://lab.llvm.org/buildbot/#/builders/68/builds/72458).
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 17, 2024
…89101)

This matches other types of relocations, e.g. to constant pool. And
makes things more consistent with PIC large code model.

Some users of the large code model may not place small data in the lower
2GB of the address space (e.g.
ClangBuiltLinux/linux#2016), so just
unconditionally use 64-bit relocations in the large code model.

So now functions in a section not marked large will use 64-bit
relocations to reference everything when using the large code model.

This also fixes some lldb tests broken by llvm#88172
(https://lab.llvm.org/buildbot/#/builders/68/builds/72458).

(cherry picked from commit 6cea7c4)
tstellar pushed a commit to llvmbot/llvm-project that referenced this pull request Apr 23, 2024
…89101)

This matches other types of relocations, e.g. to constant pool. And
makes things more consistent with PIC large code model.

Some users of the large code model may not place small data in the lower
2GB of the address space (e.g.
ClangBuiltLinux/linux#2016), so just
unconditionally use 64-bit relocations in the large code model.

So now functions in a section not marked large will use 64-bit
relocations to reference everything when using the large code model.

This also fixes some lldb tests broken by llvm#88172
(https://lab.llvm.org/buildbot/#/builders/68/builds/72458).

(cherry picked from commit 6cea7c4)
Zentrik pushed a commit to Zentrik/llvm-project that referenced this pull request Jun 30, 2024
…arge (llvm#88172)

Following llvm#78348, we should treat functions with an explicit section as
small, unless the section name is (or has the prefix) ".ltext".

Clang emits global initializers into a ".text.startup" section on Linux.
If we mix small/medium code model object files with large code model
object files, we'll end up mixing sections with and without the large
section flag.

Reland of llvm#87838 with a check for non-ELF platforms in
TargetMachine::isLargeGlobalValue(), otherwise MCJIT on Windows tests
fail.
giordano pushed a commit to JuliaLang/llvm-project that referenced this pull request Jun 30, 2024
…arge (llvm#88172)

Following llvm#78348, we should treat functions with an explicit section as
small, unless the section name is (or has the prefix) ".ltext".

Clang emits global initializers into a ".text.startup" section on Linux.
If we mix small/medium code model object files with large code model
object files, we'll end up mixing sections with and without the large
section flag.

Reland of llvm#87838 with a check for non-ELF platforms in
TargetMachine::isLargeGlobalValue(), otherwise MCJIT on Windows tests
fail.
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