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] With large code model, put functions into .ltext with large section flag #73037

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

aeubanks
Copy link
Contributor

So that when mixing small and large text, large text stays out of the way of the rest of the binary.

This is useful for mixing precompiled small code model object files and built-from-source large code model binaries so that the the text sections don't get merged.

…tion flag

So that when mixing small and large text, large text stays out of the way of the rest of the binary.

This is useful for mixing precompiled small code model object files and built-from-source large code model binaries so that the the text sections don't get merged.
@llvmbot
Copy link
Member

llvmbot commented Nov 21, 2023

@llvm/pr-subscribers-backend-x86

Author: Arthur Eubanks (aeubanks)

Changes

So that when mixing small and large text, large text stays out of the way of the rest of the binary.

This is useful for mixing precompiled small code model object files and built-from-source large code model binaries so that the the text sections don't get merged.


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

7 Files Affected:

  • (modified) llvm/include/llvm/Target/TargetMachine.h (+1-1)
  • (modified) llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp (+4-11)
  • (modified) llvm/lib/Target/TargetMachine.cpp (+10-2)
  • (modified) llvm/lib/Target/X86/X86Subtarget.cpp (+7-26)
  • (added) llvm/test/CodeGen/X86/code-model-elf-text-sections.ll (+25)
  • (modified) llvm/test/CodeGen/X86/pcsections.ll (+11-11)
  • (modified) llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll (+1-1)
diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
index c1d05b25ea21f8a..4830ecbe1cd6349 100644
--- a/llvm/include/llvm/Target/TargetMachine.h
+++ b/llvm/include/llvm/Target/TargetMachine.h
@@ -239,7 +239,7 @@ class TargetMachine {
   void setCodeModel(CodeModel::Model CM) { CMModel = CM; }
 
   void setLargeDataThreshold(uint64_t LDT) { LargeDataThreshold = LDT; }
-  bool isLargeData(const GlobalVariable *GV) const;
+  bool isLargeGlobalObject(const GlobalObject *GO) const;
 
   bool isPositionIndependent() const;
 
diff --git a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index f3ba380818901ca..143a4951c1361b7 100644
--- a/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -616,7 +616,7 @@ static unsigned getEntrySizeForKind(SectionKind Kind) {
 /// DataSections.
 static StringRef getSectionPrefixForGlobal(SectionKind Kind, bool IsLarge) {
   if (Kind.isText())
-    return ".text";
+    return IsLarge ? ".ltext" : ".text";
   if (Kind.isReadOnly())
     return IsLarge ? ".lrodata" : ".rodata";
   if (Kind.isBSS())
@@ -650,10 +650,7 @@ getELFSectionNameForGlobal(const GlobalObject *GO, SectionKind Kind,
     Name = ".rodata.cst";
     Name += utostr(EntrySize);
   } else {
-    bool IsLarge = false;
-    if (auto *GV = dyn_cast<GlobalVariable>(GO))
-      IsLarge = TM.isLargeData(GV);
-    Name = getSectionPrefixForGlobal(Kind, IsLarge);
+    Name = getSectionPrefixForGlobal(Kind, TM.isLargeGlobalObject(GO));
   }
 
   bool HasPrefix = false;
@@ -773,12 +770,8 @@ getGlobalObjectInfo(const GlobalObject *GO, const TargetMachine &TM) {
     Group = C->getName();
     IsComdat = C->getSelectionKind() == Comdat::Any;
   }
-  if (auto *GV = dyn_cast<GlobalVariable>(GO)) {
-    if (TM.isLargeData(GV)) {
-      assert(TM.getTargetTriple().getArch() == Triple::x86_64);
-      Flags |= ELF::SHF_X86_64_LARGE;
-    }
-  }
+  if (TM.isLargeGlobalObject(GO))
+    Flags |= ELF::SHF_X86_64_LARGE;
   return {Group, IsComdat, Flags};
 }
 
diff --git a/llvm/lib/Target/TargetMachine.cpp b/llvm/lib/Target/TargetMachine.cpp
index 1cba8cf8004bfb4..f7096b708b39de4 100644
--- a/llvm/lib/Target/TargetMachine.cpp
+++ b/llvm/lib/Target/TargetMachine.cpp
@@ -39,13 +39,21 @@ TargetMachine::TargetMachine(const Target &T, StringRef DataLayoutString,
 
 TargetMachine::~TargetMachine() = default;
 
-bool TargetMachine::isLargeData(const GlobalVariable *GV) const {
-  if (getTargetTriple().getArch() != Triple::x86_64 || GV->isThreadLocal())
+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;
+
+  auto *GV = cast<GlobalVariable>(GO);
+
+  if (GV->isThreadLocal())
+    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
diff --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp
index 085fdafa6b9f2c4..688d8ece6abcb91 100644
--- a/llvm/lib/Target/X86/X86Subtarget.cpp
+++ b/llvm/lib/Target/X86/X86Subtarget.cpp
@@ -83,32 +83,13 @@ X86Subtarget::classifyLocalReference(const GlobalValue *GV) const {
   if (is64Bit()) {
     // 64-bit ELF PIC local references may use GOTOFF relocations.
     if (isTargetELF()) {
-      switch (TM.getCodeModel()) {
-      // 64-bit small code model is simple: All rip-relative.
-      case CodeModel::Tiny:
-        llvm_unreachable("Tiny codesize model not supported on X86");
-      case CodeModel::Small:
-      case CodeModel::Kernel:
-        return X86II::MO_NO_FLAG;
-
-      // The large PIC code model uses GOTOFF.
-      case CodeModel::Large:
-        return X86II::MO_GOTOFF;
-
-      // Medium is a hybrid: RIP-rel for code and non-large data, GOTOFF for
-      // remaining DSO local data.
-      case CodeModel::Medium:
-        // Constant pool and jump table handling pass a nullptr to this
-        // function so we need to use isa_and_nonnull.
-        if (isa_and_nonnull<Function>(GV))
-          return X86II::MO_NO_FLAG; // All code is RIP-relative
-        if (auto *GVar = dyn_cast_or_null<GlobalVariable>(GV)) {
-          if (TM.isLargeData(GVar))
-            return X86II::MO_GOTOFF;
-        }
-        return X86II::MO_NO_FLAG;    // Local symbols use GOTOFF.
-      }
-      llvm_unreachable("invalid code model");
+      CodeModel::Model CM = TM.getCodeModel();
+      assert(CM != CodeModel::Tiny &&
+             "Tiny codesize model not supported on X86");
+      if (auto *GO = dyn_cast_or_null<GlobalObject>(GV))
+        return TM.isLargeGlobalObject(GO) ? X86II::MO_GOTOFF
+                                          : X86II::MO_NO_FLAG;
+      return CM == CodeModel::Large ? X86II::MO_GOTOFF : X86II::MO_NO_FLAG;
     }
 
     // Otherwise, this is either a RIP-relative reference or a 64-bit movabsq,
diff --git a/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll b/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll
new file mode 100644
index 000000000000000..016c9a4d7b8390e
--- /dev/null
+++ b/llvm/test/CodeGen/X86/code-model-elf-text-sections.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s -relocation-model=pic -filetype=obj -code-model=small -o %t
+; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=SMALL
+; RUN: llc < %s -relocation-model=pic -filetype=obj -code-model=medium -o %t
+; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=SMALL
+; RUN: llc < %s -relocation-model=pic -filetype=obj -code-model=large -o %t
+; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=LARGE
+
+; RUN: llc < %s -relocation-model=pic -filetype=obj -code-model=small -function-sections -o %t
+; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=SMALL-DS
+; RUN: llc < %s -relocation-model=pic -filetype=obj -code-model=medium -function-sections -o %t
+; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=SMALL-DS
+; RUN: llc < %s -relocation-model=pic -filetype=obj -code-model=large -function-sections -o %t
+; RUN: llvm-readelf -S %t | FileCheck %s --check-prefix=LARGE-DS
+
+; SMALL: .text {{.*}} AX {{.*}}
+; SMALL-DS: .text.func {{.*}} AX {{.*}}
+; LARGE: .ltext {{.*}} AXl {{.*}}
+; LARGE-DS: .ltext.func {{.*}} AXl {{.*}}
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux"
+
+define void @func() {
+  ret void
+}
diff --git a/llvm/test/CodeGen/X86/pcsections.ll b/llvm/test/CodeGen/X86/pcsections.ll
index 00c1aba18cb43f9..4fe70d93cf347be 100644
--- a/llvm/test/CodeGen/X86/pcsections.ll
+++ b/llvm/test/CodeGen/X86/pcsections.ll
@@ -19,12 +19,12 @@ define void @empty_no_aux() !pcsections !0 {
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    retq
 ; CHECK-NEXT:  .Lfunc_end0:
-; CHECK:       .section	section_no_aux,"awo",@progbits,.text
+; CHECK:       .section	section_no_aux,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base0:
 ; DEFCM-NEXT:  .long	.Lfunc_begin0-.Lpcsection_base0
 ; LARGE-NEXT:  .quad	.Lfunc_begin0-.Lpcsection_base0
 ; CHECK-NEXT:  .long	.Lfunc_end0-.Lfunc_begin0
-; CHECK-NEXT:  .text
+; CHECK-NEXT:  .{{l?}}text
 entry:
   ret void
 }
@@ -35,7 +35,7 @@ define void @empty_aux() !pcsections !1 {
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    retq
 ; CHECK-NEXT:  .Lfunc_end1:
-; CHECK:       .section	section_aux,"awo",@progbits,.text
+; CHECK:       .section	section_aux,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base1:
 ; DEFCM-NEXT:  .long	.Lfunc_begin1-.Lpcsection_base1
 ; LARGE-NEXT:  .quad	.Lfunc_begin1-.Lpcsection_base1
@@ -43,7 +43,7 @@ define void @empty_aux() !pcsections !1 {
 ; CHECK-NEXT:  .long	10
 ; CHECK-NEXT:  .long	20
 ; CHECK-NEXT:  .long	30
-; CHECK-NEXT:  .text
+; CHECK-NEXT:  .{{l?}}text
 entry:
   ret void
 }
@@ -56,22 +56,22 @@ define i64 @multiple() !pcsections !0 {
 ; CHECK-NEXT:    movq
 ; CHECK-NEXT:    retq
 ; CHECK-NEXT:  .Lfunc_end2:
-; CHECK:       .section	section_no_aux,"awo",@progbits,.text
+; CHECK:       .section	section_no_aux,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base2:
 ; DEFCM-NEXT:  .long	.Lfunc_begin2-.Lpcsection_base2
 ; LARGE-NEXT:  .quad	.Lfunc_begin2-.Lpcsection_base2
 ; CHECK-NEXT:  .long	.Lfunc_end2-.Lfunc_begin2
-; CHECK-NEXT:  .section	section_aux_42,"awo",@progbits,.text
+; CHECK-NEXT:  .section	section_aux_42,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base3:
 ; DEFCM-NEXT:  .long	.Lpcsection0-.Lpcsection_base3
 ; LARGE-NEXT:  .quad	.Lpcsection0-.Lpcsection_base3
 ; CHECK-NEXT:  .long	42
-; CHECK-NEXT:  .section	section_aux_21264,"awo",@progbits,.text
+; CHECK-NEXT:  .section	section_aux_21264,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base4:
 ; DEFCM-NEXT:  .long	.Lpcsection0-.Lpcsection_base4
 ; LARGE-NEXT:  .quad	.Lpcsection0-.Lpcsection_base4
 ; CHECK-NEXT:  .long	21264
-; CHECK-NEXT:  .text
+; CHECK-NEXT:  .{{l?}}text
 entry:
   %0 = load i64, ptr @bar, align 8, !pcsections !2
   ret i64 %0
@@ -79,7 +79,7 @@ entry:
 
 define void @multiple_uleb128() !pcsections !6 {
 ; CHECK-LABEL: multiple_uleb128:
-; CHECK:       .section	section_aux,"awo",@progbits,.text
+; CHECK:       .section	section_aux,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base5:
 ; DEFCM-NEXT:  .long	.Lfunc_begin3-.Lpcsection_base5
 ; LARGE-NEXT:  .quad	.Lfunc_begin3-.Lpcsection_base5
@@ -87,13 +87,13 @@ define void @multiple_uleb128() !pcsections !6 {
 ; CHECK-NEXT:  .byte	42
 ; CHECK-NEXT:  .ascii	"\345\216&"
 ; CHECK-NEXT:  .byte	255
-; CHECK-NEXT:  .section	section_aux_21264,"awo",@progbits,.text
+; CHECK-NEXT:  .section	section_aux_21264,"awo",@progbits,.{{l?}}text
 ; CHECK-NEXT:  .Lpcsection_base6:
 ; DEFCM-NEXT:  .long	.Lfunc_begin3-.Lpcsection_base6
 ; LARGE-NEXT:  .quad	.Lfunc_begin3-.Lpcsection_base6
 ; CHECK-NEXT:  .long	.Lfunc_end3-.Lfunc_begin3
 ; CHECK-NEXT:  .long	21264
-; CHECK-NEXT:  .text
+; CHECK-NEXT:  .{{l?}}text
 entry:
   ret void
 }
diff --git a/llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll b/llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll
index 0d5aba376080a3e..d7bc2dc117b7f70 100644
--- a/llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll
+++ b/llvm/test/ExecutionEngine/OrcLazy/debug-objects-elf-minimal.ll
@@ -44,7 +44,7 @@
 ; RUN:     --generate=__dump_jit_debug_objects %s | llvm-objdump --section-headers - | \
 ; RUN:     FileCheck --check-prefix=CHECK_LOAD_ADDR %s
 ;
-; CHECK_LOAD_ADDR-NOT: {{[0-9]*}} .text {{.*}} 0000000000000000 TEXT
+; CHECK_LOAD_ADDR-NOT: {{[0-9]*}} .ltext {{.*}} 0000000000000000 TEXT
 
 target triple = "x86_64-unknown-unknown-elf"
 

@aeubanks
Copy link
Contributor Author

still need to resolve the conversation in #70358, just putting up codegen side of this

@MaskRay
Copy link
Member

MaskRay commented Nov 22, 2023

On the linker side (I am mostly nervous about), you can use a linker script fragment for the experiment https://sourceware.org/binutils/docs/ld/Miscellaneous-Commands.html#index-INSERT

On the compiler side, I think we may need something like this. Some JIT large code model users may hard code the .text section name and the change could cause them a trouble, so an opt-out option may be needed.

@aeubanks aeubanks requested a review from lhames November 22, 2023 18:23
@aeubanks
Copy link
Contributor Author

@lhames are you aware of any issues with hardcoding the .text section name?

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

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

I think it would be reasonable to move forward and shake out any hidden JIT dependencies on large .text sections.

@aeubanks
Copy link
Contributor Author

sounds good, we can revert if anything shows up

@aeubanks aeubanks merged commit 38e4358 into llvm:main Nov 28, 2023
3 checks passed
@aeubanks aeubanks deleted the largetext2 branch November 28, 2023 20:55
@lhames
Copy link
Contributor

lhames commented Nov 28, 2023

@aeubanks JIT clients can look up sections by name (and we don't really give them a more structured way to identify sections) so it's possible that some clients are hard-coding searches for .text. As far as I know no in-tree code does that though. The risk seems low, and the fix is easy. I think it's reasonable to leave this in-tree and see what (if anything) shakes out.

Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Nov 30, 2023
Local branch amd-gfx 14c4d99 Merged main:02cbae4fe068 into amd-gfx:5118390a1fcc
Remote branch main 38e4358 [X86] With large code model, put functions into .ltext with large section flag (llvm#73037)
aeubanks added a commit that referenced this pull request Nov 30, 2023
…rge section flag (#73037)

So that when mixing small and large text, large text stays out of the
way of the rest of the binary.

This is useful for mixing precompiled small code model object files and
built-from-source large code model binaries so that the the text
sections don't get merged.

The reland fixes an issue where a function in the large code model would reference small data without GOTOFF.
gribozavr added a commit that referenced this pull request Dec 1, 2023
… with large section flag (#73037)"

This reverts commit 4bf8a68.

This commit seems to be breaking the semantics of the
ObjectFile::isSectionText method, which breaks numba/llvmlite bindings.
aeubanks added a commit that referenced this pull request Dec 1, 2023
…rge section flag (#73037)

So that when mixing small and large text, large text stays out of the
way of the rest of the binary.

This is useful for mixing precompiled small code model object files and
built-from-source large code model binaries so that the the text
sections don't get merged.

The reland fixes an issue where a function in the large code model would reference small data without GOTOFF.

This was incorrectly reverted in 76f78ec.
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 Jun 30, 2024
…o .ltext with large section flag (llvm#73037)""

This reverts commit 76f78ec.
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 Jun 30, 2024
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
… with large section flag (llvm#73037)"

This reverts commit d8a0439.

Revert "Revert "Reland [X86] With large code model, put functions into .ltext with large section flag (llvm#73037)""

This reverts commit 76f78ec.

Revert "Reland [X86] With large code model, put functions into .ltext with large section flag (llvm#73037)"

This reverts commit 4bf8a68.

Revert "Revert "[X86] With large code model, put functions into .ltext with large section flag (llvm#73037)""

This reverts commit d8d9394.

Revert "[X86] With large code model, put functions into .ltext with large section flag (llvm#73037)"

This reverts commit 38e4358.
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.

5 participants