Skip to content

[win][aarch64] Always reserve frame pointers for Arm64 Windows, take 2 #147354

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

Jump to bottom
Merged
merged 1 commit into from
Jul 8, 2025
Merged

[win][aarch64] Always reserve frame pointers for Arm64 Windows, take 2 #147354

merged 1 commit into from
Jul 8, 2025
+131 −50

Conversation

dpaoliello
Copy link
Contributor

Re-land #146582 now that the Flang bugs have been fixed.

There is no way in Arm64 Windows to indicate that a given function has used the Frame Pointer as a General Purpose Register, as such stack walks will always assume that the frame chain is valid and will follow whatever value has been saved for the Frame Pointer (even if it is pointing to data, etc.).

This change makes the Frame Pointer always reserved when building for Arm64 Windows to avoid this issue.

We will be updating the official Windows ABI documentation to reflect this requirement, and I will provide a link once it's available.

@dpaoliello
[win][aarch64] Always reserve frame pointers for Arm64 Windows (llvm#…
e2e6c78 
…146582)

There is no way in Arm64 Windows to indicate that a given function has
used the Frame Pointer as a General Purpose Register, as such stack
walks will always assume that the frame chain is valid and will follow
whatever value has been saved for the Frame Pointer (even if it is
pointing to data, etc.).

This change makes the Frame Pointer always reserved when building for
Arm64 Windows to avoid this issue.

We will be updating the official Windows ABI documentation to reflect
this requirement, and I will provide a link once it's available.
@dpaoliello dpaoliello requested a review from efriedma-quic July 7, 2025 17:38
@llvmbot 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 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025 •
edited
Loading

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-backend-aarch64

Author: Daniel Paoliello (dpaoliello)

Changes

Re-land #146582 now that the Flang bugs have been fixed.

There is no way in Arm64 Windows to indicate that a given function has used the Frame Pointer as a General Purpose Register, as such stack walks will always assume that the frame chain is valid and will follow whatever value has been saved for the Frame Pointer (even if it is pointing to data, etc.).

This change makes the Frame Pointer always reserved when building for Arm64 Windows to avoid this issue.

We will be updating the official Windows ABI documentation to reflect this requirement, and I will provide a link once it's available.

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

10 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+13-2)
  • (modified) clang/test/Driver/frame-pointer-elim.c (+6)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+21)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.h (+2)
  • (modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp (+1-1)
  • (modified) llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll (+22-3)
  • (modified) llvm/test/CodeGen/AArch64/win-sve.ll (+32-16)
  • (modified) llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll (+17-19)
  • (modified) llvm/test/CodeGen/AArch64/wineh-frame5.mir (+8-4)
  • (modified) llvm/test/CodeGen/AArch64/wineh-frame7.mir (+9-5)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index bdd77ac84913c..9ae76fc307a56 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -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.
@@ -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
diff --git a/clang/test/Driver/frame-pointer-elim.c b/clang/test/Driver/frame-pointer-elim.c
index f64ff6efc7261..0dd7eb0c738db 100644
--- a/clang/test/Driver/frame-pointer-elim.c
+++ b/clang/test/Driver/frame-pointer-elim.c
@@ -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 | \
@@ -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(); }
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 6f1ce5bdbe286..3ef7e5265c724 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -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())
+    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
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.h b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
index e7d52bb350f13..ced69c9cd3699 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
@@ -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;
 
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index fb472ddc719fc..dd23bf51a98c4 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -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()) {
diff --git a/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll b/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
index 01943f40d41e8..347f777187af7 100644
--- a/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
+++ b/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
@@ -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
@@ -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()
@@ -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
diff --git a/llvm/test/CodeGen/AArch64/win-sve.ll b/llvm/test/CodeGen/AArch64/win-sve.ll
index 691230af3e67d..53ac9344175a3 100644
--- a/llvm/test/CodeGen/AArch64/win-sve.ll
+++ b/llvm/test/CodeGen/AArch64/win-sve.ll
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll b/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
index 2002c37cb2528..6d14abdc2ed75 100644
--- a/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
+++ b/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
@@ -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"
diff --git a/llvm/test/CodeGen/AArch64/wineh-frame5.mir b/llvm/test/CodeGen/AArch64/wineh-frame5.mir
index 180c20f0148f5..0589d97ca64a2 100644
--- a/llvm/test/CodeGen/AArch64/wineh-frame5.mir
+++ b/llvm/test/CodeGen/AArch64/wineh-frame5.mir
@@ -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
@@ -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
diff --git a/llvm/test/CodeGen/AArch64/wineh-frame7.mir b/llvm/test/CodeGen/AArch64/wineh-frame7.mir
index 6d44ad3716111..b30afd2b57153 100644
--- a/llvm/test/CodeGen/AArch64/wineh-frame7.mir
+++ b/llvm/test/CodeGen/AArch64/wineh-frame7.mir
@@ -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
@@ -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
 --- |

@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-clang

Author: Daniel Paoliello (dpaoliello)

Changes

Re-land #146582 now that the Flang bugs have been fixed.

There is no way in Arm64 Windows to indicate that a given function has used the Frame Pointer as a General Purpose Register, as such stack walks will always assume that the frame chain is valid and will follow whatever value has been saved for the Frame Pointer (even if it is pointing to data, etc.).

This change makes the Frame Pointer always reserved when building for Arm64 Windows to avoid this issue.

We will be updating the official Windows ABI documentation to reflect this requirement, and I will provide a link once it's available.

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

10 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+13-2)
  • (modified) clang/test/Driver/frame-pointer-elim.c (+6)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.cpp (+21)
  • (modified) llvm/lib/Target/AArch64/AArch64FrameLowering.h (+2)
  • (modified) llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp (+1-1)
  • (modified) llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll (+22-3)
  • (modified) llvm/test/CodeGen/AArch64/win-sve.ll (+32-16)
  • (modified) llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll (+17-19)
  • (modified) llvm/test/CodeGen/AArch64/wineh-frame5.mir (+8-4)
  • (modified) llvm/test/CodeGen/AArch64/wineh-frame7.mir (+9-5)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index bdd77ac84913c..9ae76fc307a56 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -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.
@@ -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
diff --git a/clang/test/Driver/frame-pointer-elim.c b/clang/test/Driver/frame-pointer-elim.c
index f64ff6efc7261..0dd7eb0c738db 100644
--- a/clang/test/Driver/frame-pointer-elim.c
+++ b/clang/test/Driver/frame-pointer-elim.c
@@ -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 | \
@@ -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(); }
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
index 6f1ce5bdbe286..3ef7e5265c724 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
@@ -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())
+    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
diff --git a/llvm/lib/Target/AArch64/AArch64FrameLowering.h b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
index e7d52bb350f13..ced69c9cd3699 100644
--- a/llvm/lib/Target/AArch64/AArch64FrameLowering.h
+++ b/llvm/lib/Target/AArch64/AArch64FrameLowering.h
@@ -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;
 
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index fb472ddc719fc..dd23bf51a98c4 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -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()) {
diff --git a/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll b/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
index 01943f40d41e8..347f777187af7 100644
--- a/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
+++ b/llvm/test/CodeGen/AArch64/regress-w29-reserved-with-fp.ll
@@ -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
@@ -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()
@@ -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
diff --git a/llvm/test/CodeGen/AArch64/win-sve.ll b/llvm/test/CodeGen/AArch64/win-sve.ll
index 691230af3e67d..53ac9344175a3 100644
--- a/llvm/test/CodeGen/AArch64/win-sve.ll
+++ b/llvm/test/CodeGen/AArch64/win-sve.ll
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll b/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
index 2002c37cb2528..6d14abdc2ed75 100644
--- a/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
+++ b/llvm/test/CodeGen/AArch64/wincfi-missing-seh-directives.ll
@@ -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"
diff --git a/llvm/test/CodeGen/AArch64/wineh-frame5.mir b/llvm/test/CodeGen/AArch64/wineh-frame5.mir
index 180c20f0148f5..0589d97ca64a2 100644
--- a/llvm/test/CodeGen/AArch64/wineh-frame5.mir
+++ b/llvm/test/CodeGen/AArch64/wineh-frame5.mir
@@ -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
@@ -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
diff --git a/llvm/test/CodeGen/AArch64/wineh-frame7.mir b/llvm/test/CodeGen/AArch64/wineh-frame7.mir
index 6d44ad3716111..b30afd2b57153 100644
--- a/llvm/test/CodeGen/AArch64/wineh-frame7.mir
+++ b/llvm/test/CodeGen/AArch64/wineh-frame7.mir
@@ -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
@@ -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
 --- |

efriedma-quic
efriedma-quic approved these changes Jul 8, 2025
View reviewed changes
Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

LGTM.

(Sorry, didn't realize you were waiting for me on this. It's exactly the same patch I already approved.)

@dpaoliello dpaoliello merged commit 1762b30 into llvm:main Jul 8, 2025
13 checks passed
@dpaoliello dpaoliello deleted the arm64winfp branch July 8, 2025 19:18
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 8, 2025

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building clang,llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/26338

Here is the relevant piece of the build log for the reference 
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/lua.test (3058 of 3069)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/breakpoint_oneline_callback.test (3059 of 3069)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/no_threadState.test (3060 of 3069)
UNSUPPORTED: lldb-shell :: Register/riscv64-gp-read.test (3061 of 3069)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Python/Crashlog/last_exception_backtrace_crashlog.test (3062 of 3069)
UNSUPPORTED: lldb-shell :: ScriptInterpreter/Lua/fail_breakpoint_oneline.test (3063 of 3069)
UNSUPPORTED: lldb-shell :: SymbolFile/PDB/type-quals.test (3064 of 3069)
UNSUPPORTED: lldb-shell :: Register/loongarch64-gp-read.test (3065 of 3069)
PASS: lldb-api :: terminal/TestEditlineCompletions.py (3066 of 3069)
UNRESOLVED: lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py (3067 of 3069)
******************** TEST 'lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib --cmake-build-type Release -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch -p TestDAP_launch.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 1762b3043c5ec58cfb2be8911535c3cd4d1f64b0)
  clang revision 1762b3043c5ec58cfb2be8911535c3cd4d1f64b0
  llvm revision 1762b3043c5ec58cfb2be8911535c3cd4d1f64b0
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch
runCmd: settings clear --all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output: 

runCmd: settings set target.auto-apply-fixits false

@klausler
Copy link
Contributor

klausler commented Jul 9, 2025

Is it possible that this commit has broken the Windows flang-new build bot? There were a lot of changes that went in at the same time and it's hard to identify the culprit.

@efriedma-quic
Copy link
Collaborator

You mean https://lab.llvm.org/buildbot/#/builders/207/builds/3640 ? Yes, that's this patch.

@klausler
Copy link
Contributor

klausler commented Jul 9, 2025

Is anybody working on a fix or reversion?

@dpaoliello
Copy link
Contributor Author

Is anybody working on a fix or reversion?

I can work on a fix: should be trivial.

@dpaoliello
Copy link
Contributor Author

Fix: #147837

@luporl
Copy link
Contributor

luporl commented Jul 10, 2025

Thanks for the fix.
But Clang::frame-pointer-elim.c test also started failing on Windows on ARM with this PR:

Can you please take a look?

@dpaoliello
Copy link
Contributor Author

Thanks for the fix. But Clang::frame-pointer-elim.c test also started failing on Windows on ARM with this PR:

Can you please take a look?

PR to fix: #148000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efriedma-quic efriedma-quic efriedma-quic approved these changes

Assignees
No one assigned
Labels
backend:AArch64 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dpaoliello @llvmbot @llvm-ci @klausler @efriedma-quic @luporl