[AArch64] Fix save register pairing for Windows AAPCS

[sanwou01]

Summary

On Windows, when a function does not have an unwind table (for example, EH
filtering funclets), we don't correctly pair FP and LR to form the frame record
in all circumstances.

Fix this by invalidating a pair when the second register is FP when compiling
for Windows, even when CFI is not needed.

Fixes PR44271 introduced by D65653.

[rengolin]

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

1877

This seems awfully permissive. Shouldn't you check that the pairs are explicitly:

FP + LR
LR + FP

1986

Is Windows' only ABI WindowsAAPCS?

[sanwou01]

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

1877

Hm, this invalidates any pair with FP as the second register, so it follows that FP will be the first register of a pair (on Windows/WinAAPCS). The assert below (line 2028) checks that when FP is the first register of a pair, LR is the second.

Maybe I misunderstood your comment, but that seems quite precise to me!

1986

Good point. It might be so in the AArch64 backend, but who knows when a new ABI comes around.

[rengolin]

LGTM, thanks!

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

1877

Right, my comment was about the assert below, but I guess you're right. The code that uses this function is not that trivial to read. :)

1986

Right, we change that when it does. :)

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

static bool needsWinCFI(const MachineFunction &MF) {
   return MF.getTarget().getMCAsmInfo()->usesWindowsCFI() &&
          F.needsUnwindTableEntry();
 }
 
 static bool isTargetDarwin(const MachineFunction &MF) {
   return MF.getSubtarget<AArch64Subtarget>().isTargetDarwin();
 }
 
+static bool isTargetWindows(const MachineFunction &MF) {
+  return MF.getSubtarget<AArch64Subtarget>().isTargetWindows();
+}
+
 // Convenience function to determine whether I is an SVE callee save.
 static bool IsSVECalleeSave(MachineBasicBlock::iterator I) {
   switch (I->getOpcode()) {
   default:
     return false;
   case AArch64::STR_ZXI:
   case AArch64::STR_PXI:
   case AArch64::LDR_ZXI:
static bool invalidateWindowsRegisterPairing(unsigned Reg1, unsigned Reg2,
   // If we are generating register pairs for a Windows function that requires
   // EH support, then pair consecutive registers only.  There are no unwind
   // opcodes for saves/restores of non-consectuve register pairs.
   // The unwind opcodes are save_regp, save_regp_x, save_fregp, save_frepg_x.
   // https://docs.microsoft.com/en-us/cpp/build/arm64-exception-handling
 
   // TODO: LR can be paired with any register.  We don't support this yet in
   // the MCLayer.  We need to add support for the save_lrpair unwind code.
+  if (Reg2 == AArch64::FP)
+    return true;
   if (!NeedsWinCFI)
     return false;
   if (Reg2 == Reg1 + 1)
     return false;
   return true;
 }
 
 /// Returns true if Reg1 and Reg2 cannot be paired using a ldp/stp instruction.
 /// WindowsCFI requires that only consecutive registers can be paired.
 /// LR and FP need to be allocated together when the frame needs to save
 /// the frame-record. This means any other register pairing with LR is invalid.
 static bool invalidateRegisterPairing(unsigned Reg1, unsigned Reg2,
-                                      bool NeedsWinCFI, bool NeedsFrameRecord) {
-  if (NeedsWinCFI)
-    return invalidateWindowsRegisterPairing(Reg1, Reg2, true);
+                                      bool UsesWinAAPCS, bool NeedsWinCFI, bool NeedsFrameRecord) {
+  if (UsesWinAAPCS)
+    return invalidateWindowsRegisterPairing(Reg1, Reg2, NeedsWinCFI);
 
   // If we need to store the frame record, don't pair any register
   // with LR other than FP.
   if (NeedsFrameRecord)
     return Reg2 == AArch64::LR;
 
   return false;
 }

 static void computeCalleeSaveRegisterPairs(
     MachineFunction &MF, const std::vector<CalleeSavedInfo> &CSI,
     const TargetRegisterInfo *TRI, SmallVectorImpl<RegPairInfo> &RegPairs,
     bool &NeedShadowCallStackProlog, bool NeedsFrameRecord) {
 
   if (CSI.empty())
     return;
 
+  bool IsWindows = isTargetWindows(MF);
   bool NeedsWinCFI = needsWinCFI(MF);
   AArch64FunctionInfo *AFI = MF.getInfo<AArch64FunctionInfo>();
   MachineFrameInfo &MFI = MF.getFrameInfo();
   CallingConv::ID CC = MF.getFunction().getCallingConv();
   unsigned Count = CSI.size();
   (void)CC;
   // MachO's compact unwind format relies on all registers being stored in
   // pairs.
else
       llvm_unreachable("Unsupported register class.");
 
     // Add the next reg to the pair if it is in the same register class.
     if (i + 1 < Count) {
       unsigned NextReg = CSI[i + 1].getReg();
       switch (RPI.Type) {
       case RegPairInfo::GPR:
         if (AArch64::GPR64RegClass.contains(NextReg) &&
-            !invalidateRegisterPairing(RPI.Reg1, NextReg, NeedsWinCFI,
+            !invalidateRegisterPairing(RPI.Reg1, NextReg, IsWindows, NeedsWinCFI,
                                        NeedsFrameRecord))
           RPI.Reg2 = NextReg;
         break;
       case RegPairInfo::FPR64:
         if (AArch64::FPR64RegClass.contains(NextReg) &&
             !invalidateWindowsRegisterPairing(RPI.Reg1, NextReg, NeedsWinCFI))
           RPI.Reg2 = NextReg;
         break;
for (unsigned i = 0; i < Count; ++i) {
     assert((!RPI.isPaired() ||
             (CSI[i].getFrameIdx() + 1 == CSI[i + 1].getFrameIdx())) &&
            "Out of order callee saved regs!");
 
     assert((!RPI.isPaired() || !NeedsFrameRecord || RPI.Reg2 != AArch64::FP ||
             RPI.Reg1 == AArch64::LR) &&
            "FrameRecord must be allocated together with LR");
 
+    // Windows AAPCS has FP and LR reversed.
+    assert((!RPI.isPaired() || !NeedsFrameRecord || RPI.Reg1 != AArch64::FP ||
+            RPI.Reg2 == AArch64::LR) &&
+           "FrameRecord must be allocated together with LR");
+
     // MachO's compact unwind format relies on all registers being stored in
     // adjacent register pairs.
     assert((!produceCompactUnwindFrame(MF) ||
             CC == CallingConv::PreserveMost ||
             (RPI.isPaired() &&
              ((RPI.Reg1 == AArch64::LR && RPI.Reg2 == AArch64::FP) ||
               RPI.Reg1 + 1 == RPI.Reg2))) &&
            "Callee-save registers not saved as adjacent register pair!");

llvm/test/CodeGen/AArch64/win64-no-uwtable.ll

+; RUN: llc -mtriple=arm64-windows -o - %s | FileCheck %s
+
+declare void @f()
+declare void @g()
+
+; Function Attrs: nounwind
+define dso_local void @SEHfilter() nounwind "frame-pointer"="all" {
+; CHECK-LABEL: @SEHfilter
+; CHECK:       %bb.0:
+; CHECK-NEXT:  stp     x30, x29, [sp, #-32]!
+; CHECK-NEXT:  str     x19, [sp, #16]
+; CHECK-NEXT:  ldr     w19, [x8]
+; CHECK-NEXT:  mov     x29, sp
+; CHECK-NEXT:  bl      g
+; CHECK-NEXT:  cbz     w19, .LBB0_2
+; CHECK-NEXT:  ; %bb.1:
+; CHECK-NEXT:  ldr     x19, [sp, #16]
+; CHECK-NEXT:  ldp     x30, x29, [sp], #32
+; CHECK-NEXT:  ret
+; CHECK-NEXT:  .LBB0_2:                                ; %if.end.i
+; CHECK-NEXT:  bl      f
+; CHECK-NEXT:  brk     #0x1
+  %1 = load i32, i32* undef, align 4
+  tail call void @g()
+  %tobool.i = icmp eq i32 %1, 0
+  br i1 %tobool.i, label %if.end.i, label %exit
+
+if.end.i:
+  call void @f()
+  unreachable
+
+exit:
+  ret void
+}
+