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][ISel] Don't select MOV/ADD64ri32 for tglobaltlsaddr under large code models #77175

Closed
wants to merge 2 commits into from

Conversation

nmosier
Copy link
Contributor

@nmosier nmosier commented Jan 6, 2024

This patch fixes a bug (GitHub issue #77128) that caused the compiler to emit 32-bit (rather than 64-bit) relocations for TLS offsets under a medium/large code model for x86-64 targets, resulting in link-time errors.

The root cause of the bug is an X86 instruction selection pattern that errantly matches tglobaltlsaddr SDNodes to target MOV64ri32/ADD64ri32 SDNodes during the Select phase of instruction selection, regardless of the code model.

This patch adds the requirement Requires<[NearData]> to both X86 selection patterns, ensuring that they only match when the code model is not large. It also adds a new test (llvm/test/CodeGen/X86/tls-largecode.ll) to ensure the patterns are not matched for medium/large code models.

…code models

This patch fixes a bug (GitHub issue llvm#77128) that caused the compiler
to emit 32-bit (rather than 64-bit) relocations for TLS offsets under a
medium/large code model for x86-64 targets, resulting in link-time errors.

The root cause of the bug is an X86 instruction selection pattern that
errantly matches tglobaltlsaddr SDNodes to target MOV64ri32/ADD64ri32 SDNodes
during the Select phase of instruction selection, regardless of the code
model.

This patch adds the requirement `Requires<[NearData]>` to both X86 selection
patterns, ensuring that they only match when the code model is tiny/small/kernel.
It also adds a new test (llvm/test/CodeGen/X86/tls-largecode.ll) to ensure the patterns are not matched for medium/large code models.
Copy link

github-actions bot commented Jan 6, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 6, 2024

@llvm/pr-subscribers-backend-x86

Author: Nicholas Mosier (nmosier)

Changes

This patch fixes a bug (GitHub issue #77128) that caused the compiler to emit 32-bit (rather than 64-bit) relocations for TLS offsets under a medium/large code model for x86-64 targets, resulting in link-time errors.

The root cause of the bug is an X86 instruction selection pattern that errantly matches tglobaltlsaddr SDNodes to target MOV64ri32/ADD64ri32 SDNodes during the Select phase of instruction selection, regardless of the code model.

This patch adds the requirement Requires&lt;[NearData]&gt; to both X86 selection patterns, ensuring that they only match when the code model is not large. It also adds a new test (llvm/test/CodeGen/X86/tls-largecode.ll) to ensure the patterns are not matched for medium/large code models.


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86InstrCompiler.td (+2-2)
  • (added) llvm/test/CodeGen/X86/tls-codemodels.ll (+36)
diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index c77c77ee4a3eeb..67fb593e391d34 100644
--- a/llvm/lib/Target/X86/X86InstrCompiler.td
+++ b/llvm/lib/Target/X86/X86InstrCompiler.td
@@ -1267,10 +1267,10 @@ def : Pat<(i64 (X86RecoverFrameAlloc mcsym:$dst)), (MOV64ri mcsym:$dst)>;
 // tls has some funny stuff here...
 // This corresponds to movabs $foo@tpoff, %rax
 def : Pat<(i64 (X86Wrapper tglobaltlsaddr :$dst)),
-          (MOV64ri32 tglobaltlsaddr :$dst)>;
+          (MOV64ri32 tglobaltlsaddr :$dst)>, Requires<[NearData]>;
 // This corresponds to add $foo@tpoff, %rax
 def : Pat<(add GR64:$src1, (X86Wrapper tglobaltlsaddr :$dst)),
-          (ADD64ri32 GR64:$src1, tglobaltlsaddr :$dst)>;
+          (ADD64ri32 GR64:$src1, tglobaltlsaddr :$dst)>, Requires<[NearData]>;
 
 
 // Direct PC relative function call for small code model. 32-bit displacement
diff --git a/llvm/test/CodeGen/X86/tls-codemodels.ll b/llvm/test/CodeGen/X86/tls-codemodels.ll
new file mode 100644
index 00000000000000..644e07300f3928
--- /dev/null
+++ b/llvm/test/CodeGen/X86/tls-codemodels.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -code-model=small | FileCheck %s --check-prefix=CHECK-SMALL
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -code-model=kernel | FileCheck %s --check-prefix=CHECK-KERNEL
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -code-model=medium | FileCheck %s --check-prefix=CHECK-MEDIUM
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -code-model=large | FileCheck %s --check-prefix=CHECK-LARGE
+
+@x = dso_local thread_local global i32 0, align 4
+
+define dso_local void @test() local_unnamed_addr {
+; CHECK-SMALL-LABEL: test:
+; CHECK-SMALL:       # %bb.0: # %entry
+; CHECK-SMALL-NEXT:    movl $0, %fs:x@TPOFF
+; CHECK-SMALL-NEXT:    retq
+;
+; CHECK-KERNEL-LABEL: test:
+; CHECK-KERNEL:       # %bb.0: # %entry
+; CHECK-KERNEL-NEXT:    movl $0, %fs:x@TPOFF
+; CHECK-KERNEL-NEXT:    retq
+;
+; CHECK-MEDIUM-LABEL: test:
+; CHECK-MEDIUM:       # %bb.0: # %entry
+; CHECK-MEDIUM-NEXT:    movl $0, %fs:x@TPOFF
+; CHECK-MEDIUM-NEXT:    retq
+;
+; CHECK-LARGE-LABEL: test:
+; CHECK-LARGE:       # %bb.0: # %entry
+; CHECK-LARGE-NEXT:    movabsq $x@TPOFF, %rax
+; CHECK-LARGE-NEXT:    movl $0, %fs:(%rax)
+; CHECK-LARGE-NEXT:    retq
+entry:
+  %0 = call align 4 ptr @llvm.threadlocal.address.p0(ptr align 4 @x)
+  store i32 0, ptr %0, align 4
+  ret void
+}
+
+declare nonnull ptr @llvm.threadlocal.address.p0(ptr nonnull)

@nmosier nmosier closed this Jan 6, 2024
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.

None yet

2 participants