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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/Target/X86/X86CallingConv.td:29-30
+    : CCIf<"State.isVarArg() && "
+           "State.getMachineFunction().getSubtarget().getTargetTriple()."
+           "isOSWindows()",
+           A>;
----------------
rnk wrote:
> 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.
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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108887