[clang codegen] Fix ABI for HVA returns on AArch64 MSVC.

[efriedma]

Summary

MSVC normally has a bunch of restrictions on returning values directly, which don't apply to passing values directly. (This roughly corresponds to the definition of a C++14 aggregate.) However, these restrictions don't apply to HVAs; make sure we check for that.

Fixes https://github.com/llvm/llvm-project/issues/62223

[efriedma]

Restrict to AArch64.

Actually, it seems like something sort of similar happens with x86 vectorcall. But I haven't tried to test all the permutations of that, so don't modify the behavior for now.

[rnk]

Interesting. In Clang, we basically layer the C++ rules over the C rules: if C++ aspects of a class allow it to be passed directly transparently, then we defer down to the C rules, which deal with HVAs, structs, and things like that.

In D153179#4429813, @efriedma wrote:

Restrict to AArch64.

Actually, it seems like something sort of similar happens with x86 vectorcall. But I haven't tried to test all the permutations of that, so don't modify the behavior for now.

Can you file an issue for that and cc me?

clang/test/CodeGenCXX/homogeneous-aggregates.cpp

294

Can you do the same test again with a non-vector field (double) for test coverage?

295

Did you mean to inherit from base here to make a two-element HVA?

[efriedma]

Filed https://github.com/llvm/llvm-project/issues/63417 . (On a related note, I also filed https://github.com/llvm/llvm-project/issues/63360 for an issue I stumbled over involving deleted copy constructors.)

Interesting. In Clang, we basically layer the C++ rules over the C rules: if C++ aspects of a class allow it to be passed directly transparently, then we defer down to the C rules, which deal with HVAs, structs, and things like that.

Yes... clearly MSVC is doing something very different if they're getting results like this by accident.

[efriedma]

Updated testcase.

[efriedma]

Fix typo

[efriedma]

Upload correct diff.

[rnk]

lgtm

[rnk]

The other thing I'm getting at here is that our TargetInfo.cpp abstraction is pretty well built out, even if it's a huge mess. The C++ rules are generally platform neutral, and we don't usually have to resort to these kinds of triple checks, but sometimes it's the most straightforward approach.

clang/lib/CodeGen/MicrosoftCXXABI.cpp

 //===--- MicrosoftCXXABI.cpp - Emit LLVM Code from ASTs for a Module ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This provides C++ code generation targeting the Microsoft Visual C++ ABI.
 // The class in this file generates structures that follow the Microsoft
 // Visual C++ ABI, which is actually not very well documented at all outside
 // of Microsoft.
 //
 //===----------------------------------------------------------------------===//
 
+#include "ABIInfo.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGVTables.h"
 #include "CodeGenModule.h"
 #include "CodeGenTypes.h"
 #include "TargetInfo.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/CXXInheritance.h"
static bool isDeletingDtor(GlobalDecl GD) {
   return isa<CXXDestructorDecl>(GD.getDecl()) &&
          GD.getDtorType() == Dtor_Deleting;
 }
 
 bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl GD) const {
   return isDeletingDtor(GD);
 }
 
-static bool isTrivialForMSVC(const CXXRecordDecl *RD) {
+static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
+                             CodeGenModule &CGM) {
+  // On AArch64, HVAs that can be passed in registers can also be returned
+  // in registers. (Note this is using the MSVC definition of an HVA; see
+  // isPermittedToBeHomogeneousAggregate().)
+  const Type *Base = nullptr;
+  uint64_t NumElts = 0;
+  if (CGM.getTarget().getTriple().isAArch64() &&
+      CGM.getTypes().getABIInfo().isHomogeneousAggregate(Ty, Base, NumElts) &&
+      isa<VectorType>(Base)) {
+    return true;
+  }
+
   // We use the C++14 definition of an aggregate, so we also
   // check for:
   //   No private or protected non static data members.
   //   No base classes
   //   No virtual functions
   // Additionally, we need to ensure that there is a trivial copy assignment
   // operator, a trivial destructor and no user-provided constructors.
   if (RD->hasProtectedFields() || RD->hasPrivateFields())
static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
   return true;
 }
 
 bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   const CXXRecordDecl *RD = FI.getReturnType()->getAsCXXRecordDecl();
   if (!RD)
     return false;
 
-  bool isTrivialForABI = RD->canPassInRegisters() && isTrivialForMSVC(RD);
+  bool isTrivialForABI = RD->canPassInRegisters() &&
+                         isTrivialForMSVC(RD, FI.getReturnType(), CGM);
 
   // MSVC always returns structs indirectly from C++ instance methods.
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
 
   if (isIndirectReturn) {
     CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
     FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
 

clang/test/CodeGenCXX/homogeneous-aggregates.cpp

struct HFA {
   double x;
   double y;
   double z;
   non_empty e;
 };
 double foo(HFA v) { return v.x + v.y; }
 // WOA64: define dso_local noundef double @"?foo@non_empty_field@@YANUHFA@1@@Z"([4 x double] %{{.*}})
 }
+
+namespace pr62223 {
+// HVAs don't follow the normal rules for return values. That means they can
+// have base classes, user-defined ctors, and protected/private members.
+// (The same restrictions that apply to HVA arguments still apply.)
+typedef double V __attribute((ext_vector_type(2)));
+struct base { V v; };
+struct test : base { test(double); protected: V v2;};
+test f(test *x) { return *x; }
+// WOA64: define dso_local %"struct.pr62223::test" @"?f@pr62223@@YA?AUtest@1@PEAU21@@Z"(ptr noundef %{{.*}})
+
+// The above rule only apples to HVAs, not HFAs.
+struct base2 { double v; };
+struct test2 : base2 { test2(double); protected: double v2;};
+test2 f(test2 *x) { return *x; }
+// WOA64: define dso_local void @"?f@pr62223@@YA?AUtest2@1@PEAU21@@Z"(ptr inreg noalias sret(%"struct.pr62223::test2") align 8 %{{.*}}, ptr noundef %{{.*}})
+}