Skip to content

Conversation

aemerson
Copy link
Contributor

@aemerson aemerson commented Aug 30, 2025

When using no-movt we don't use the pcrel version of the literal load.
This change also unifies logic with the ARM version of this function as well,
which has:

  if (!Subtarget.useMovt() || ForceELFGOTPIC) {
    // For ELF non-PIC, use GOT PIC code sequence as well because R_ARM_GOT_ABS
    // does not have assembler support.
    if (TM.isPositionIndependent() || ForceELFGOTPIC)
      expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_pcrel, ARM::LDRi12);
    else
      expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_abs, ARM::LDRi12);
    return;
  }

rdar://138334512

@aemerson aemerson marked this pull request as ready for review August 30, 2025 21:32
Copy link
Contributor Author

aemerson commented Aug 30, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Aug 30, 2025

@llvm/pr-subscribers-backend-arm

Author: Amara Emerson (aemerson)

Changes

When using no-movt we don't use the pcrel version of the literal load.
This change also unifies logic with the ARM version of this function as well,
which has:

  if (!Subtarget.useMovt() || ForceELFGOTPIC) {
    // For ELF non-PIC, use GOT PIC code sequence as well because R_ARM_GOT_ABS
    // does not have assembler support.
    if (TM.isPositionIndependent() || ForceELFGOTPIC)
      expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_pcrel, ARM::LDRi12);
    else
      expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_abs, ARM::LDRi12);
    return;
  }

rdar://138334512


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

2 Files Affected:

  • (modified) llvm/lib/Target/ARM/Thumb2InstrInfo.cpp (+7-3)
  • (modified) llvm/test/CodeGen/ARM/stack-guard-nomovt.ll (+56-20)
diff --git a/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp b/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
index 8b254fafc438e..7b9e4f0a55143 100644
--- a/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ b/llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -263,11 +263,15 @@ void Thumb2InstrInfo::expandLoadStackGuard(
 
   const auto *GV = cast<GlobalValue>((*MI->memoperands_begin())->getValue());
   const ARMSubtarget &Subtarget = MF.getSubtarget<ARMSubtarget>();
+  bool IsPIC = MF.getTarget().isPositionIndependent();
   if (Subtarget.isTargetELF() && !GV->isDSOLocal())
     expandLoadStackGuardBase(MI, ARM::t2LDRLIT_ga_pcrel, ARM::t2LDRi12);
-  else if (!Subtarget.useMovt())
-    expandLoadStackGuardBase(MI, ARM::tLDRLIT_ga_abs, ARM::t2LDRi12);
-  else if (MF.getTarget().isPositionIndependent())
+  else if (!Subtarget.useMovt()) {
+    expandLoadStackGuardBase(
+        MI, IsPIC ? ARM::t2LDRLIT_ga_pcrel : ARM::tLDRLIT_ga_abs,
+        ARM::t2LDRi12);
+  }
+  else if (IsPIC)
     expandLoadStackGuardBase(MI, ARM::t2MOV_ga_pcrel, ARM::t2LDRi12);
   else
     expandLoadStackGuardBase(MI, ARM::t2MOVi32imm, ARM::t2LDRi12);
diff --git a/llvm/test/CodeGen/ARM/stack-guard-nomovt.ll b/llvm/test/CodeGen/ARM/stack-guard-nomovt.ll
index 6802dabfda87a..2003fd747f43d 100644
--- a/llvm/test/CodeGen/ARM/stack-guard-nomovt.ll
+++ b/llvm/test/CodeGen/ARM/stack-guard-nomovt.ll
@@ -1,27 +1,63 @@
-; RUN: llc -relocation-model=static -mattr=+no-movt < %s | FileCheck %s
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -relocation-model=static -mattr=+no-movt < %s | FileCheck %s --check-prefix=STATIC
+; RUN: llc -relocation-model=pic -mattr=+no-movt < %s | FileCheck %s --check-prefix=PIC
 
 target triple = "thumbv7a-linux-gnueabi"
 
 define i32 @test1() #0 {
-; CHECK-LABEL: test1:
-; CHECK:       @ %bb.0:
-; CHECK-NEXT:    push	{r7, lr}
-; CHECK-NEXT:    sub.w	sp, sp, #1032
-; CHECK-NEXT:    ldr	r0, .LCPI0_0
-; CHECK-NEXT:    ldr	r0, [r0]
-; CHECK-NEXT:    str.w	r0, [sp, #1028]
-; CHECK-NEXT:    add	r0, sp, #4
-; CHECK-NEXT:    bl	foo
-; CHECK-NEXT:    ldr.w	r0, [sp, #1028]
-; CHECK-NEXT:    ldr	r1, .LCPI0_0
-; CHECK-NEXT:    ldr	r1, [r1]
-; CHECK-NEXT:    cmp	r1, r0
-; CHECK-NEXT:    ittt	eq
-; CHECK-NEXT:    moveq	r0, #0
-; CHECK-NEXT:    addeq.w	sp, sp, #1032
-; CHECK-NEXT:    popeq	{r7, pc}
-; CHECK-NEXT:  .LBB0_1:
-; CHECK-NEXT:    bl __stack_chk_fail
+; STATIC-LABEL: test1:
+; STATIC:       @ %bb.0:
+; STATIC-NEXT:    push {r7, lr}
+; STATIC-NEXT:    sub.w sp, sp, #1032
+; STATIC-NEXT:    ldr r0, .LCPI0_0
+; STATIC-NEXT:    ldr r0, [r0]
+; STATIC-NEXT:    str.w r0, [sp, #1028]
+; STATIC-NEXT:    add r0, sp, #4
+; STATIC-NEXT:    bl foo
+; STATIC-NEXT:    ldr.w r0, [sp, #1028]
+; STATIC-NEXT:    ldr r1, .LCPI0_0
+; STATIC-NEXT:    ldr r1, [r1]
+; STATIC-NEXT:    cmp r1, r0
+; STATIC-NEXT:    ittt eq
+; STATIC-NEXT:    moveq r0, #0
+; STATIC-NEXT:    addeq.w sp, sp, #1032
+; STATIC-NEXT:    popeq {r7, pc}
+; STATIC-NEXT:  .LBB0_1:
+; STATIC-NEXT:    bl __stack_chk_fail
+; STATIC-NEXT:    .p2align 2
+; STATIC-NEXT:  @ %bb.2:
+; STATIC-NEXT:  .LCPI0_0:
+; STATIC-NEXT:    .long __stack_chk_guard
+;
+; PIC-LABEL: test1:
+; PIC:       @ %bb.0:
+; PIC-NEXT:    push {r7, lr}
+; PIC-NEXT:    sub.w sp, sp, #1032
+; PIC-NEXT:    ldr r0, .LCPI0_0
+; PIC-NEXT:  .LPC0_0:
+; PIC-NEXT:    add r0, pc
+; PIC-NEXT:    ldr r0, [r0]
+; PIC-NEXT:    str.w r0, [sp, #1028]
+; PIC-NEXT:    add r0, sp, #4
+; PIC-NEXT:    bl foo
+; PIC-NEXT:    ldr.w r0, [sp, #1028]
+; PIC-NEXT:    ldr r1, .LCPI0_1
+; PIC-NEXT:  .LPC0_1:
+; PIC-NEXT:    add r1, pc
+; PIC-NEXT:    ldr r1, [r1]
+; PIC-NEXT:    cmp r1, r0
+; PIC-NEXT:    ittt eq
+; PIC-NEXT:    moveq r0, #0
+; PIC-NEXT:    addeq.w sp, sp, #1032
+; PIC-NEXT:    popeq {r7, pc}
+; PIC-NEXT:  .LBB0_1:
+; PIC-NEXT:    bl __stack_chk_fail
+; PIC-NEXT:    .p2align 2
+; PIC-NEXT:  @ %bb.2:
+; PIC-NEXT:  .LCPI0_0:
+; PIC-NEXT:    .long __stack_chk_guard-(.LPC0_0+4)
+; PIC-NEXT:  .LCPI0_1:
+; PIC-NEXT:    .long __stack_chk_guard-(.LPC0_1+4)
   %a1 = alloca [256 x i32], align 4
   call void @foo(ptr %a1) #3
   ret i32 0

Copy link

github-actions bot commented Aug 30, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@aemerson aemerson changed the title Use t2LDRLIT_ga_pcrel for loading stack guards with no-movt in PIC mode. [ARM] Use t2LDRLIT_ga_pcrel for loading stack guards with no-movt in PIC mode. Aug 30, 2025
@aemerson aemerson force-pushed the users/aemerson/movt-pic branch from 887619b to c2f59d4 Compare August 30, 2025 21:48
@aemerson aemerson force-pushed the users/aemerson/movt-pic branch from c2f59d4 to 65becc4 Compare September 1, 2025 04:47
…PIC mode.

When using no-movt we don't use the pcrel version of the literal load.
This change also unifies logic with the ARM version of this function as well,
which has:
```
  if (!Subtarget.useMovt() || ForceELFGOTPIC) {
    // For ELF non-PIC, use GOT PIC code sequence as well because R_ARM_GOT_ABS
    // does not have assembler support.
    if (TM.isPositionIndependent() || ForceELFGOTPIC)
      expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_pcrel, ARM::LDRi12);
    else
      expandLoadStackGuardBase(MI, ARM::LDRLIT_ga_abs, ARM::LDRi12);
    return;
  }
```

rdar://138334512
@aemerson aemerson force-pushed the users/aemerson/movt-pic branch from 65becc4 to 0c955f4 Compare September 1, 2025 04:48
@aemerson aemerson merged commit 64b9896 into main Sep 1, 2025
8 of 9 checks passed
@aemerson aemerson deleted the users/aemerson/movt-pic branch September 1, 2025 05:31
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.

4 participants