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()) { ---------------- 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? 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