[AArch64] Treat x18 as callee-saved in functions with Windows calling convention on Darwin

[dzhidzhoev]

Summary

rGcf97e0ec42b8 makes $x18 to be treated as callee-saved in functions with
Windows calling convention on non-Windows OSes.

Here we mark $x18 as callee-saved for functions with Windows calling
convention on Darwin, as well as on other non-Windows platforms, in
order to prevent some miscompilations (like miscompilation of
win64cc-darwin-backup-x18.ll).

Since getCalleeSavedRegs doesn't return x18 in list of callee-saved
registers, assignCalleeSavedSpillSlots and determineCalleeSaves
consider different sets of registers as callee-saved. It causes an
error:

Assertion failed: ((!HasCalleeSavedStackSize || getCalleeSavedStackSize() == Size) && "Invalid size calculated for callee saves"), function getCalleeSavedStackSize, file 
AArch64MachineFunctionInfo.h, line 292.

[mstorsjo]

I think this looks reasonable, but I’ll have a closer read later.

Where did you trigger this issue - by building Wine on macOS? (I did try building Wine on macOS/arm64 in 2020, and the logic for backing up x18 landed in 2019, so I wonder why I didn’t notice this issue… Maybe the Xcode clang didn’t contain that change yet at that point, or maybe it’s not noticed or the compiler is built without assertions?)

FWIW, with a recent refactoring in Wine, where a proper syscall interface is used between windows code and the native unix code, I think the whole mechanism for backing up/restoring x18 might be unnecessary, so we could consider ripping it out altogether too. I guess I could run some tests to see what the current state is…

[dzhidzhoev]

In D130676#3684388, @mstorsjo wrote:

Where did you trigger this issue - by building Wine on macOS? (I did try building Wine on macOS/arm64 in 2020, and the logic for backing up x18 landed in 2019, so I wonder why I didn’t notice this issue… Maybe the Xcode clang didn’t contain that change yet at that point, or maybe it’s not noticed or the compiler is built without assertions?)

I was trying to investigate this issue https://github.com/llvm/llvm-project/issues/54079 and have found that the source code that doesn't compile with GlobalISel don't compile with SelectionDAG too, with debug clang build.

In D130676#3684388, @mstorsjo wrote:

FWIW, with a recent refactoring in Wine, where a proper syscall interface is used between windows code and the native unix code, I think the whole mechanism for backing up/restoring x18 might be unnecessary, so we could consider ripping it out altogether too. I guess I could run some tests to see what the current state is…

TBH I'm not familiar with running Windows code on Darwin, so I would appreciate your feedback.

[mstorsjo]

LGTM, thanks!

In practice, Wine works mostly quite well without this compiler help feature now, so we could consider removing the whole concept of extra backup of x18 on non-Windows platforms sometime in the future too, but I think it's best to wait until Wine has completed their transition to split all code that calls into host libraries behind a syscall interface (aka the PE split).

llvm/lib/Target/AArch64/AArch64CallingConvention.td

 def CSR_Darwin_AArch64_AAPCS : CalleeSavedRegs<(add LR, FP, X19, X20, X21, X22,
                                                 X23, X24, X25, X26, X27, X28,
                                                 D8,  D9,  D10, D11,
                                                 D12, D13, D14, D15)>;
 
 def CSR_Darwin_AArch64_AAVPCS : CalleeSavedRegs<(add LR, FP, X19, X20, X21,
                                                  X22, X23, X24, X25, X26, X27,
                                                  X28, (sequence "Q%u", 8, 23))>;
+
+// For Windows calling convention on a non-windows OS, where X18 is treated
+// as reserved, back up X18 when entering non-windows code (marked with the
+// Windows calling convention) and restore when returning regardless of
+// whether the individual function uses it - it might call other functions
+// that clobber it.
+def CSR_Darwin_AArch64_AAPCS_Win64
+    : CalleeSavedRegs<(add CSR_Darwin_AArch64_AAPCS, X18)>;
+
 def CSR_Darwin_AArch64_AAPCS_ThisReturn
     : CalleeSavedRegs<(add CSR_Darwin_AArch64_AAPCS, X0)>;
 
 def CSR_Darwin_AArch64_AAPCS_SwiftError
     : CalleeSavedRegs<(sub CSR_Darwin_AArch64_AAPCS, X21)>;
 
 def CSR_Darwin_AArch64_AAPCS_SwiftTail
     : CalleeSavedRegs<(sub CSR_Darwin_AArch64_AAPCS, X20, X22)>;

llvm/lib/Target/AArch64/AArch64FrameLowering.cpp

static void computeCalleeSaveRegisterPairs(
   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.
-  assert((!produceCompactUnwindFrame(MF) ||
-          CC == CallingConv::PreserveMost || CC == CallingConv::CXX_FAST_TLS ||
+  assert((!produceCompactUnwindFrame(MF) || CC == CallingConv::PreserveMost ||
+          CC == CallingConv::CXX_FAST_TLS || CC == CallingConv::Win64 ||
           (Count & 1) == 0) &&
          "Odd number of callee-saved regs to spill!");
   int ByteOffset = AFI->getCalleeSavedStackSize();
   int StackFillDir = -1;
   int RegInc = 1;
   unsigned FirstReg = 0;
   if (NeedsWinCFI) {
     // For WinCFI, fill the stack from the bottom up.
for (unsigned i = FirstReg; i < Count; i += RegInc) {
 
     // 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 || CC == CallingConv::CXX_FAST_TLS ||
+    assert((!produceCompactUnwindFrame(MF) || CC == CallingConv::PreserveMost ||
+            CC == CallingConv::CXX_FAST_TLS || CC == CallingConv::Win64 ||
             (RPI.isPaired() &&
              ((RPI.Reg1 == AArch64::LR && RPI.Reg2 == AArch64::FP) ||
               RPI.Reg1 + 1 == RPI.Reg2))) &&
            "Callee-save registers not saved as adjacent register pair!");
 
     RPI.FrameIdx = CSI[i].getFrameIdx();
     if (NeedsWinCFI &&
         RPI.isPaired()) // RPI.FrameIdx must be the lower index of the pair

llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp

if (MF->getSubtarget<AArch64Subtarget>().getTargetLowering()
           ->supportSwiftError() &&
       MF->getFunction().getAttributes().hasAttrSomewhere(
           Attribute::SwiftError))
     return CSR_Darwin_AArch64_AAPCS_SwiftError_SaveList;
   if (MF->getFunction().getCallingConv() == CallingConv::SwiftTail)
     return CSR_Darwin_AArch64_AAPCS_SwiftTail_SaveList;
   if (MF->getFunction().getCallingConv() == CallingConv::PreserveMost)
     return CSR_Darwin_AArch64_RT_MostRegs_SaveList;
+  if (MF->getFunction().getCallingConv() == CallingConv::Win64)
+    return CSR_Darwin_AArch64_AAPCS_Win64_SaveList;
   return CSR_Darwin_AArch64_AAPCS_SaveList;
 }
 
 const MCPhysReg *AArch64RegisterInfo::getCalleeSavedRegsViaCopy(
     const MachineFunction *MF) const {
   assert(MF && "Invalid MachineFunction pointer.");
   if (MF->getFunction().getCallingConv() == CallingConv::CXX_FAST_TLS &&
       MF->getInfo<AArch64FunctionInfo>()->isSplitCSR())

llvm/test/CodeGen/AArch64/win64cc-darwin-backup-x18.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+
+;; Testing that x18 is backed up and restored, and that x29 (if used) still
+;; points to the x29,x30 pair on the stack.
+
+; RUN: llc < %s -mtriple=arm64-apple-darwin | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-apple-darwin -mattr=+reserve-x18 | FileCheck %s
+
+declare dso_local void @other()
+
+define win64cc void @func(i32 noundef %0) #0 {
+; CHECK-LABEL: func:
+; CHECK:         ; %bb.0: ; %entry
+; CHECK-NEXT:    str x18, [sp, #-32]! ; 8-byte Folded Spill
+; CHECK-NEXT:    stp x29, x30, [sp, #16] ; 16-byte Folded Spill
+; CHECK:         ldp x29, x30, [sp, #16] ; 16-byte Folded Reload
+; CHECK-NEXT:    ldr x18, [sp], #32 ; 8-byte Folded Reload
+; CHECK-NEXT:    ret
+entry:
+  %al = alloca i32, align 4
+  store i32 %0, ptr %al, align 4
+  tail call void @other()
+  ret void
+}
+
+define i32 @main() {
+  call win64cc void @func(i32 noundef 17)
+  ret i32 0
+}
+
+attributes #0 = { nounwind }