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] Fix lowering TLS under darwin large code model #80907

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

aeubanks
Copy link
Contributor

@aeubanks aeubanks commented Feb 6, 2024

OpFlag and WrapperKind should be chosen consistently with each other in regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes #80831.

OpFlag and WrapperKind should be chosen consistently with each other in regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes llvm#80831.
@aeubanks aeubanks requested a review from rnk February 6, 2024 21:48
@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2024

@llvm/pr-subscribers-backend-x86

Author: Arthur Eubanks (aeubanks)

Changes

OpFlag and WrapperKind should be chosen consistently with each other in regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes #80831.


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

2 Files Affected:

  • (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+6-4)
  • (modified) llvm/test/CodeGen/X86/tls-models.ll (+1)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 9a9b0680acc20..b5b76c66c2e49 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -18710,16 +18710,18 @@ X86TargetLowering::LowerGlobalTLSAddress(SDValue Op, SelectionDAG &DAG) const {
   if (Subtarget.isTargetDarwin()) {
     // Darwin only has one model of TLS.  Lower to that.
     unsigned char OpFlag = 0;
-    unsigned WrapperKind = Subtarget.isPICStyleRIPRel() ?
-                           X86ISD::WrapperRIP : X86ISD::Wrapper;
+    unsigned WrapperKind = 0;
 
     // In PIC mode (unless we're in RIPRel PIC mode) we add an offset to the
     // global base reg.
     bool PIC32 = PositionIndependent && !Subtarget.is64Bit();
-    if (PIC32)
+    if (PIC32) {
       OpFlag = X86II::MO_TLVP_PIC_BASE;
-    else
+      WrapperKind = X86ISD::Wrapper;
+    } else {
       OpFlag = X86II::MO_TLVP;
+      WrapperKind = X86ISD::WrapperRIP;
+    }
     SDLoc DL(Op);
     SDValue Result = DAG.getTargetGlobalAddress(GA->getGlobal(), DL,
                                                 GA->getValueType(0),
diff --git a/llvm/test/CodeGen/X86/tls-models.ll b/llvm/test/CodeGen/X86/tls-models.ll
index fc8e302338d96..8de9de15a5f06 100644
--- a/llvm/test/CodeGen/X86/tls-models.ll
+++ b/llvm/test/CodeGen/X86/tls-models.ll
@@ -5,6 +5,7 @@
 
 ; Darwin always uses the same model.
 ; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck -check-prefix=DARWIN %s
+; RUN: llc < %s -mtriple=x86_64-apple-darwin -code-model=large | FileCheck -check-prefix=DARWIN %s
 
 @external_gd = external thread_local global i32
 @internal_gd = internal thread_local global i32 42

@nikic
Copy link
Contributor

nikic commented Feb 7, 2024

Confirmed that this patch fixes my original issue as well.

@aeubanks aeubanks merged commit 5a83bcc into llvm:main Feb 7, 2024
5 of 6 checks passed
@aeubanks aeubanks deleted the darwintlslarge branch February 7, 2024 17:16
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 8, 2024
OpFlag and WrapperKind should be chosen consistently with each other in
regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes llvm#80831.

(cherry picked from commit 5a83bcc)
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Feb 9, 2024
OpFlag and WrapperKind should be chosen consistently with each other in
regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes llvm#80831.

(cherry picked from commit 5a83bcc)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
OpFlag and WrapperKind should be chosen consistently with each other in
regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes llvm#80831.

(cherry picked from commit 5a83bcc)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
OpFlag and WrapperKind should be chosen consistently with each other in
regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes llvm#80831.

(cherry picked from commit 5a83bcc)
tstellar pushed a commit to tstellar/llvm-project that referenced this pull request Feb 14, 2024
OpFlag and WrapperKind should be chosen consistently with each other in
regards to PIC, otherwise we hit asserts later on.

Broken by c04a05d.

Fixes llvm#80831.

(cherry picked from commit 5a83bcc)
@pointhex pointhex mentioned this pull request May 7, 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.

TLS with large code model on x86_64-apple-darwin asserts
4 participants