[ARM] r11 is reserved when using -mframe-chain=aapcs #86951
Conversation
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-backend-arm Author: None (ostannard) ChangesWhen using the -mframe-chain=aapcs or -mframe-chain=aapcs-leaf options, we cannot use r11 as an allocatable register, even if -fomit-frame-pointer is also used. This is so that r11 will always point to a valid frame record, even if we don't create one in every function. Full diff: https://github.com/llvm/llvm-project/pull/86951.diff 2 Files Affected:
diff --git a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
index 9adf758b46c481..c149db3144c7c2 100644
--- a/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
+++ b/llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
@@ -207,7 +207,7 @@ getReservedRegs(const MachineFunction &MF) const {
markSuperRegs(Reserved, ARM::PC);
markSuperRegs(Reserved, ARM::FPSCR);
markSuperRegs(Reserved, ARM::APSR_NZCV);
- if (TFI->hasFP(MF))
+ if (TFI->isFPReserved(MF))
markSuperRegs(Reserved, STI.getFramePointerReg());
if (hasBasePointer(MF))
markSuperRegs(Reserved, BasePtr);
diff --git a/llvm/test/CodeGen/ARM/frame-pointer-reserved.ll b/llvm/test/CodeGen/ARM/frame-pointer-reserved.ll
new file mode 100644
index 00000000000000..d1a50c25432abf
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/frame-pointer-reserved.ll
@@ -0,0 +1,23 @@
+; RUN: llc -mtriple armv7a-none-eabi < %s --frame-pointer=all | FileCheck %s --check-prefixes CHECK,FPALL-ARM
+; RUN: llc -mtriple armv6m-none-eabi < %s --frame-pointer=all | FileCheck %s --check-prefixes CHECK,FPALL-THUMB1
+; RUN: llc -mtriple armv7m-none-eabi < %s --frame-pointer=all | FileCheck %s --check-prefixes CHECK,FPALL-THUMB2
+; RUN: llc -mtriple armv7a-none-eabi < %s --frame-pointer=none | FileCheck %s --check-prefixes CHECK,FPNONE
+; RUN: llc -mtriple armv6m-none-eabi < %s --frame-pointer=none | FileCheck %s --check-prefixes CHECK,FPNONE
+; RUN: llc -mtriple armv7m-none-eabi < %s --frame-pointer=none | FileCheck %s --check-prefixes CHECK,FPNONE
+
+; When the AAPCS frame chain is enabled, check that r11 is either used as a
+; frame pointer, which must point to an ABI-compatible frame record, or not
+; used at all, so that it continues to point to a valid frame record for the
+; calling function.
+
+define i32 @foo(i32 %a) "target-features"="+aapcs-frame-chain" {
+; CHECK-LABEL: foo:
+; FPALL-ARM: add r11, sp,
+; FPALL-THUMB1: mov r11, sp
+; FPALL-THUMB2: add.w r11, sp,
+; FPNONE-NOT: r11
+entry:
+ tail call void asm sideeffect "", "~{r0},~{r1},~{r2},~{r3},~{r4},~{r5},~{r6},~{r7},~{r8},~{r9},~{r10},~{r12},~{lr}"()
+ ret i32 %a
+}
+
|
|
This isn't really consistent with how we handle frame-pointer options on other targets. On the target-independent side: there are three possible settings for the frame pointer attribute/flag: "all", "non-leaf", and "none". In "all" and "non-leaf" mode, we want to preserve the frame pointer register; in "none" mode, we don't. If we do want a "don't create a frame pointer, but reserve the register" mode, I think we should change the way the target-independent settings work. That isn't to say the change to ARMBaseRegisterInfo.cpp is wrong, but I'm not sure the implementation of isFPReserved() works the way we want. |
|
Ah, I've just discovered the target-independent @pratlucas: you originally implemented these options, do you know how they should interact with the target-independent options? |
|
@pratlucas ping |
|
Ping |
|
Since the frame-chain options originally come from armclang, we can look at their documentation to see the expected semantics: https://developer.arm.com/documentation/101754/0622/armclang-Reference/armclang-Command-line-Options/-mframe-chain . The frontend flag semantics are... not really what I would have chosen, but seem good enough that it's not worth diverging. It would be nice to make sure -momit-leaf-frame-pointer interacts reasonably with -mframe-chain, though. Regardless of what the frontend flags look like, though, I think we want the internal representation to match what I mentioned before: change the definition of the "frame-pointer" function attribute in LLVM IR to add the value "reserved", and query that to check whether the frame pointer register should be reserved. The value fits cleanly into the existing hierarchy, has an obvious cross-platform meaning, and allows us to narrow the target-specific attribute to only specify whether r11 is the frame pointer register in Thumb mode. |
This adds a new value "reserved" to the "frame-pointer" function attribute. When this value is used, the frame pointer register must either be reserved, or updated to point to a new frame record, but must not be used for any other purpose. This is not yet supported by most targets, but will be used for the Arm -mframe-chain= option.
When using the -mframe-chain=aapcs or -mframe-chain=aapcs-leaf options, we cannot use r11 as an allocatable register, even if -fomit-frame-pointer is also used. This is so that r11 will always point to a valid frame record, even if we don't create one in every function. This uses the new "frame-pointer"="reserved" function attribute to represent the case where the frame pointer is reserved but not (always) used. This means that we can remove the "aapcs-frame-chain-leaf" subtarget feature, so that the "frame-pointer" attribute always controls the emission of the frame pointer, and the "aapcs-frame-chain" subtarget feature seelcts which ABI is followed.
ostannard
force-pushed
the
frame-pointer-reserved
branch
from
8d9a8e1
to
7272dd8
Compare
llvmbot
added
clang
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
When using the -mframe-chain=aapcs or -mframe-chain=aapcs-leaf options, we cannot use r11 as an allocatable register, even if -fomit-frame-pointer is also used. This is so that r11 will always point to a valid frame record, even if we don't create one in every function.