Skip to content

[win][aarch64] Always reserve frame pointers for Arm64 Windows #146582

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 2, 2025
Merged

[win][aarch64] Always reserve frame pointers for Arm64 Windows #146582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 13 additions & 2 deletions clang/lib/Driver/ToolChains/CommonArgs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,13 @@ static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
// even if new frame records are not created.
static bool mustMaintainValidFrameChain(const llvm::opt::ArgList &Args,
const llvm::Triple &Triple) {
if (Triple.isARM() || Triple.isThumb()) {
switch (Triple.getArch()) {
default:
return false;
case llvm::Triple::arm:
case llvm::Triple::armeb:
case llvm::Triple::thumb:
case llvm::Triple::thumbeb:
// For 32-bit Arm, the -mframe-chain=aapcs and -mframe-chain=aapcs+leaf
// options require the frame pointer register to be reserved (or point to a
// new AAPCS-compilant frame record), even with -fno-omit-frame-pointer.
Expand All @@ -183,8 +189,13 @@ static bool mustMaintainValidFrameChain(const llvm::opt::ArgList &Args,
return V != "none";
}
return false;

case llvm::Triple::aarch64:
// Arm64 Windows requires that the frame chain is valid, as there is no
// way to indicate during a stack walk that a frame has used the frame
// pointer as a general purpose register.
return Triple.isOSWindows();
}
return false;
}

// True if a target-specific option causes -fno-omit-frame-pointer to also
Expand Down
6 changes: 6 additions & 0 deletions clang/test/Driver/frame-pointer-elim.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// KEEP-NON-LEAF: "-mframe-pointer=non-leaf"
// KEEP-NONE-NOT: warning: argument unused
// KEEP-NONE: "-mframe-pointer=none"
// KEEP-RESERVED-NOT: warning: argument unused
// KEEP-RESERVED: "-mframe-pointer=reserved"

// On Linux x86, omit frame pointer when optimization is enabled.
// RUN: %clang -### --target=i386-linux -S -fomit-frame-pointer %s 2>&1 | \
Expand Down Expand Up @@ -215,5 +217,9 @@
// RUN: %clang -### --target=aarch64-none-elf -S -O1 -fno-omit-frame-pointer %s 2>&1 | \
// RUN: FileCheck --check-prefix=KEEP-NON-LEAF %s

// AArch64 Windows requires that the frame pointer be reserved
// RUN: %clang -### --target=aarch64-pc-windows-msvc -S -fomit-frame-pointer %s 2>&1 | \
// RUN: FileCheck --check-prefix=KEEP-RESERVED %s

void f0() {}
void f1() { f0(); }
21 changes: 21 additions & 0 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,27 @@ bool AArch64FrameLowering::hasFPImpl(const MachineFunction &MF) const {
return false;
}

/// Should the Frame Pointer be reserved for the current function?
bool AArch64FrameLowering::isFPReserved(const MachineFunction &MF) const {
const TargetMachine &TM = MF.getTarget();
const Triple &TT = TM.getTargetTriple();

// These OSes require the frame chain is valid, even if the current frame does
// not use a frame pointer.
if (TT.isOSDarwin() || TT.isOSWindows())
Copy link
Collaborator

Choose a reason for hiding this comment

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

With this change, the clang change doesn't actually do anything: we reserve the frame pointer whatever the user requests. Should we just drop the clang change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally, I'd like to keep the Clang change so that anything else in the frontend understands that frame pointers are reserved (not sure how vital or interesting that is though).

Happy to remove it if someone else has strong opinions.

Copy link
Member

Choose a reason for hiding this comment

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

I think just removing the clang change will fix the fact that this fails for flang, so it might be worth removing for that reason

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'm not familiar with how flang works, but if it's going to scrape(?) the command line args from clang, then it needs to be able to handle all the same command line arguments that clang does.

This is not a bug in clang: this is a bug in flang and would be hit if anyone tried building Arm32 code in flang.

return true;

// If the function has a frame pointer, it is reserved.
if (hasFP(MF))
return true;

// Frontend has requested to preserve the frame pointer.
if (TM.Options.FramePointerIsReserved(MF))
return true;

return false;
}

/// hasReservedCallFrame - Under normal circumstances, when a frame pointer is
/// not required, we reserve argument space for call sites in the function
/// immediately on entry to the current function. This eliminates the need for
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ class AArch64FrameLowering : public TargetFrameLowering {
orderFrameObjects(const MachineFunction &MF,
SmallVectorImpl<int> &ObjectsToAllocate) const override;

bool isFPReserved(const MachineFunction &MF) const;

protected:
bool hasFPImpl(const MachineFunction &MF) const override;

Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ AArch64RegisterInfo::getStrictlyReservedRegs(const MachineFunction &MF) const {
markSuperRegs(Reserved, AArch64::WSP);
markSuperRegs(Reserved, AArch64::WZR);

if (TFI->hasFP(MF) || TT.isOSDarwin())
if (TFI->isFPReserved(MF))
markSuperRegs(Reserved, AArch64::W29);

if (MF.getSubtarget<AArch64Subtarget>().isWindowsArm64EC()) {
Expand Down
25 changes: 22 additions & 3 deletions llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
; RUN: llc -mtriple=aarch64-none-linux-gnu -frame-pointer=all < %s | FileCheck %s
; RUN: llc -mtriple=aarch64-none-linux-gnu -frame-pointer=none < %s | \
; RUN: FileCheck %s --check-prefixes=CHECK,NONE
; RUN: llc -mtriple=aarch64-none-linux-gnu -frame-pointer=reserved < %s | \
; RUN: FileCheck %s --check-prefixes=CHECK,RESERVED
; RUN: llc -mtriple=aarch64-none-linux-gnu -frame-pointer=all < %s | \
; RUN: FileCheck %s --check-prefixes=CHECK,ALL

; By default, Darwin and Windows will reserve x29
; RUN: llc -mtriple=aarch64-darwin -frame-pointer=none < %s | \
; RUN: FileCheck %s --check-prefixes=CHECK,RESERVED
; RUN: llc -mtriple=aarch64-darwin -frame-pointer=none < %s | \
; RUN: FileCheck %s --check-prefixes=CHECK,RESERVED
@var = global i32 0

declare void @bar()

define void @test_w29_reserved() {
; CHECK-LABEL: test_w29_reserved:
; CHECK: mov x29, sp
; ALL: add x29, sp
; NONE-NOT: add x29
; NONE-NOT: mov x29
; RESERVED-NOT: add x29
; RESERVED-NOT: mov x29

%val1 = load volatile i32, ptr @var
%val2 = load volatile i32, ptr @var
Expand All @@ -16,8 +31,11 @@ define void @test_w29_reserved() {
%val7 = load volatile i32, ptr @var
%val8 = load volatile i32, ptr @var
%val9 = load volatile i32, ptr @var
%val10 = load volatile i32, ptr @var

; CHECK-NOT: ldr w29,
; NONE: ldr w29,
; ALL-NOT: ldr w29,
; RESERVED-NOT: ldr w29,

; Call to prevent fp-elim that occurs regardless in leaf functions.
call void @bar()
Expand All @@ -31,6 +49,7 @@ define void @test_w29_reserved() {
store volatile i32 %val7, ptr @var
store volatile i32 %val8, ptr @var
store volatile i32 %val9, ptr @var
store volatile i32 %val10, ptr @var

ret void
; CHECK: ret
Expand Down
48 changes: 32 additions & 16 deletions llvm/test/CodeGen/AArch64/win-sve.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,18 @@ define i32 @f(<vscale x 2 x i64> %x) {
; CHECK-NEXT: .seh_save_zreg z22, 16
; CHECK-NEXT: str z23, [sp, #17, mul vl] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_zreg z23, 17
; CHECK-NEXT: stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_fplr_x 16
; CHECK-NEXT: str x28, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x28, 16
; CHECK-NEXT: str x30, [sp, #8] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: .seh_endprologue
; CHECK-NEXT: bl g
; CHECK-NEXT: mov w0, #3 // =0x3
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: ldp x29, x30, [sp] // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_fplr 0
; CHECK-NEXT: ldr x30, [sp, #8] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: ldr x28, [sp] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x28, 0
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldr z8, [sp, #2, mul vl] // 16-byte Folded Reload
Expand Down Expand Up @@ -365,8 +369,10 @@ define void @f3(i64 %n, <vscale x 2 x i64> %x) {
; CHECK-NEXT: .seh_save_zreg z22, 16
; CHECK-NEXT: str z23, [sp, #17, mul vl] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_zreg z23, 17
; CHECK-NEXT: stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_fplr_x 16
; CHECK-NEXT: str x28, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x28, 16
; CHECK-NEXT: str x30, [sp, #8] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: sub sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: .seh_endprologue
Expand All @@ -376,8 +382,10 @@ define void @f3(i64 %n, <vscale x 2 x i64> %x) {
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldp x29, x30, [sp] // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_fplr 0
; CHECK-NEXT: ldr x30, [sp, #8] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: ldr x28, [sp] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x28, 0
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldr z8, [sp, #2, mul vl] // 16-byte Folded Reload
Expand Down Expand Up @@ -511,8 +519,10 @@ define void @f4(i64 %n, <vscale x 2 x i64> %x) {
; CHECK-NEXT: .seh_save_zreg z22, 16
; CHECK-NEXT: str z23, [sp, #17, mul vl] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_zreg z23, 17
; CHECK-NEXT: stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_fplr_x 16
; CHECK-NEXT: str x28, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x28, 16
; CHECK-NEXT: str x30, [sp, #8] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: sub sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: addvl sp, sp, #-1
Expand All @@ -526,8 +536,10 @@ define void @f4(i64 %n, <vscale x 2 x i64> %x) {
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldp x29, x30, [sp] // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_fplr 0
; CHECK-NEXT: ldr x30, [sp, #8] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: ldr x28, [sp] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x28, 0
; CHECK-NEXT: add sp, sp, #16
; CHECK-NEXT: .seh_stackalloc 16
; CHECK-NEXT: ldr z8, [sp, #2, mul vl] // 16-byte Folded Reload
Expand Down Expand Up @@ -1093,8 +1105,10 @@ define void @f7(i64 %n) {
; CHECK-LABEL: f7:
; CHECK: .seh_proc f7
; CHECK-NEXT: // %bb.0:
; CHECK-NEXT: stp x29, x30, [sp, #-16]! // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_fplr_x 16
; CHECK-NEXT: str x28, [sp, #-16]! // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg_x x28, 16
; CHECK-NEXT: str x30, [sp, #8] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: addvl sp, sp, #-1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: .seh_endprologue
Expand All @@ -1103,8 +1117,10 @@ define void @f7(i64 %n) {
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: addvl sp, sp, #1
; CHECK-NEXT: .seh_allocz 1
; CHECK-NEXT: ldp x29, x30, [sp], #16 // 16-byte Folded Reload
; CHECK-NEXT: .seh_save_fplr_x 16
; CHECK-NEXT: ldr x30, [sp, #8] // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg x30, 8
; CHECK-NEXT: ldr x28, [sp], #16 // 8-byte Folded Reload
; CHECK-NEXT: .seh_save_reg_x x28, 16
; CHECK-NEXT: .seh_endepilogue
; CHECK-NEXT: ret
; CHECK-NEXT: .seh_endfunclet
Expand Down
36 changes: 17 additions & 19 deletions llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,23 @@
; prologue has a corresponding seh directive.
;
; CHECK-NOT: error: Incorrect size for
; CHECK: foo:
; CHECK: .seh_proc foo
; CHECK: sub sp, sp, #288
; CHECK: .seh_stackalloc 288
; CHECK: str x19, [sp] // 8-byte Folded Spill
; CHECK: .seh_save_reg x19, 0
; CHECK: str x21, [sp, #8] // 8-byte Folded Spill
; CHECK: .seh_save_reg x21, 8
; CHECK: stp x23, x24, [sp, #16] // 16-byte Folded Spill
; CHECK: .seh_save_regp x23, 16
; CHECK: stp x25, x26, [sp, #32] // 16-byte Folded Spill
; CHECK: .seh_save_regp x25, 32
; CHECK: stp x27, x28, [sp, #48] // 16-byte Folded Spill
; CHECK: .seh_save_regp x27, 48
; CHECK: stp x29, x30, [sp, #64] // 16-byte Folded Spill
; CHECK: .seh_save_fplr 64
; CHECK: sub sp, sp, #224
; CHECK: .seh_stackalloc 224
; CHECK: .seh_endprologue
; CHECK-LABEL: foo:
; CHECK-NEXT: .seh_proc foo
; CHECK: sub sp, sp, #496
; CHECK-NEXT: .seh_stackalloc 496
; CHECK-NEXT: str x19, [sp, #208] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x19, 208
; CHECK-NEXT: str x21, [sp, #216] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x21, 216
; CHECK-NEXT: stp x23, x24, [sp, #224] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_regp x23, 224
; CHECK-NEXT: stp x25, x26, [sp, #240] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_regp x25, 240
; CHECK-NEXT: stp x27, x28, [sp, #256] // 16-byte Folded Spill
; CHECK-NEXT: .seh_save_regp x27, 256
; CHECK-NEXT: str x30, [sp, #272] // 8-byte Folded Spill
; CHECK-NEXT: .seh_save_reg x30, 272
; CHECK-NEXT: .seh_endprologue

target datalayout = "e-m:w-p:64:64-i32:32-i64:64-i128:128-n32:64-S128-Fn32"
target triple = "aarch64-unknown-windows-msvc19.42.34436"
Expand Down
12 changes: 8 additions & 4 deletions llvm/test/CodeGen/AArch64/wineh-frame5.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
# CHECK-LABEL: bb.0.entry:
# CHECK: early-clobber $sp = frame-setup STRXpre killed $x19, $sp, -32
# CHECK-NEXT: frame-setup SEH_SaveReg_X 19, -32
# CHECK-NEXT: frame-setup STPXi killed $fp, killed $lr, $sp, 1
# CHECK-NEXT: frame-setup SEH_SaveFPLR 8
# CHECK-NEXT: frame-setup STRXui killed $x28, $sp, 1
# CHECK-NEXT: frame-setup SEH_SaveReg 28, 8
# CHECK-NEXT: frame-setup STRXui killed $lr, $sp, 2
# CHECK-NEXT: frame-setup SEH_SaveReg 30, 16
# CHECK-NEXT: $sp = frame-setup SUBXri $sp, 496, 0
# CHECK-NEXT: frame-setup SEH_StackAlloc 496
# CHECK-NEXT: frame-setup SEH_PrologEnd
Expand All @@ -15,8 +17,10 @@
# CHECK: frame-destroy SEH_EpilogStart
# CHECK-NEXT: $sp = frame-destroy ADDXri $sp, 496, 0
# CHECK-NEXT: frame-destroy SEH_StackAlloc 496
# CHECK-NEXT: $fp, $lr = frame-destroy LDPXi $sp, 1
# CHECK-NEXT: frame-destroy SEH_SaveFPLR 8
# CHECK-NEXT: $lr = frame-destroy LDRXui $sp, 2
# CHECK-NEXT: frame-destroy SEH_SaveReg 30, 16
# CHECK-NEXT: $x28 = frame-destroy LDRXui $sp, 1
# CHECK-NEXT: frame-destroy SEH_SaveReg 28, 8
# CHECK-NEXT: early-clobber $sp, $x19 = frame-destroy LDRXpost $sp, 32
# CHECK-NEXT: frame-destroy SEH_SaveReg_X 19, -32
# CHECK-NEXT: frame-destroy SEH_EpilogEnd
Expand Down
14 changes: 9 additions & 5 deletions llvm/test/CodeGen/AArch64/wineh-frame7.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
# Test that stack probe results in Nop unwind codes in the prologue. Test
# save_fplr, save_reg_x and stack_alloc with multiple updates.

# CHECK: early-clobber $sp = frame-setup STPXpre killed $fp, killed $lr, $sp, -2
# CHECK-NEXT: frame-setup SEH_SaveFPLR_X -16
# CHECK: early-clobber $sp = frame-setup STRXpre killed $x28, $sp, -32
# CHECK-NEXT: frame-setup SEH_SaveReg_X 28, -32
# CHECK-NEXT: frame-setup STPXi killed $fp, killed $lr, $sp, 1
# CHECK-NEXT: frame-setup SEH_SaveFPLR 8
# CHECK-NEXT: $x15 = frame-setup MOVZXi 56009, 0
# CHECK-NEXT: frame-setup SEH_Nop
# CHECK-NEXT: $x15 = frame-setup MOVKXi $x15, 2, 16
# CHECK-NEXT: frame-setup SEH_Nop
# CHECK-NEXT: frame-setup BL &__chkstk, implicit-def $lr, implicit $sp, implicit $x15
# CHECK-NEXT: frame-setup BL &__chkstk, implicit-def $lr, implicit $sp, implicit $x15, implicit-def dead $x16, implicit-def dead $x17, implicit-def dead $nzcv
# CHECK-NEXT: frame-setup SEH_Nop
# CHECK-NEXT: $sp = frame-setup SUBXrx64 killed $sp, killed $x15, 28
# CHECK-NEXT: frame-setup SEH_StackAlloc 2993296
Expand All @@ -19,8 +21,10 @@
# CHECK-NEXT: frame-destroy SEH_StackAlloc 2990080
# CHECK-NEXT: $sp = frame-destroy ADDXri $sp, 3216, 0
# CHECK-NEXT: frame-destroy SEH_StackAlloc 3216
# CHECK-NEXT: early-clobber $sp, $fp, $lr = frame-destroy LDPXpost $sp, 2
# CHECK-NEXT: frame-destroy SEH_SaveFPLR_X -16
# CHECK-NEXT: $fp, $lr = frame-destroy LDPXi $sp, 1
# CHECK-NEXT: frame-destroy SEH_SaveFPLR 8
# CHECK-NEXT: early-clobber $sp, $x28 = frame-destroy LDRXpost $sp, 32
# CHECK-NEXT: frame-destroy SEH_SaveReg_X 28, -32
# CHECK-NEXT: frame-destroy SEH_EpilogEnd
# CHECK-NEXT: RET_ReallyLR implicit killed $w0
--- |
Expand Down
Loading