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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Target/X86/X86CallingConv.td:29-30
+    : CCIf<"State.isVarArg() && "
+           "State.getMachineFunction().getSubtarget().getTargetTriple()."
+           "isOSWindows()",
+           A>;
----------------
mstorsjo wrote:
> rnk wrote:
> > pengfei wrote:
> > > rnk wrote:
> > > > Please check what mingw does in this case.
> > > 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.
> > @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.
> 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.
> 
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108887/new/

https://reviews.llvm.org/D108887