[AArch64] Add -mlr-for-calls-only to replace the now removed -ffixed-x30 flag. #98073

aemerson · 2024-07-08T20:29:40Z

This re-introduces the effective behaviour that was reverted in 7ad481e.

This time we're not using the same mechanism, exposing another reservation feature
that prevents only regalloc from using the register, but not for other required uses
like ABIs.

This also fixes a consequent issue with reserving LR, which is that frame lowering
was only adding live-in flags for non-reserved regs. This would cause issues later
since the outliner needs accurate flags to determine when LR needs to be preserved.

rdar://131313095

…x30 flag. This re-introduces the effective behaviour that was reverted in 7ad481e. This time we're not using the same mechanism, exposing another reservation feature that prevents only regalloc from using the register, but not for other required uses like ABIs. This also fixes a consequent issue with reserving LR, which is that frame lowering was only adding live-in flags for non-reserved regs. This would cause issues later since the outliner needs accurate flags to determine when LR needs to be preserved. rdar://131313095

llvmbot · 2024-07-08T20:30:09Z

@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-clang

Author: Amara Emerson (aemerson)

Changes

This re-introduces the effective behaviour that was reverted in 7ad481e.

This time we're not using the same mechanism, exposing another reservation feature
that prevents only regalloc from using the register, but not for other required uses
like ABIs.

This also fixes a consequent issue with reserving LR, which is that frame lowering
was only adding live-in flags for non-reserved regs. This would cause issues later
since the outliner needs accurate flags to determine when LR needs to be preserved.

rdar://131313095

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

9 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+3)
(added) clang/include/clang/Driver/aarch64-mlr-for-calls-only.c (+4)
(modified) clang/lib/Driver/ToolChains/Arch/AArch64.cpp (+3)
(modified) llvm/lib/Target/AArch64/AArch64Features.td (+4)
(modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+5-2)
(modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp (+3)
(modified) llvm/lib/Target/AArch64/AArch64Subtarget.h (+1)
(modified) llvm/test/CodeGen/AArch64/arm64-platform-reg.ll (+6-1)
(added) llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir (+110)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 58ca6f2bea9e4..714d5de76dd99 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4917,6 +4917,9 @@ foreach i = {1-31} in
   def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group<m_Group>,
     HelpText<"Reserve the x"#i#" register (AArch64/RISC-V only)">;
 
+def mlr_for_calls_only : Flag<["-"], "mlr-for-calls-only">, Group<m_aarch64_Features_Group>,
+  HelpText<"Do not allocate the LR register for general purpose usage, only for calls. (AArch64 only)">;
+
 foreach i = {8-15,18} in
   def fcall_saved_x#i : Flag<["-"], "fcall-saved-x"#i>, Group<m_aarch64_Features_Group>,
     HelpText<"Make the x"#i#" register call-saved (AArch64 only)">;
diff --git a/clang/include/clang/Driver/aarch64-mlr-for-calls-only.c b/clang/include/clang/Driver/aarch64-mlr-for-calls-only.c
new file mode 100644
index 0000000000000..1fe3bdab4f3e4
--- /dev/null
+++ b/clang/include/clang/Driver/aarch64-mlr-for-calls-only.c
@@ -0,0 +1,4 @@
+
+// RUN: %clang --target=aarch64-none-gnu -mlr-for-calls-only -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK < %t %s
+// CHECK: "-target-feature" "+reserve-lr-for-ra"
diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index ec248b80251ea..5fbf38cdda12b 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -398,6 +398,9 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
   if (Args.hasArg(options::OPT_ffixed_x28))
     Features.push_back("+reserve-x28");
 
+  if (Args.hasArg(options::OPT_mlr_for_calls_only))
+    Features.push_back("+reserve-lr-for-ra");
+
   if (Args.hasArg(options::OPT_fcall_saved_x8))
     Features.push_back("+call-saved-x8");
 
diff --git a/llvm/lib/Target/AArch64/AArch64Features.td b/llvm/lib/Target/AArch64/AArch64Features.td
index 8754ea4974999..50ff25ff1ba0c 100644
--- a/llvm/lib/Target/AArch64/AArch64Features.td
+++ b/llvm/lib/Target/AArch64/AArch64Features.td
@@ -571,6 +571,10 @@ foreach i = {1-7,9-15,18,20-28} in
                                              "Reserve X"#i#", making it unavailable "
                                              "as a GPR">;
 
+def FeatureReserveLRForRA : SubtargetFeature<"reserve-lr-for-ra",
+                                             "ReserveLRForRA", "true",
+                                             "Reserve LR for call use only">;
+
 foreach i = {8-15,18} in
     def FeatureCallSavedX#i : SubtargetFeature<"call-saved-x"#i,
          "CustomCallSavedXRegs["#i#"]", "true", "Make X"#i#" callee saved.">;
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 75e89e8222ae9..c760d29e9d138 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -3267,10 +3267,13 @@ bool AArch64FrameLowering::spillCalleeSavedRegisters(
         InsertSEH(MIB, TII, MachineInstr::FrameSetup);
     } else { // The code when the pair of ZReg is not present
       MachineInstrBuilder MIB = BuildMI(MBB, MI, DL, TII.get(StrOpc));
-      if (!MRI.isReserved(Reg1))
+      const AArch64RegisterInfo *RegInfo =
+          static_cast<const AArch64RegisterInfo *>(
+              MF.getSubtarget().getRegisterInfo());
+      if (!RegInfo->isStrictlyReservedReg(MF, Reg1))
         MBB.addLiveIn(Reg1);
       if (RPI.isPaired()) {
-        if (!MRI.isReserved(Reg2))
+        if (!RegInfo->isStrictlyReservedReg(MF, Reg2))
           MBB.addLiveIn(Reg2);
         MIB.addReg(Reg2, getPrologueDeath(MF, Reg2));
         MIB.addMemOperand(MF.getMachineMemOperand(
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index cc50b59dd8d7e..5408db0e653a4 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -501,6 +501,9 @@ AArch64RegisterInfo::getReservedRegs(const MachineFunction &MF) const {
       markSuperRegs(Reserved, AArch64::GPR32commonRegClass.getRegister(i));
   }
 
+  if (MF.getSubtarget<AArch64Subtarget>().isLRReservedForRA())
+    markSuperRegs(Reserved, AArch64::LR);
+
   assert(checkAllSuperRegsMarked(Reserved));
   return Reserved;
 }
diff --git a/llvm/lib/Target/AArch64/AArch64Subtarget.h b/llvm/lib/Target/AArch64/AArch64Subtarget.h
index 5faba09aa67bd..4b840b24ba134 100644
--- a/llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ b/llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -209,6 +209,7 @@ class AArch64Subtarget final : public AArch64GenSubtargetInfo {
     AllReservedX |= ReserveXRegisterForRA;
     return AllReservedX.count();
   }
+  bool isLRReservedForRA() const { return ReserveLRForRA; }
   bool isXRegCustomCalleeSaved(size_t i) const {
     return CustomCallSavedXRegs[i];
   }
diff --git a/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll b/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
index 3df2ef7aa59fc..eefc858db2752 100644
--- a/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
@@ -34,6 +34,7 @@
 ; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x26 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X26
 ; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x27 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X27
 ; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x28 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X28
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-lr-for-ra -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-LR-RA
 
 ; Test multiple of reserve-x# options together.
 ; RUN: llc -mtriple=arm64-linux-gnu \
@@ -72,6 +73,7 @@
 ; RUN: -mattr=+reserve-x26 \
 ; RUN: -mattr=+reserve-x27 \
 ; RUN: -mattr=+reserve-x28 \
+; RUN: -mattr=+reserve-lr-for-ra \
 ; RUN: -reserve-regs-for-regalloc=X8,X16,X17,X19 \
 ; RUN: -o - %s | FileCheck %s \
 ; RUN: --check-prefix=CHECK-RESERVE \
@@ -102,7 +104,8 @@
 ; RUN: --check-prefix=CHECK-RESERVE-X25 \
 ; RUN: --check-prefix=CHECK-RESERVE-X26 \
 ; RUN: --check-prefix=CHECK-RESERVE-X27 \
-; RUN: --check-prefix=CHECK-RESERVE-X28
+; RUN: --check-prefix=CHECK-RESERVE-X28 \
+; RUN: --check-prefix=CHECK-RESERVE-LR-RA
 
 ; x18 is reserved as a platform register on Darwin but not on other
 ; systems. Create loads of register pressure and make sure this is respected.
@@ -149,6 +152,7 @@ define void @keep_live() {
 ; CHECK-RESERVE-X26-NOT: ldr x26
 ; CHECK-RESERVE-X27-NOT: ldr x27
 ; CHECK-RESERVE-X28-NOT: ldr x28
+; CHECK-RESERVE-LR-RA-NOT: ldr x30
 ; CHECK-RESERVE: Spill
 ; CHECK-RESERVE-NOT: ldr fp
 ; CHECK-RESERVE-X1-NOT: ldr x1,
@@ -178,6 +182,7 @@ define void @keep_live() {
 ; CHECK-RESERVE-X26-NOT: ldr x26
 ; CHECK-RESERVE-X27-NOT: ldr x27
 ; CHECK-RESERVE-X28-NOT: ldr x28
+; CHECK-RESERVE-LR-RA-NOT: ldr x30
 ; CHECK-RESERVE: ret
   ret void
 }
diff --git a/llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir b/llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir
new file mode 100644
index 0000000000000..1a53fa1e43dfd
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir
@@ -0,0 +1,110 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -run-pass=prologepilog -o - %s | FileCheck %s
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128-Fn32"
+  target triple = "arm64e-apple-ios18.4.0"
+
+  declare void @spam()
+
+  define i32 @check_lr_live(ptr %arg) #0 {
+  bb:
+    %icmp = icmp eq ptr %arg, null
+    %or = or i1 %icmp, false
+    br i1 %or, label %bb2, label %bb1
+
+  bb1:                                              ; preds = %bb
+    call void @spam()
+    br label %bb2
+
+  bb2:                                              ; preds = %bb1, %bb
+    %phi = phi i32 [ -536870206, %bb ], [ 0, %bb1 ]
+    ret i32 %phi
+  }
+
+  attributes #0 = { nounwind "target-features"="+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+reserve-lr-for-ra" }
+
+...
+# This test checks that the live-in flags for LR are correctly set when LR is reserved, but live for a call.
+---
+name:            check_lr_live
+alignment:       4
+tracksRegLiveness: true
+tracksDebugUserValues: true
+liveins:
+  - { reg: '$x0' }
+frameInfo:
+  maxAlignment:    1
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+  savePoint:       '%bb.2'
+  restorePoint:    '%bb.2'
+machineFunctionInfo: {}
+body:             |
+  ; CHECK-LABEL: name: check_lr_live
+  ; CHECK: bb.0.bb:
+  ; CHECK-NEXT:   successors: %bb.1(0x20000000), %bb.2(0x60000000)
+  ; CHECK-NEXT:   liveins: $x0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $x8 = COPY $x0
+  ; CHECK-NEXT:   renamable $w0 = MOVi32imm -536870206
+  ; CHECK-NEXT:   CBNZX killed renamable $x8, %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   B %bb.4
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2.bb:
+  ; CHECK-NEXT:   successors: %bb.4(0x2aaaaaab), %bb.3(0x55555555)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   CBNZW $wzr, %bb.4
+  ; CHECK-NEXT:   B %bb.3
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3.bb1:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT:   liveins: $lr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   early-clobber $sp = frame-setup STPXpre killed $fp, killed $lr, $sp, -2 :: (store (s64) into %stack.1), (store (s64) into %stack.0)
+  ; CHECK-NEXT:   BL @spam, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+  ; CHECK-NEXT:   renamable $w0 = COPY $wzr
+  ; CHECK-NEXT:   early-clobber $sp, $fp, $lr = frame-destroy LDPXpost $sp, 2 :: (load (s64) from %stack.1), (load (s64) from %stack.0)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4.bb2:
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   RET_ReallyLR implicit $w0
+  bb.0.bb:
+    successors: %bb.4(0x20000000), %bb.1(0x60000000)
+    liveins: $x0
+
+    renamable $x8 = COPY $x0
+    renamable $w0 = MOVi32imm -536870206
+    CBNZX killed renamable $x8, %bb.1
+
+  bb.4:
+    liveins: $w0
+
+    B %bb.3
+
+  bb.1.bb:
+    successors: %bb.3(0x2aaaaaab), %bb.2(0x55555555)
+    liveins: $w0
+
+    CBNZW $wzr, %bb.3
+    B %bb.2
+
+  bb.2.bb1:
+    ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+    BL @spam, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+    ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+    renamable $w0 = COPY $wzr
+
+  bb.3.bb2:
+    liveins: $w0
+
+    RET_ReallyLR implicit $w0
+
+...

llvmbot · 2024-07-08T20:30:10Z

@llvm/pr-subscribers-clang-driver

Author: Amara Emerson (aemerson)

Changes

This re-introduces the effective behaviour that was reverted in 7ad481e.

This time we're not using the same mechanism, exposing another reservation feature
that prevents only regalloc from using the register, but not for other required uses
like ABIs.

This also fixes a consequent issue with reserving LR, which is that frame lowering
was only adding live-in flags for non-reserved regs. This would cause issues later
since the outliner needs accurate flags to determine when LR needs to be preserved.

rdar://131313095

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

9 Files Affected:

(modified) clang/include/clang/Driver/Options.td (+3)
(added) clang/include/clang/Driver/aarch64-mlr-for-calls-only.c (+4)
(modified) clang/lib/Driver/ToolChains/Arch/AArch64.cpp (+3)
(modified) llvm/lib/Target/AArch64/AArch64Features.td (+4)
(modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+5-2)
(modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp (+3)
(modified) llvm/lib/Target/AArch64/AArch64Subtarget.h (+1)
(modified) llvm/test/CodeGen/AArch64/arm64-platform-reg.ll (+6-1)
(added) llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir (+110)

diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 58ca6f2bea9e44..714d5de76dd996 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4917,6 +4917,9 @@ foreach i = {1-31} in
   def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group<m_Group>,
     HelpText<"Reserve the x"#i#" register (AArch64/RISC-V only)">;
 
+def mlr_for_calls_only : Flag<["-"], "mlr-for-calls-only">, Group<m_aarch64_Features_Group>,
+  HelpText<"Do not allocate the LR register for general purpose usage, only for calls. (AArch64 only)">;
+
 foreach i = {8-15,18} in
   def fcall_saved_x#i : Flag<["-"], "fcall-saved-x"#i>, Group<m_aarch64_Features_Group>,
     HelpText<"Make the x"#i#" register call-saved (AArch64 only)">;
diff --git a/clang/include/clang/Driver/aarch64-mlr-for-calls-only.c b/clang/include/clang/Driver/aarch64-mlr-for-calls-only.c
new file mode 100644
index 00000000000000..1fe3bdab4f3e43
--- /dev/null
+++ b/clang/include/clang/Driver/aarch64-mlr-for-calls-only.c
@@ -0,0 +1,4 @@
+
+// RUN: %clang --target=aarch64-none-gnu -mlr-for-calls-only -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK < %t %s
+// CHECK: "-target-feature" "+reserve-lr-for-ra"
diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index ec248b80251ea3..5fbf38cdda12b1 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -398,6 +398,9 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
   if (Args.hasArg(options::OPT_ffixed_x28))
     Features.push_back("+reserve-x28");
 
+  if (Args.hasArg(options::OPT_mlr_for_calls_only))
+    Features.push_back("+reserve-lr-for-ra");
+
   if (Args.hasArg(options::OPT_fcall_saved_x8))
     Features.push_back("+call-saved-x8");
 
diff --git a/llvm/lib/Target/AArch64/AArch64Features.td b/llvm/lib/Target/AArch64/AArch64Features.td
index 8754ea4974999f..50ff25ff1ba0c8 100644
--- a/llvm/lib/Target/AArch64/AArch64Features.td
+++ b/llvm/lib/Target/AArch64/AArch64Features.td
@@ -571,6 +571,10 @@ foreach i = {1-7,9-15,18,20-28} in
                                              "Reserve X"#i#", making it unavailable "
                                              "as a GPR">;
 
+def FeatureReserveLRForRA : SubtargetFeature<"reserve-lr-for-ra",
+                                             "ReserveLRForRA", "true",
+                                             "Reserve LR for call use only">;
+
 foreach i = {8-15,18} in
     def FeatureCallSavedX#i : SubtargetFeature<"call-saved-x"#i,
          "CustomCallSavedXRegs["#i#"]", "true", "Make X"#i#" callee saved.">;
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 75e89e8222ae95..c760d29e9d1385 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -3267,10 +3267,13 @@ bool AArch64FrameLowering::spillCalleeSavedRegisters(
         InsertSEH(MIB, TII, MachineInstr::FrameSetup);
     } else { // The code when the pair of ZReg is not present
       MachineInstrBuilder MIB = BuildMI(MBB, MI, DL, TII.get(StrOpc));
-      if (!MRI.isReserved(Reg1))
+      const AArch64RegisterInfo *RegInfo =
+          static_cast<const AArch64RegisterInfo *>(
+              MF.getSubtarget().getRegisterInfo());
+      if (!RegInfo->isStrictlyReservedReg(MF, Reg1))
         MBB.addLiveIn(Reg1);
       if (RPI.isPaired()) {
-        if (!MRI.isReserved(Reg2))
+        if (!RegInfo->isStrictlyReservedReg(MF, Reg2))
           MBB.addLiveIn(Reg2);
         MIB.addReg(Reg2, getPrologueDeath(MF, Reg2));
         MIB.addMemOperand(MF.getMachineMemOperand(
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index cc50b59dd8d7e1..5408db0e653a49 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -501,6 +501,9 @@ AArch64RegisterInfo::getReservedRegs(const MachineFunction &MF) const {
       markSuperRegs(Reserved, AArch64::GPR32commonRegClass.getRegister(i));
   }
 
+  if (MF.getSubtarget<AArch64Subtarget>().isLRReservedForRA())
+    markSuperRegs(Reserved, AArch64::LR);
+
   assert(checkAllSuperRegsMarked(Reserved));
   return Reserved;
 }
diff --git a/llvm/lib/Target/AArch64/AArch64Subtarget.h b/llvm/lib/Target/AArch64/AArch64Subtarget.h
index 5faba09aa67bdc..4b840b24ba134b 100644
--- a/llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ b/llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -209,6 +209,7 @@ class AArch64Subtarget final : public AArch64GenSubtargetInfo {
     AllReservedX |= ReserveXRegisterForRA;
     return AllReservedX.count();
   }
+  bool isLRReservedForRA() const { return ReserveLRForRA; }
   bool isXRegCustomCalleeSaved(size_t i) const {
     return CustomCallSavedXRegs[i];
   }
diff --git a/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll b/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
index 3df2ef7aa59fc8..eefc858db27526 100644
--- a/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-platform-reg.ll
@@ -34,6 +34,7 @@
 ; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x26 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X26
 ; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x27 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X27
 ; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-x28 -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-X28
+; RUN: llc -mtriple=arm64-linux-gnu -mattr=+reserve-lr-for-ra -o - %s | FileCheck %s --check-prefixes=CHECK-RESERVE,CHECK-RESERVE-LR-RA
 
 ; Test multiple of reserve-x# options together.
 ; RUN: llc -mtriple=arm64-linux-gnu \
@@ -72,6 +73,7 @@
 ; RUN: -mattr=+reserve-x26 \
 ; RUN: -mattr=+reserve-x27 \
 ; RUN: -mattr=+reserve-x28 \
+; RUN: -mattr=+reserve-lr-for-ra \
 ; RUN: -reserve-regs-for-regalloc=X8,X16,X17,X19 \
 ; RUN: -o - %s | FileCheck %s \
 ; RUN: --check-prefix=CHECK-RESERVE \
@@ -102,7 +104,8 @@
 ; RUN: --check-prefix=CHECK-RESERVE-X25 \
 ; RUN: --check-prefix=CHECK-RESERVE-X26 \
 ; RUN: --check-prefix=CHECK-RESERVE-X27 \
-; RUN: --check-prefix=CHECK-RESERVE-X28
+; RUN: --check-prefix=CHECK-RESERVE-X28 \
+; RUN: --check-prefix=CHECK-RESERVE-LR-RA
 
 ; x18 is reserved as a platform register on Darwin but not on other
 ; systems. Create loads of register pressure and make sure this is respected.
@@ -149,6 +152,7 @@ define void @keep_live() {
 ; CHECK-RESERVE-X26-NOT: ldr x26
 ; CHECK-RESERVE-X27-NOT: ldr x27
 ; CHECK-RESERVE-X28-NOT: ldr x28
+; CHECK-RESERVE-LR-RA-NOT: ldr x30
 ; CHECK-RESERVE: Spill
 ; CHECK-RESERVE-NOT: ldr fp
 ; CHECK-RESERVE-X1-NOT: ldr x1,
@@ -178,6 +182,7 @@ define void @keep_live() {
 ; CHECK-RESERVE-X26-NOT: ldr x26
 ; CHECK-RESERVE-X27-NOT: ldr x27
 ; CHECK-RESERVE-X28-NOT: ldr x28
+; CHECK-RESERVE-LR-RA-NOT: ldr x30
 ; CHECK-RESERVE: ret
   ret void
 }
diff --git a/llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir b/llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir
new file mode 100644
index 00000000000000..1a53fa1e43dfdf
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/lr-reserved-for-ra-live-in.mir
@@ -0,0 +1,110 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -run-pass=prologepilog -o - %s | FileCheck %s
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128-Fn32"
+  target triple = "arm64e-apple-ios18.4.0"
+
+  declare void @spam()
+
+  define i32 @check_lr_live(ptr %arg) #0 {
+  bb:
+    %icmp = icmp eq ptr %arg, null
+    %or = or i1 %icmp, false
+    br i1 %or, label %bb2, label %bb1
+
+  bb1:                                              ; preds = %bb
+    call void @spam()
+    br label %bb2
+
+  bb2:                                              ; preds = %bb1, %bb
+    %phi = phi i32 [ -536870206, %bb ], [ 0, %bb1 ]
+    ret i32 %phi
+  }
+
+  attributes #0 = { nounwind "target-features"="+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+reserve-lr-for-ra" }
+
+...
+# This test checks that the live-in flags for LR are correctly set when LR is reserved, but live for a call.
+---
+name:            check_lr_live
+alignment:       4
+tracksRegLiveness: true
+tracksDebugUserValues: true
+liveins:
+  - { reg: '$x0' }
+frameInfo:
+  maxAlignment:    1
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+  savePoint:       '%bb.2'
+  restorePoint:    '%bb.2'
+machineFunctionInfo: {}
+body:             |
+  ; CHECK-LABEL: name: check_lr_live
+  ; CHECK: bb.0.bb:
+  ; CHECK-NEXT:   successors: %bb.1(0x20000000), %bb.2(0x60000000)
+  ; CHECK-NEXT:   liveins: $x0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   renamable $x8 = COPY $x0
+  ; CHECK-NEXT:   renamable $w0 = MOVi32imm -536870206
+  ; CHECK-NEXT:   CBNZX killed renamable $x8, %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   B %bb.4
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2.bb:
+  ; CHECK-NEXT:   successors: %bb.4(0x2aaaaaab), %bb.3(0x55555555)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   CBNZW $wzr, %bb.4
+  ; CHECK-NEXT:   B %bb.3
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3.bb1:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT:   liveins: $lr
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   early-clobber $sp = frame-setup STPXpre killed $fp, killed $lr, $sp, -2 :: (store (s64) into %stack.1), (store (s64) into %stack.0)
+  ; CHECK-NEXT:   BL @spam, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+  ; CHECK-NEXT:   renamable $w0 = COPY $wzr
+  ; CHECK-NEXT:   early-clobber $sp, $fp, $lr = frame-destroy LDPXpost $sp, 2 :: (load (s64) from %stack.1), (load (s64) from %stack.0)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4.bb2:
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   RET_ReallyLR implicit $w0
+  bb.0.bb:
+    successors: %bb.4(0x20000000), %bb.1(0x60000000)
+    liveins: $x0
+
+    renamable $x8 = COPY $x0
+    renamable $w0 = MOVi32imm -536870206
+    CBNZX killed renamable $x8, %bb.1
+
+  bb.4:
+    liveins: $w0
+
+    B %bb.3
+
+  bb.1.bb:
+    successors: %bb.3(0x2aaaaaab), %bb.2(0x55555555)
+    liveins: $w0
+
+    CBNZW $wzr, %bb.3
+    B %bb.2
+
+  bb.2.bb1:
+    ADJCALLSTACKDOWN 0, 0, implicit-def dead $sp, implicit $sp
+    BL @spam, csr_darwin_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp
+    ADJCALLSTACKUP 0, 0, implicit-def dead $sp, implicit $sp
+    renamable $w0 = COPY $wzr
+
+  bb.3.bb2:
+    liveins: $w0
+
+    RET_ReallyLR implicit $w0
+
+...

clang/include/clang/Driver/aarch64-mlr-for-calls-only.c

Co-authored-by: Francis Visoiu Mistrih <[email protected]>

francisvm

Things LGTM, and thanks for fixing the liveness issue. Please let @efriedma-quic comment on the flag name as well.

efriedma-quic · 2024-07-08T23:20:01Z

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

+      const AArch64RegisterInfo *RegInfo =
+          static_cast<const AArch64RegisterInfo *>(
+              MF.getSubtarget().getRegisterInfo());
+      if (!RegInfo->isStrictlyReservedReg(MF, Reg1))


I'm not confident this does what you want: even if you get the liveness right inside FrameLowering, target-independent code like LivePhysRegs will ignore registers marked isReserved(). So I'm not sure any liveness computations based on a reserved LR can be trusted.

Maybe not, but I'm not sure what other options we have. It's worked for so far except for this case with the outliner.

We could unreserve LR after register allocation, I guess? That would have roughly the right semantics.

Or we could make the outliner use some other method to try to figure out the "liveness" of LR if it's a reserved register. I mean, if LR is reserved, the only way it can be live is if the prologue doesn't save/restore it (in which case it's live across the whole function).

We could unreserve LR after register allocation, I guess? That would have roughly the right semantics.

Having getReservedRegs(MF) return different things depending on where it's running seems worse than the bug itself.

With shrink-wrapping the prolog can be in successor blocks, so LR can be live on some CFG paths and not others.

Having getReservedRegs(MF) return different things depending on where it's running seems worse than the bug itself.

I think this can happen anyway? For example, with the frame/base pointer.

With shrink-wrapping the prolog can be in successor blocks, so LR can be live on some CFG paths and not others.

Oh, right.

Other possibilities:

We could mess with the allocation orders so LR isn't allocated, despite being an allocatable register. This has basically all the properties we want... but we don't have the necessary infrastructure to make it straightforward.

We could introduce a distinction between "reserved for regalloc" and "tracked for liveness". So we'd have registers where we track liveness, but the register allocator doesn't allocate them.

Maybe worth bringing up on Discourse to see if there are other opinions.

(I'm not trying to make things complicated here, but I really don't want to take shortcuts when it comes to core MIR datastructures.)

I think this can happen anyway? For example, with the frame/base pointer.

Indeed :(

We could mess with the allocation orders so LR isn't allocated, despite being an allocatable register. This has basically all the properties we want... but we don't have the necessary infrastructure to make it straightforward.

Yeah, I thought about something along these lines. Perhaps if we could dynamically alter regclasses.

I think I'll give the changing reservedness approach a go here since there's already some precedent for it.

efriedma-quic · 2024-07-08T23:29:43Z

clang/include/clang/Driver/Options.td

@@ -4917,6 +4917,9 @@ foreach i = {1-31} in
  def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group<m_Group>,
    HelpText<"Reserve the x"#i#" register (AArch64/RISC-V only)">;

+def mlr_for_calls_only : Flag<["-"], "mlr-for-calls-only">, Group<m_aarch64_Features_Group>,


Name seems okay.

…ng before/after regalloc

efriedma-quic

The updated approach makes sense, I think.

Please check that AArch64TargetLowering::LowerRETURNADDR works correctly when LR is reserved; I think it should just work, but I'm not completely sure.

Otherwise LGTM

aemerson · 2024-07-10T22:13:05Z

Thanks, the live-in flag seems to be added correctly for the return in the new test.

…x30 flag. (llvm#98073) This re-introduces the effective behaviour that was reverted in 7ad481e. This time we're not using the same mechanism, exposing another reservation feature that prevents only regalloc from using the register, but not for other required uses like ABIs. This also fixes a consequent issue with reserving LR, which is that frame lowering was only adding live-in flags for non-reserved regs. This would cause issues later since the outliner needs accurate flags to determine when LR needs to be preserved. rdar://131313095

llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jul 8, 2024

aemerson requested a review from efriedma-quic July 8, 2024 20:30

aemerson requested a review from francisvm July 8, 2024 20:30

aemerson mentioned this pull request Jul 8, 2024

Revert "[AArch64] Add support for -ffixed-x30" #88019

Merged

francisvm reviewed Jul 8, 2024

View reviewed changes

clang/include/clang/Driver/aarch64-mlr-for-calls-only.c Outdated Show resolved Hide resolved

whitespace

2bf70e8

Co-authored-by: Francis Visoiu Mistrih <[email protected]>

francisvm approved these changes Jul 8, 2024

View reviewed changes

efriedma-quic requested a review from MaskRay July 8, 2024 23:12

efriedma-quic reviewed Jul 8, 2024

View reviewed changes

aemerson added 2 commits July 8, 2024 23:04

Fix framelayout-sve.mir test

da20973

Rework to instead conditionally reserve based on whether we're checki…

58517b2

…ng before/after regalloc

efriedma-quic approved these changes Jul 10, 2024

View reviewed changes

aemerson merged commit 9865171 into llvm:main Jul 10, 2024
7 checks passed

aemerson deleted the co2-main branch July 10, 2024 22:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AArch64] Add -mlr-for-calls-only to replace the now removed -ffixed-x30 flag. #98073

[AArch64] Add -mlr-for-calls-only to replace the now removed -ffixed-x30 flag. #98073

Uh oh!

aemerson commented Jul 8, 2024

Uh oh!

llvmbot commented Jul 8, 2024 •

edited

Loading

Uh oh!

llvmbot commented Jul 8, 2024

Uh oh!

Uh oh!

francisvm left a comment

Uh oh!

efriedma-quic Jul 8, 2024

Uh oh!

aemerson Jul 9, 2024

Uh oh!

efriedma-quic Jul 9, 2024

Uh oh!

aemerson Jul 9, 2024

Uh oh!

efriedma-quic Jul 9, 2024

Uh oh!

aemerson Jul 9, 2024

Uh oh!

efriedma-quic Jul 8, 2024

Uh oh!

efriedma-quic left a comment

Uh oh!

aemerson commented Jul 10, 2024

Uh oh!

Uh oh!

Uh oh!

[AArch64] Add -mlr-for-calls-only to replace the now removed -ffixed-x30 flag. #98073

[AArch64] Add -mlr-for-calls-only to replace the now removed -ffixed-x30 flag. #98073

Uh oh!

Conversation

aemerson commented Jul 8, 2024

Uh oh!

llvmbot commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 8, 2024

Uh oh!

Uh oh!

francisvm left a comment

Choose a reason for hiding this comment

Uh oh!

efriedma-quic Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

aemerson Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

efriedma-quic Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

aemerson Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

efriedma-quic Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

aemerson Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

efriedma-quic Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

aemerson commented Jul 10, 2024

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jul 8, 2024 •

edited

Loading