[all-commits] [llvm/llvm-project] 476997: MSVC ABI: Looks like even non-aarch64 uses the MSV...

[David Blaikie via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 4769976c49be468d7629d513080e6959a25adcfe
      https://github.com/llvm/llvm-project/commit/4769976c49be468d7629d513080e6959a25adcfe
  Author: David Blaikie <dblaikie at gmail.com>
  Date:   2022-10-04 (Tue, 04 Oct 2022)

  Changed paths:
    M clang/lib/CodeGen/MicrosoftCXXABI.cpp
    M clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

  Log Message:
  -----------
  MSVC ABI: Looks like even non-aarch64 uses the MSVC/14 definition for pod/aggregate passing

Details posted here: https://reviews.llvm.org/D119051#3747201

3 cases that were inconsistent with the MSABI without this patch applied:
  https://godbolt.org/z/GY48qxh3G - field with protected member
  https://godbolt.org/z/Mb1PYhjrP - non-static data member initializer
  https://godbolt.org/z/sGvxcEPjo - defaulted copy constructor

I'm not sure what's suitable/sufficient testing for this - I did verify
the three cases above. Though if it helps to add them as explicit tests,
I can do that too.

Also, I was wondering if the other use of isTrivialForAArch64MSVC in
isPermittedToBeHomogenousAggregate could be another source of bugs - I
tried changing the function to unconditionally call
isTrivialFor(AArch64)MSVC without testing AArch64 first, but no tests
fail, so it looks like this is undertested in any case. 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.

Differential Revision: https://reviews.llvm.org/D133817