[X86][MS] Fix the aligement mismatch of vector variable arguments on Win32

[pengfei]

Summary

The alignment of vector variable arguments in callee side is 4, which is
aligned with MSVC. But the caller aligns them to the size of vector
arguments. It results in run fails. This patch fixes this problem by
trimming it to 4 bytes for variable arguments on Win32.

Fixed vector arguments are passed by pointer on Win32. So they don't have
the problem.

I don't find a doc in MSDN for this calling conversion, so I did several
experiments here: https://godbolt.org/z/n1zn1Gx1z

[rnk]

Thanks!

llvm/lib/Target/X86/X86CallingConv.td

29–30

Please check what mingw does in this case.

[pengfei]

Add test for mingw.

[pengfei]

llvm/lib/Target/X86/X86CallingConv.td

29–30

With this patch, mingw also uses 4 bytes alignment for both caller and callee.
But I'm not sure if it is the correct approach since I don't have a mingw environment to test.
@LiuChen3 fixed Linux 32 bits problem in D78564 by changing callee's alignment to the vector size. Not sure if we should fix in that way.

[rnk]

llvm/lib/Target/X86/X86CallingConv.td

29–30

@mstorsjo , do you mind checking what GCC does on Windows here? Mainly I'm looking out for the possibility that GCC passes such vectors indirectly when passed through varargs.

[mstorsjo]

llvm/lib/Target/X86/X86CallingConv.td

29–30

Sorry for the delay. I checked the testcase from Compiler Explorer with GCC targeting mingw (10.2), and I get this:

__Z8testm128iz:
        pushl   %ebp
        movl    %esp, %ebp
        leal    27(%ebp), %eax
        andl    $-16, %eax
        vmovups (%eax), %xmm0
        vmovups %xmm0, _res1
        movl    8(%ebp), %eax
        popl    %ebp
        ret
[...]
__Z17testPastArgumentsv:
        pushl   %ebp
        movl    %esp, %ebp
        andl    $-16, %esp
        subl    $32, %esp
        vmovups _a, %xmm0
        vmovups %xmm0, 16(%esp)
        movl    $1, (%esp)
        call    __Z8testm128iz
        leave
        ret

So based on that, I'd say that GCC consistently aligns them as a caller, and considers them aligned as a callee. So to be compatible, we'd need to do things the other way for mingw targets.

Then again, I guess such a thing could be considered a calling convention bug in GCC which might be fixable too (with a minor ABI break, but I guess such cases are rare in practice), and I'm not sure if they care about being calling convention compatible with MSVC regarding vector arguments either.

[rnk]

llvm/lib/Target/X86/X86CallingConv.td

29–30

No problem, I think this just means we should tweak this isOSWindows condition to isWindowsMSVCEnvironment.

How do I test mingw on compiler explorer? That would be handy for me.

[mstorsjo]

llvm/lib/Target/X86/X86CallingConv.td

29–30

I don't think compiler explorer has mingw GCC compilers unfortunately, so I just tested with a local cross compiler I happened to have around. (I have no idea why they don't provide that configuration there, especially as they have a MSVC-in-wine setup anyway.)

[pengfei]

Keep the change working only for MSVC compatiblity. Mingw is using a different alignment.
Thanks Reid and Martin for the help.

[pengfei]

Remove callee test since the alignment of va variables is generated in front end.

[rnk]

lgtm, thanks!

[pengfei]

Thanks @rnk and @mstorsjo

llvm/lib/Target/X86/X86CallingConv.td

: CCIf<!strconcat("static_cast<const X86Subtarget&>"
            A>;
 
 /// CCIfNotSubtarget - Match if the current subtarget doesn't has a feature F.
 class CCIfNotSubtarget<string F, CCAction A>
     : CCIf<!strconcat("!static_cast<const X86Subtarget&>"
                        "(State.getMachineFunction().getSubtarget()).", F),
            A>;
 
+/// CCIfIsVarArgOnWin - Match if isVarArg on Windows 32bits.
+class CCIfIsVarArgOnWin<CCAction A>
+    : CCIf<"State.isVarArg() && "
+           "State.getMachineFunction().getSubtarget().getTargetTriple()."
+           "isWindowsMSVCEnvironment()",
+           A>;
+
 // Register classes for RegCall
 class RC_X86_RegCall {
   list<Register> GPR_8 = [];
   list<Register> GPR_16 = [];
   list<Register> GPR_32 = [];
   list<Register> GPR_64 = [];
   list<Register> FP_CALL = [FP0];
   list<Register> FP_RET = [FP0, FP1];
def CC_X86_32_Vector_Common : CallingConv<[
   CCIfType<[v32i8, v16i16, v8i32, v4i64, v16f16, v8f32, v4f64],
            CCAssignToStack<32, 32>>,
 
   // 512-bit AVX 512-bit vectors get 64-byte stack slots that are 64-byte aligned.
   CCIfType<[v64i8, v32i16, v16i32, v8i64, v32f16, v16f32, v8f64],
            CCAssignToStack<64, 64>>
 ]>;
 
+/// CC_X86_Win32_Vector - In X86 Win32 calling conventions, extra vector
+/// values are spilled on the stack.
+def CC_X86_Win32_Vector : CallingConv<[
+  // Other SSE vectors get 16-byte stack slots that are 4-byte aligned.
+  CCIfType<[v16i8, v8i16, v4i32, v2i64, v8f16, v4f32, v2f64],
+           CCAssignToStack<16, 4>>,
+
+  // 256-bit AVX vectors get 32-byte stack slots that are 4-byte aligned.
+  CCIfType<[v32i8, v16i16, v8i32, v4i64, v16f16, v8f32, v4f64],
+           CCAssignToStack<32, 4>>,
+
+  // 512-bit AVX 512-bit vectors get 64-byte stack slots that are 4-byte aligned.
+  CCIfType<[v64i8, v32i16, v16i32, v8i64, v32f16, v16f32, v8f64],
+           CCAssignToStack<64, 4>>
+]>;
+
 // CC_X86_32_Vector_Standard - The first 3 vector arguments are passed in
 // vector registers
 def CC_X86_32_Vector_Standard : CallingConv<[
   // SSE vector arguments are passed in XMM registers.
   CCIfNotVarArg<CCIfType<[v16i8, v8i16, v4i32, v2i64, v8f16, v4f32, v2f64],
                 CCAssignToReg<[XMM0, XMM1, XMM2]>>>,
 
   // AVX 256-bit vector arguments are passed in YMM registers.
   CCIfNotVarArg<CCIfType<[v32i8, v16i16, v8i32, v4i64, v16f16, v8f32, v4f64],
                 CCIfSubtarget<"hasAVX()",
                 CCAssignToReg<[YMM0, YMM1, YMM2]>>>>,
 
   // AVX 512-bit vector arguments are passed in ZMM registers.
   CCIfNotVarArg<CCIfType<[v64i8, v32i16, v16i32, v8i64, v32f16, v16f32, v8f64],
                 CCAssignToReg<[ZMM0, ZMM1, ZMM2]>>>,
 
+  CCIfIsVarArgOnWin<CCDelegateTo<CC_X86_Win32_Vector>>,
   CCDelegateTo<CC_X86_32_Vector_Common>
 ]>;
 
 // CC_X86_32_Vector_Darwin - The first 4 vector arguments are passed in
 // vector registers.
 def CC_X86_32_Vector_Darwin : CallingConv<[
   // SSE vector arguments are passed in XMM registers.
   CCIfNotVarArg<CCIfType<[v16i8, v8i16, v4i32, v2i64, v8f16, v4f32, v2f64],

llvm/test/CodeGen/X86/vaargs-win32.ll

+; RUN: llc -mcpu=generic -mtriple=i686-pc-windows-msvc -mattr=+sse < %s | FileCheck %s --check-prefix=MSVC
+; RUN: llc -mcpu=generic -mtriple=i686-pc-mingw32 -mattr=+sse < %s | FileCheck %s --check-prefix=MINGW
+
+@a = external dso_local global <4 x float>, align 16
+
+define dso_local void @testPastArguments() nounwind {
+; MSVC-LABEL: testPastArguments:
+; MSVC:       # %bb.0: # %entry
+; MSVC-NEXT:    subl $20, %esp
+; MSVC-NEXT:    movaps _a, %xmm0
+; MSVC-NEXT:    movups %xmm0, 4(%esp)
+; MSVC-NEXT:    movl $1, (%esp)
+; MSVC-NEXT:    calll _testm128
+; MSVC-NEXT:    addl $20, %esp
+; MSVC-NEXT:    retl
+;
+; MINGW-LABEL: testPastArguments:
+; MINGW:       # %bb.0: # %entry
+; MINGW-NEXT:    pushl %ebp
+; MINGW-NEXT:    movl %esp, %ebp
+; MINGW-NEXT:    andl $-16, %esp
+; MINGW-NEXT:    subl $48, %esp
+; MINGW-NEXT:    movaps _a, %xmm0
+; MINGW-NEXT:    movaps %xmm0, 16(%esp)
+; MINGW-NEXT:    movl $1, (%esp)
+; MINGW-NEXT:    calll _testm128
+; MINGW-NEXT:    movl %ebp, %esp
+; MINGW-NEXT:    popl %ebp
+; MINGW-NEXT:    retl
+entry:
+  %0 = load <4 x float>, <4 x float>* @a, align 16
+  %call = tail call i32 (i32, ...) @testm128(i32 1, <4 x float> inreg %0)
+  ret void
+}
+
+declare i32 @testm128(i32, ...) nounwind

llvm/test/CodeGen/X86/win32-spill-xmm.ll

entry:
   ret void
 }
 
 declare void @bar(<16 x float> %a, i32 %b)
 
 ; Check that proper alignment of spilled vector does not affect vargs
 
 ; CHECK-LABEL: vargs_not_affected
-; CHECK: movl 28(%ebp), %eax
+; CHECK: movl 28(%esp), %eax
 define i32 @vargs_not_affected(<4 x float> %v, i8* %f, ...) {
 entry:
   %ap = alloca i8*, align 4
   %0 = bitcast i8** %ap to i8*
   call void @llvm.va_start(i8* %0)
   %argp.cur = load i8*, i8** %ap, align 4
   %argp.next = getelementptr inbounds i8, i8* %argp.cur, i32 4
   store i8* %argp.next, i8** %ap, align 4