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

release/18.x: [X86] Always use 64-bit relocations in no-PIC large code model (#89101) #89124

Merged
merged 1 commit into from
Apr 23, 2024

Conversation

llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Apr 17, 2024

Backport 6cea7c4

Requested by: @aeubanks

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Apr 17, 2024
@llvmbot
Copy link
Collaborator Author

llvmbot commented Apr 17, 2024

@MaskRay What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Apr 17, 2024

@llvm/pr-subscribers-backend-x86

Author: None (llvmbot)

Changes

Backport 6cea7c4

Requested by: @aeubanks


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelDAGToDAG.cpp (+3-4)
  • (modified) llvm/test/CodeGen/X86/code-model-elf.ll (+2-2)
diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 833f058253d880..553d338b77904a 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -2923,11 +2923,10 @@ bool X86DAGToDAGISel::selectAddr(SDNode *Parent, SDValue N, SDValue &Base,
 }
 
 bool X86DAGToDAGISel::selectMOV64Imm32(SDValue N, SDValue &Imm) {
-  // Cannot use 32 bit constants to reference objects in kernel code model.
-  // Cannot use 32 bit constants to reference objects in large PIC mode since
-  // GOTOFF is 64 bits.
+  // Cannot use 32 bit constants to reference objects in kernel/large code
+  // model.
   if (TM.getCodeModel() == CodeModel::Kernel ||
-      (TM.getCodeModel() == CodeModel::Large && TM.isPositionIndependent()))
+      TM.getCodeModel() == CodeModel::Large)
     return false;
 
   // In static codegen with small code model, we can get the address of a label
diff --git a/llvm/test/CodeGen/X86/code-model-elf.ll b/llvm/test/CodeGen/X86/code-model-elf.ll
index afcffb3a7adeda..b6634403dc1d05 100644
--- a/llvm/test/CodeGen/X86/code-model-elf.ll
+++ b/llvm/test/CodeGen/X86/code-model-elf.ll
@@ -346,7 +346,7 @@ define dso_local ptr @lea_forced_small_data() #0 {
 ;
 ; LARGE-STATIC-LABEL: lea_forced_small_data:
 ; LARGE-STATIC:       # %bb.0:
-; LARGE-STATIC-NEXT:    movl $forced_small_data, %eax
+; LARGE-STATIC-NEXT:    movabsq $forced_small_data, %rax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: lea_forced_small_data:
@@ -399,7 +399,7 @@ define dso_local i32 @load_forced_small_data() #0 {
 ;
 ; LARGE-STATIC-LABEL: load_forced_small_data:
 ; LARGE-STATIC:       # %bb.0:
-; LARGE-STATIC-NEXT:    movl $forced_small_data+8, %eax
+; LARGE-STATIC-NEXT:    movabsq $forced_small_data+8, %rax
 ; LARGE-STATIC-NEXT:    movl (%rax), %eax
 ; LARGE-STATIC-NEXT:    retq
 ;

@aeubanks
Copy link
Contributor

@tstellar looks like the version check is failing, how can I get this merged?

@tstellar
Copy link
Collaborator

@aeubanks I'll merge this once this PR lands: #89291

…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 tstellar merged commit a981a4f into llvm:release/18.x Apr 23, 2024
7 of 8 checks passed
@tstellar
Copy link
Collaborator

tstellar commented May 1, 2024

Hi @aeubanks (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@aeubanks
Copy link
Contributor

aeubanks commented May 2, 2024

this is mostly a fix to revert back to previous behavior in certain cases, so probably not worth a release note

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants