[PATCH] D133817: MSVC ABI: Looks like even non-aarch64 uses the MSVC/14 definition for pod/aggregate passing

Wed Sep 14 10:43:52 PDT 2022

rnk added a comment.

Thanks!

> But I had
> trouble figuring out how to exercise this functionality properly to add
> test coverage and then compare that to MSVC itself... - I got very
> confused/turned around trying to test this, so I've given up enough to
> send what I have out for review, but happy to look further into this
> with help.

I think I would take the three cases you found (protected, nsdmi, defaulted copy ctor), add each to this file, and check for the IR prototypes here in this test file. Either sret is used, or it is not.

Regarding HFAs, I believe that logic is only used on x86/x64 for the __vectorcall calling convention. I believe it is well-tested with respect to C-like structs, but the C++ aspects that you are changing are not well tested. I think I managed to construct a case using NSDMIs (Richard used to prefer the terminology "default member initializers" which is simpler): https://godbolt.org/z/daPzKxj3h It looks like we don't match arm64 MSVC's behavior exactly here, but your change to remove the "!isAArch64" test would probably cause us to change behavior in this case, right?

================
Comment at: clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp:77

+struct SmallWithSmallWithPrivate {
+  SmallWithPrivate p;
----------------
I'm amused that this class is passed directly, but if you pass the field by itself, it is passed indirectly. :shrug:

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133817