[AArch64] [GlobalISel] Fix clobbered callee saved registers with win64 varargs

[mstorsjo]

Summary

This fixes a regression since 1c10d5b175992a9d056a2d763a932e5652386fc1
/ https://reviews.llvm.org/D130903 by applying the same fix from
SelectionDAG from 8cb3667541a94c4fa11b06e19020f753414c1d03 /
https://reviews.llvm.org/D35720.

This could possibly have been detected if the existing testcases
in win64_vararg.ll had been tested with GlobalISel too, but all
the IR snippets there fail to be translated with GlobalISel.

This adds a separate testcase based on real world LLVM IR (instead of
hand-reduced IR), which GlobalISel does translate happily - tested
with both SelectionDAG and GlobalISel.

Before this change, the stack object locations (visible in MIR
with "llc -print-after-all") didn't match with what the prologue
emitted by AArch64FrameLowering actually looked like, which caused
clobbered callee saved registers when function local stack objects
aliased the actual location of the callee saved registers.

This fixes https://github.com/llvm/llvm-project/issues/64740.

[arsenm]

llvm/test/CodeGen/AArch64/win64_vararg2.ll

73–74

Use named values

[tschuett]

llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

567

Is this:
if (GPRSaveSize & 15 > 0) or
if (GPRSaveSize & 15 == 15) ?

[mstorsjo]

llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

567

It's if ((GPRSaveSize & 15) > 0) - or practically if ((GPRSaveSize & 15) == 8) as GPRSaveSize is in increments of 8. It's essentially checking whether GPRArgRegs.size() - FirstVariadicGPR is even or odd.

This is kept very similar to https://github.com/llvm/llvm-project/blob/llvmorg-17.0.0-rc2/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp#L6699-L6706.

llvm/test/CodeGen/AArch64/win64_vararg2.ll

73–74

Ok, I can change it that way.

FWIW, this was just the pretty much unreduced output from Clang. We've got a bunch of preexisting similar hand-reduced IR cases in win64_vararg.ll, but all of those make globalisel bail out, so I wanted to have a case that globalisel actually does accept.

[mstorsjo]

Simplified the testcase a little, rewrote the test IR to use named values.

[dzhidzhoev]

llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

570

nit: static_cast?

[mstorsjo]

Use static_cast

[dzhidzhoev]

Looks good to me

[mstorsjo]

In D158272#4600549, @dzhidzhoev wrote:

Looks good to me

Thanks! Would you care to set the formal approval flag as well? (I'd like to get this landed so I can flag it for backporting soon - this can cause subtle breakage for any vararg function on windows/aarch64, if compiled without optimizations.)

For context; D130903 ported a bunch of logic from SelectionDAG to GlobalISel, but skipped these lines as they were believed to not be necessary - but they turned out to be needed after all. This patch readds the few lines that were skipped. (They have been in place in the SelectionDAG implementation since 2017.)

llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

void AArch64CallLowering::saveVarArgRegisters(
   unsigned NumVariadicGPRArgRegs = GPRArgRegs.size() - FirstVariadicGPR + 1;
 
   unsigned GPRSaveSize = 8 * (GPRArgRegs.size() - FirstVariadicGPR);
   int GPRIdx = 0;
   if (GPRSaveSize != 0) {
     if (IsWin64CC) {
       GPRIdx = MFI.CreateFixedObject(GPRSaveSize,
                                      -static_cast<int>(GPRSaveSize), false);
+      if (GPRSaveSize & 15)
+        // The extra size here, if triggered, will always be 8.
+        MFI.CreateFixedObject(16 - (GPRSaveSize & 15),
+                              -static_cast<int>(alignTo(GPRSaveSize, 16)),
+                              false);
     } else
       GPRIdx = MFI.CreateStackObject(GPRSaveSize, Align(8), false);
 
     auto FIN = MIRBuilder.buildFrameIndex(p0, GPRIdx);
     auto Offset =
         MIRBuilder.buildConstant(MRI.createGenericVirtualRegister(s64), 8);
 
     for (unsigned i = FirstVariadicGPR; i < GPRArgRegs.size(); ++i) {

llvm/test/CodeGen/AArch64/win64_vararg2.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=aarch64-pc-win32 | FileCheck %s
+; RUN: llc < %s -global-isel -mtriple=aarch64-pc-win32 | FileCheck %s --check-prefix=GISEL
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define i1 @va_func(i32 %a, i8 %b, i8 %c, ...) {
+; CHECK-LABEL: va_func:
+; CHECK:       .seh_proc va_func
+; CHECK-NEXT:  // %bb.0:
+; CHECK-NEXT:    sub sp, sp, #80
+; CHECK-NEXT:    .seh_stackalloc 80
+; CHECK-NEXT:    str x19, [sp, #16] // 8-byte Folded Spill
+; CHECK-NEXT:    .seh_save_reg x19, 16
+; CHECK-NEXT:    str x30, [sp, #24] // 8-byte Folded Spill
+; CHECK-NEXT:    .seh_save_reg x30, 24
+; CHECK-NEXT:    .seh_endprologue
+; CHECK-NEXT:    mov w19, w0
+; CHECK-NEXT:    stp x3, x4, [sp, #40]
+; CHECK-NEXT:    stp x5, x6, [sp, #56]
+; CHECK-NEXT:    str x7, [sp, #72]
+; CHECK-NEXT:    str w0, [sp, #12]
+; CHECK-NEXT:    strb w1, [sp, #11]
+; CHECK-NEXT:    strb w2, [sp, #10]
+; CHECK-NEXT:    bl other
+; CHECK-NEXT:    cmp w19, w0
+; CHECK-NEXT:    cset w0, ls
+; CHECK-NEXT:    .seh_startepilogue
+; CHECK-NEXT:    ldr x30, [sp, #24] // 8-byte Folded Reload
+; CHECK-NEXT:    .seh_save_reg x30, 24
+; CHECK-NEXT:    ldr x19, [sp, #16] // 8-byte Folded Reload
+; CHECK-NEXT:    .seh_save_reg x19, 16
+; CHECK-NEXT:    add sp, sp, #80
+; CHECK-NEXT:    .seh_stackalloc 80
+; CHECK-NEXT:    .seh_endepilogue
+; CHECK-NEXT:    ret
+; CHECK-NEXT:    .seh_endfunclet
+; CHECK-NEXT:    .seh_endproc
+;
+; GISEL-LABEL: va_func:
+; GISEL:       .seh_proc va_func
+; GISEL-NEXT:  // %bb.0:
+; GISEL-NEXT:    sub sp, sp, #80
+; GISEL-NEXT:    .seh_stackalloc 80
+; GISEL-NEXT:    str x19, [sp, #16] // 8-byte Folded Spill
+; GISEL-NEXT:    .seh_save_reg x19, 16
+; GISEL-NEXT:    str x30, [sp, #24] // 8-byte Folded Spill
+; GISEL-NEXT:    .seh_save_reg x30, 24
+; GISEL-NEXT:    .seh_endprologue
+; GISEL-NEXT:    stp x3, x4, [sp, #40]
+; GISEL-NEXT:    mov w19, w0
+; GISEL-NEXT:    stp x5, x6, [sp, #56]
+; GISEL-NEXT:    str w0, [sp, #12]
+; GISEL-NEXT:    str x7, [sp, #72]
+; GISEL-NEXT:    strb w1, [sp, #11]
+; GISEL-NEXT:    strb w2, [sp, #10]
+; GISEL-NEXT:    bl other
+; GISEL-NEXT:    cmp w19, w0
+; GISEL-NEXT:    cset w0, ls
+; GISEL-NEXT:    .seh_startepilogue
+; GISEL-NEXT:    ldr x30, [sp, #24] // 8-byte Folded Reload
+; GISEL-NEXT:    .seh_save_reg x30, 24
+; GISEL-NEXT:    ldr x19, [sp, #16] // 8-byte Folded Reload
+; GISEL-NEXT:    .seh_save_reg x19, 16
+; GISEL-NEXT:    add sp, sp, #80
+; GISEL-NEXT:    .seh_stackalloc 80
+; GISEL-NEXT:    .seh_endepilogue
+; GISEL-NEXT:    ret
+; GISEL-NEXT:    .seh_endfunclet
+; GISEL-NEXT:    .seh_endproc
+  %a_alloc = alloca i32, align 4
+  %b_alloc = alloca i8, align 1
+  %c_alloc = alloca i8, align 1
+  store i32 %a, ptr %a_alloc, align 4
+  store i8 %b, ptr %b_alloc, align 1
+  store i8 %c, ptr %c_alloc, align 1
+  %a_load = load i32, ptr %a_alloc, align 4
+  %ret = call noundef i32 @other()
+  %cmp = icmp ule i32 %a_load, %ret
+  ret i1 %cmp
+}
+
+declare i32 @other()