mstorsjo added inline comments.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:4364-4365
// MS x64 ABI requirement: "Any argument that doesn't fit in 8 bytes, or is
// not 1, 2, 4, or 8 bytes, must be passed by reference."
if (isAggregateTypeForABI(Ty) || Ty->isMemberPointerType()) {
----------------
mstorsjo wrote:
> rnk wrote:
> > I wonder if we should make the check more general. There are other cases to
> > consider, like `__int128`. How does GCC pass other large types through
> > varargs? It looks to me like clang passes those indirectly, so we could go
> > ahead and check the type size directly without these aggregate checks.
> >
> > Separately, I think `X86_64ABIInfo::EmitMSVAArg` has a bug, it always
> > passes false for indirect, but it should match this code here exactly.
> > Ideally, they would share an implementation.
> I'll have a look at what GCC does for `__int128` yeah.
>
> Yes, `X86_64ABIInfo::EmitMSVAArg` is lacking, I've got a separate patch
> fixing that one up to be posted after this one (but this patch fixes an
> actual issue I've run into, the other one is more of a correctness thing).
>
> The open question about that one, is what to do about `long double`. If using
> `__attribute__((ms_abi))` and `__builtin_ms_va_list` on a non-windows
> platform, we don't know if they're meaning to interact with the MS or GNU ABI
> regarding `long double`s.
>
> On one hand, one would mostly be using those constructs to reimplement
> Windows API (i.e. implementing wine or something similar), and in that case,
> only the MS behaviour is of interest. On the other hand, mapping `va_arg(ap,
> long double)` to the GNU case doesn't take anything away for the user either,
> because if you want a MSVC long double, you can just do `va_arg(ap, double)`.
> Then finally, I guess there's no guarantee that the platform where doing that
> even has an exactly matching long double?
GCC does the same for `__int128` in varargs as it does for `long double`, so
that does indeed simplify the code a fair bit.
And I also realized it's not a problem wrt `long double` and the
`__builtin_ms_va_list`; if the caller does `va_arg(ap, long double)`, that has
to be handled according to whatever the size of `long double` is on platform,
which hopefully would match what GCC uses on windows.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103452/new/
https://reviews.llvm.org/D103452
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
