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 #87838

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

aeubanks
Copy link
Contributor

@aeubanks aeubanks commented Apr 5, 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.

…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.
@aeubanks aeubanks requested review from rnk and MaskRay April 5, 2024 22:56
@llvmbot
Copy link
Member

llvmbot commented Apr 5, 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.


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

2 Files Affected:

  • (modified) llvm/lib/Target/TargetMachine.cpp (+14-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 a7fe329b064ee1..61abb85b6458f6 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -51,9 +51,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 +84,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 016c9a4d7b8390..66a6fd37675427 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 merged commit e27c373 into llvm:main Apr 8, 2024
5 of 6 checks passed
@aeubanks aeubanks deleted the large-text-explicit-section branch April 8, 2024 22:40
aeubanks added a commit that referenced this pull request Apr 8, 2024
… small/large (#87838)"

This reverts commit e27c373.

Breaks ExecutionEngine/MCJIT/test-global-ctors.ll on windows, e.g. https://lab.llvm.org/buildbot/#/builders/117/builds/18749.
aeubanks added a commit to aeubanks/llvm-project that referenced this pull request Apr 9, 2024
…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.
aeubanks added a commit that referenced this pull request Apr 16, 2024
…arge (#88172)

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.
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