[PATCH] D134688: MSVC AArch64 ABI: Homogeneous aggregates

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Nice! Mostly comment copy edits.



================
Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:4478-4480
   // MSVC Windows on Arm64 considers a type not HFA if it is not an
   // aggregate according to the C++14 spec. This is not consistent with the
   // AAPCS64, but is defacto spec on that platform.
----------------
These comments seem stale. Are the checks below almost equivalent to C++14 isAggregate? In any case, they associate with the logic below, which is arm-specific.

The arm check deserves some kind of comment to say that all aggregates are permitted to be HFAs for non-ARM platforms, which mostly affects vectorcall on x64/x86.


================
Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:4498
+  // anything containing/derived from one is non-homogeneous.
+  // Instnead we could add another CXXABI entry point to query this property and
+  // have ABIInfo::isHomogeneousAggregate use that property.
----------------
typo Instnead


================
Comment at: clang/lib/CodeGen/MicrosoftCXXABI.cpp:4501-4503
+  // base/field while not true of the outer struct (eg: if you have a base/field
+  // that has an non-trivial copy assignment/dtor/default ctor, then the outer
+  // struct's corresponding operation must be non-trivial.
----------------
There's an open parenthetical here, maybe just make it `. For example, if you have...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134688