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

[ARM] Fix -mno-omit-leaf-frame-pointer flag doesn't works on 32-bit ARM #109628

Merged
merged 1 commit into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,11 @@ X86 Support
Arm and AArch64 Support
^^^^^^^^^^^^^^^^^^^^^^^

- In the ARM Target, the frame pointer (FP) of a leaf function can be retained
by using the ``-fno-omit-frame-pointer`` option. If you want to eliminate the FP
in leaf functions after enabling ``-fno-omit-frame-pointer``, you can do so by adding
the ``-momit-leaf-frame-pointer`` option.

Android Support
^^^^^^^^^^^^^^^

Expand Down
6 changes: 6 additions & 0 deletions llvm/docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ Changes to the ARM Backend
the required alignment space with a sequence of `0x0` bytes (the requested
fill value) rather than NOPs.

* The default behavior for frame pointers in leaf functions has been updated.
When the `-fno-omit-frame-pointer` option is specified, `FPKeepKindStr` is
set to `-mframe-pointer=all`, meaning the frame pointer (FP) is now retained
in leaf functions by default. To eliminate the frame pointer in leaf functions,
you must explicitly use the `-momit-leaf-frame-pointer` option.

Changes to the AVR Backend
--------------------------

Expand Down
6 changes: 0 additions & 6 deletions llvm/include/llvm/CodeGen/TargetFrameLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -277,12 +277,6 @@ class TargetFrameLowering {
return false;
}

/// Return true if the target wants to keep the frame pointer regardless of
/// the function attribute "frame-pointer".
virtual bool keepFramePointer(const MachineFunction &MF) const {
return false;
}

/// hasFP - Return true if the specified function should have a dedicated
/// frame pointer register. For most targets this is true only if the function
/// has variable sized allocas or if frame pointer elimination is disabled.
Expand Down
8 changes: 0 additions & 8 deletions llvm/lib/CodeGen/TargetOptionsImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ using namespace llvm;
/// DisableFramePointerElim - This returns true if frame pointer elimination
/// optimization should be disabled for the given machine function.
bool TargetOptions::DisableFramePointerElim(const MachineFunction &MF) const {
// Check to see if the target want to forcibly keep frame pointer.
if (MF.getSubtarget().getFrameLowering()->keepFramePointer(MF))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keepFramePointer is implemented as a target independent method... if you're going to get rid of the call here, you should also get rid of the interface from TargetFrameLowering. (No other in-tree target implements it anyway, but we want to keep the code clean, and give an obvious error in case there is someone with an out-of-tree dependency.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the interface from TargetFrameLowering in the latest submission.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse me, Please check the code again, thank you. @efriedma-quic

return true;

const Function &F = MF.getFunction();

if (!F.hasFnAttribute("frame-pointer"))
Expand All @@ -41,10 +37,6 @@ bool TargetOptions::DisableFramePointerElim(const MachineFunction &MF) const {
}

bool TargetOptions::FramePointerIsReserved(const MachineFunction &MF) const {
// Check to see if the target want to forcibly keep frame pointer.
if (MF.getSubtarget().getFrameLowering()->keepFramePointer(MF))
return true;

const Function &F = MF.getFunction();

if (!F.hasFnAttribute("frame-pointer"))
Expand Down
7 changes: 6 additions & 1 deletion llvm/lib/Target/ARM/ARMFrameLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,10 @@ bool ARMFrameLowering::hasFP(const MachineFunction &MF) const {
const TargetRegisterInfo *RegInfo = MF.getSubtarget().getRegisterInfo();
const MachineFrameInfo &MFI = MF.getFrameInfo();

// Check to see if the target want to forcibly keep frame pointer.
if (keepFramePointer(MF))
return true;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine.


// ABI-required frame pointer.
if (MF.getTarget().Options.DisableFramePointerElim(MF))
return true;
Expand Down Expand Up @@ -2365,7 +2369,8 @@ void ARMFrameLowering::determineCalleeSaves(MachineFunction &MF,
// to take advantage the eliminateFrameIndex machinery. This also ensures it
// is spilled in the order specified by getCalleeSavedRegs() to make it easier
// to combine multiple loads / stores.
bool CanEliminateFrame = !(requiresAAPCSFrameRecord(MF) && hasFP(MF));
bool CanEliminateFrame = !(requiresAAPCSFrameRecord(MF) && hasFP(MF)) &&
!MF.getTarget().Options.DisableFramePointerElim(MF);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also seems fine.

bool CS1Spilled = false;
bool LRSpilled = false;
unsigned NumGPRSpills = 0;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/ARM/ARMFrameLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ARMFrameLowering : public TargetFrameLowering {
MutableArrayRef<CalleeSavedInfo> CSI,
const TargetRegisterInfo *TRI) const override;

bool keepFramePointer(const MachineFunction &MF) const override;
bool keepFramePointer(const MachineFunction &MF) const;

bool enableCalleeSaveSkip(const MachineFunction &MF) const override;

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/2011-03-15-LdStMultipleBug.ll
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

@oStruct = external global %struct.Outer, align 4

define void @main(i8 %val8) nounwind {
define void @main(i8 %val8) nounwind "frame-pointer"="none" {
; CHECK-LABEL: main:
; CHECK: @ %bb.0: @ %for.body.lr.ph
; CHECK-NEXT: movw r0, :lower16:(L_oStruct$non_lazy_ptr-(LPC0_0+4))
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/2011-12-19-sjlj-clobber.ll
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
; Radar 10567930: Make sure that all the caller-saved registers are saved and
; restored in a function with setjmp/longjmp EH. In particular, r6 was not
; being saved here.
; CHECK: push {r4, r5, r6, r7, lr}
; CHECK: push.w {r4, r5, r6, r7, r8, r10, r11, lr}

%0 = type opaque
%struct.NSConstantString = type { ptr, i32, ptr, i32 }
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/arm-shrink-wrapping.ll
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,7 @@ if.end:
; Another infinite loop test this time with two nested infinite loop.
; infiniteloop3
; bx lr
define void @infiniteloop3() "frame-pointer"="all" {
define void @infiniteloop3() "frame-pointer"="none" {
; ARM-LABEL: infiniteloop3:
; ARM: @ %bb.0: @ %entry
; ARM-NEXT: mov r0, #0
Expand Down
56 changes: 26 additions & 30 deletions llvm/test/CodeGen/ARM/atomic-load-store.ll
Original file line number Diff line number Diff line change
Expand Up @@ -324,18 +324,17 @@ define void @test_old_store_64bit(ptr %p, i64 %v) {
;
; ARMOPTNONE-LABEL: test_old_store_64bit:
; ARMOPTNONE: @ %bb.0:
; ARMOPTNONE-NEXT: push {r4, r5, r7, lr}
; ARMOPTNONE-NEXT: add r7, sp, #8
; ARMOPTNONE-NEXT: push {r8, r10, r11}
; ARMOPTNONE-NEXT: sub sp, sp, #24
; ARMOPTNONE-NEXT: str r0, [sp, #4] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r2, [sp, #8] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r1, [sp, #12] @ 4-byte Spill
; ARMOPTNONE-NEXT: dmb ish
; ARMOPTNONE-NEXT: ldr r1, [r0]
; ARMOPTNONE-NEXT: ldr r0, [r0, #4]
; ARMOPTNONE-NEXT: str r1, [sp, #16] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r0, [sp, #20] @ 4-byte Spill
; ARMOPTNONE-NEXT: push {r4, r5, r7, r8, r10, r11, lr}
; ARMOPTNONE-NEXT: add r7, sp, #20
; ARMOPTNONE-NEXT: sub sp, sp, #24
; ARMOPTNONE-NEXT: str r0, [sp, #4] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r2, [sp, #8] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r1, [sp, #12] @ 4-byte Spill
; ARMOPTNONE-NEXT: dmb ish
; ARMOPTNONE-NEXT: ldr r1, [r0]
; ARMOPTNONE-NEXT: ldr r0, [r0, #4]
; ARMOPTNONE-NEXT: str r1, [sp, #16] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r0, [sp, #20] @ 4-byte Spill
; ARMOPTNONE-NEXT: b LBB5_1
; ARMOPTNONE-NEXT: LBB5_1: @ %atomicrmw.start
; ARMOPTNONE-NEXT: @ =>This Loop Header: Depth=1
Expand Down Expand Up @@ -382,8 +381,7 @@ define void @test_old_store_64bit(ptr %p, i64 %v) {
; ARMOPTNONE-NEXT: LBB5_5: @ %atomicrmw.end
; ARMOPTNONE-NEXT: dmb ish
; ARMOPTNONE-NEXT: sub sp, r7, #20
; ARMOPTNONE-NEXT: pop {r8, r10, r11}
; ARMOPTNONE-NEXT: pop {r4, r5, r7, pc}
; ARMOPTNONE-NEXT: pop {r4, r5, r7, r8, r10, r11, pc}
;
; THUMBTWO-LABEL: test_old_store_64bit:
; THUMBTWO: @ %bb.0:
Expand Down Expand Up @@ -864,20 +862,19 @@ define void @store_atomic_f64__seq_cst(ptr %ptr, double %val1) {
;
; ARMOPTNONE-LABEL: store_atomic_f64__seq_cst:
; ARMOPTNONE: @ %bb.0:
; ARMOPTNONE-NEXT: push {r4, r5, r7, lr}
; ARMOPTNONE-NEXT: add r7, sp, #8
; ARMOPTNONE-NEXT: push {r8, r10, r11}
; ARMOPTNONE-NEXT: sub sp, sp, #24
; ARMOPTNONE-NEXT: str r0, [sp, #4] @ 4-byte Spill
; ARMOPTNONE-NEXT: vmov d16, r1, r2
; ARMOPTNONE-NEXT: vmov r1, r2, d16
; ARMOPTNONE-NEXT: str r2, [sp, #8] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r1, [sp, #12] @ 4-byte Spill
; ARMOPTNONE-NEXT: dmb ish
; ARMOPTNONE-NEXT: ldr r1, [r0]
; ARMOPTNONE-NEXT: ldr r0, [r0, #4]
; ARMOPTNONE-NEXT: str r1, [sp, #16] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r0, [sp, #20] @ 4-byte Spill
; ARMOPTNONE-NEXT: push {r4, r5, r7, r8, r10, r11, lr}
; ARMOPTNONE-NEXT: add r7, sp, #20
; ARMOPTNONE-NEXT: sub sp, sp, #24
; ARMOPTNONE-NEXT: str r0, [sp, #4] @ 4-byte Spill
; ARMOPTNONE-NEXT: vmov d16, r1, r2
; ARMOPTNONE-NEXT: vmov r1, r2, d16
; ARMOPTNONE-NEXT: str r2, [sp, #8] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r1, [sp, #12] @ 4-byte Spill
; ARMOPTNONE-NEXT: dmb ish
; ARMOPTNONE-NEXT: ldr r1, [r0]
; ARMOPTNONE-NEXT: ldr r0, [r0, #4]
; ARMOPTNONE-NEXT: str r1, [sp, #16] @ 4-byte Spill
; ARMOPTNONE-NEXT: str r0, [sp, #20] @ 4-byte Spill
; ARMOPTNONE-NEXT: b LBB13_1
; ARMOPTNONE-NEXT: LBB13_1: @ %atomicrmw.start
; ARMOPTNONE-NEXT: @ =>This Loop Header: Depth=1
Expand Down Expand Up @@ -924,8 +921,7 @@ define void @store_atomic_f64__seq_cst(ptr %ptr, double %val1) {
; ARMOPTNONE-NEXT: LBB13_5: @ %atomicrmw.end
; ARMOPTNONE-NEXT: dmb ish
; ARMOPTNONE-NEXT: sub sp, r7, #20
; ARMOPTNONE-NEXT: pop {r8, r10, r11}
; ARMOPTNONE-NEXT: pop {r4, r5, r7, pc}
; ARMOPTNONE-NEXT: pop {r4, r5, r7, r8, r10, r11, pc}
;
; THUMBTWO-LABEL: store_atomic_f64__seq_cst:
; THUMBTWO: @ %bb.0:
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/ARM/call-tc.ll
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ define void @t1() "frame-pointer"="all" {
ret void
}

define void @t2() "frame-pointer"="all" {
define void @t2() "frame-pointer"="none" {
; CHECKV6-LABEL: t2:
; CHECKV6: bx r0
; CHECKT2D-LABEL: t2:
Expand Down Expand Up @@ -102,7 +102,7 @@ bb:

; Make sure codegenprep is duplicating ret instructions to enable tail calls.
; rdar://11140249
define i32 @t8(i32 %x) nounwind ssp "frame-pointer"="all" {
define i32 @t8(i32 %x) nounwind ssp "frame-pointer"="none" {
entry:
; CHECKT2D-LABEL: t8:
; CHECKT2D-NOT: push
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/debug-frame.ll
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ entry:
; Test 4
;-------------------------------------------------------------------------------

define void @test4() nounwind {
define void @test4() nounwind "frame-pointer"="none" {
entry:
ret void
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/ehabi.ll
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ entry:
; Test 4
;-------------------------------------------------------------------------------

define void @test4() nounwind {
define void @test4() nounwind "frame-pointer"="none" {
entry:
ret void
}
Expand Down
6 changes: 3 additions & 3 deletions llvm/test/CodeGen/ARM/fast-isel-frameaddr.ll
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ entry:
; DARWIN-THUMB2: mov r0, r7

; LINUX-ARM-LABEL: frameaddr_index0:
; LINUX-ARM: push {r11, lr}
; LINUX-ARM: push {r11}
; LINUX-ARM: mov r11, sp
; LINUX-ARM: mov r0, r11

Expand All @@ -42,7 +42,7 @@ entry:
; DARWIN-THUMB2: ldr r0, [r7]

; LINUX-ARM-LABEL: frameaddr_index1:
; LINUX-ARM: push {r11, lr}
; LINUX-ARM: push {r11}
; LINUX-ARM: mov r11, sp
; LINUX-ARM: ldr r0, [r11]

Expand Down Expand Up @@ -73,7 +73,7 @@ entry:
; DARWIN-THUMB2: ldr r0, [r0]

; LINUX-ARM-LABEL: frameaddr_index3:
; LINUX-ARM: push {r11, lr}
; LINUX-ARM: push {r11}
; LINUX-ARM: mov r11, sp
; LINUX-ARM: ldr r0, [r11]
; LINUX-ARM: ldr r0, [r0]
Expand Down
11 changes: 7 additions & 4 deletions llvm/test/CodeGen/ARM/frame-chain.ll
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@
define dso_local noundef i32 @leaf(i32 noundef %0) {
; LEAF-FP-LABEL: leaf:
; LEAF-FP: @ %bb.0:
; LEAF-FP-NEXT: .pad #4
; LEAF-FP-NEXT: sub sp, sp, #4
; LEAF-FP-NEXT: str r0, [sp]
; LEAF-FP-NEXT: .save {r11, lr}
; LEAF-FP-NEXT: push {r11, lr}
; LEAF-FP-NEXT: .setfp r11, sp
; LEAF-FP-NEXT: mov r11, sp
; LEAF-FP-NEXT: push {r0}
; LEAF-FP-NEXT: add r0, r0, #4
; LEAF-FP-NEXT: add sp, sp, #4
; LEAF-FP-NEXT: mov sp, r11
; LEAF-FP-NEXT: pop {r11, lr}
; LEAF-FP-NEXT: mov pc, lr
;
; LEAF-FP-AAPCS-LABEL: leaf:
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/ifcvt5.ll
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

@x = external global ptr ; <ptr> [#uses=1]

define void @foo(i32 %a) "frame-pointer"="all" {
define void @foo(i32 %a) "frame-pointer"="none" {
; A8-LABEL: foo:
; A8: @ %bb.0: @ %entry
; A8-NEXT: movw r1, :lower16:(L_x$non_lazy_ptr-(LPC0_0+8))
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/ARM/ldrd.ll
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ define void @ldrd_postupdate_inc(ptr %p0) "frame-pointer"="all" {
; NORMAL: strd r1, r2, [r0], #-8
; CONSERVATIVE-NOT: strd
; CHECK: bx lr
define ptr @strd_postupdate_dec(ptr %p0, i32 %v0, i32 %v1) "frame-pointer"="all" {
define ptr @strd_postupdate_dec(ptr %p0, i32 %v0, i32 %v1) "frame-pointer"="none" {
%p0.1 = getelementptr i32, ptr %p0, i32 1
store i32 %v0, ptr %p0
store i32 %v1, ptr %p0.1
Expand All @@ -180,7 +180,7 @@ define ptr @strd_postupdate_dec(ptr %p0, i32 %v0, i32 %v1) "frame-pointer"="all"
; NORMAL: strd r1, r2, [r0], #8
; CONSERVATIVE-NOT: strd
; CHECK: bx lr
define ptr @strd_postupdate_inc(ptr %p0, i32 %v0, i32 %v1) "frame-pointer"="all" {
define ptr @strd_postupdate_inc(ptr %p0, i32 %v0, i32 %v1) "frame-pointer"="none" {
%p0.1 = getelementptr i32, ptr %p0, i32 1
store i32 %v0, ptr %p0
store i32 %v1, ptr %p0.1
Expand Down
9 changes: 5 additions & 4 deletions llvm/test/CodeGen/ARM/stack-frame-layout-remarks.ll
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
; BOTH: [SP-8]{{.+}}8{{.+}}4
; DEBUG: a @ dot.c:13
; STRIPPED-NOT: a @ dot.c:13
define void @cleanup_array(ptr %0) #1 {
define void @cleanup_array(ptr %0) #3 {
%2 = alloca ptr, align 8
store ptr %0, ptr %2, align 8
call void @llvm.dbg.declare(metadata ptr %2, metadata !41, metadata !DIExpression()), !dbg !46
Expand All @@ -62,7 +62,7 @@ define void @cleanup_array(ptr %0) #1 {
; BOTH: [SP-8]{{.+}}8{{.+}}4
; DEBUG: res @ dot.c:21
; STRIPPED-NOT: res @ dot.c:21
define void @cleanup_result(ptr %0) #1 {
define void @cleanup_result(ptr %0) #3 {
%2 = alloca ptr, align 8
store ptr %0, ptr %2, align 8
call void @llvm.dbg.declare(metadata ptr %2, metadata !47, metadata !DIExpression()), !dbg !51
Expand Down Expand Up @@ -92,7 +92,7 @@ define void @cleanup_result(ptr %0) #1 {
; BOTH: [SP-40]{{.+}}4{{.+}}4
; DEBUG: i @ dot.c:55
; STRIPPED-NOT: i @ dot.c:55
define i32 @do_work(ptr %0, ptr %1, ptr %2) #1 {
define i32 @do_work(ptr %0, ptr %1, ptr %2) #3 {
%4 = alloca i32, align 4
%5 = alloca ptr, align 8
%6 = alloca ptr, align 8
Expand Down Expand Up @@ -144,7 +144,7 @@ define i32 @do_work(ptr %0, ptr %1, ptr %2) #1 {
; BOTH: [SP-20]{{.+}}4{{.*}}4
; DEBUG: i @ dot.c:69
; STRIPPED-NOT: i @ dot.c:69
define ptr @gen_array(i32 %0) #1 {
define ptr @gen_array(i32 %0) #3 {
%2 = alloca ptr, align 8
%3 = alloca i32, align 4
%4 = alloca ptr, align 8
Expand Down Expand Up @@ -227,6 +227,7 @@ uselistorder ptr @llvm.dbg.declare, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
attributes #0 = { nocallback nofree nosync nounwind readnone speculatable willreturn }
attributes #1 = { "frame-pointer"="all" }
attributes #2 = { ssp "stack-protector-buffer-size"="5" "frame-pointer"="all" }
attributes #3 = { "frame-pointer"="none" }

!llvm.dbg.cu = !{!0, !2}
!llvm.module.flags = !{!18, !19, !20, !21, !22, !23, !24}
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/stack-size-section.ll
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@ define void @dynalloc(i32 %N) #0 {
ret void
}

attributes #0 = { "frame-pointer"="all" }
attributes #0 = { "frame-pointer"="none" }
Loading
Loading