[clang] [clang][CodeGen] Fix MSVC ABI for classes with non static data members of reference type (PR #90547)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: Max Winkler (MaxEW707)

<details>
<summary>Changes</summary>

https://godbolt.org/z/verKj4cnj for reference.
https://godbolt.org/z/z3W9v7o4n for reference.

For global functions and static methods the MSVC ABI returns structs/classes with a reference type non static data member indirectly.
>From local testing this is recursively applied to any inner structs/classes.
>From local testing this ABI holds true for all currently support architectures including ARM64EC.

I tested locally against MSVC 1939 and MSVC 1929.

The only reference I could find on MSDN to this ABI constraint is this quote, "and no non-static data members of reference type", [here](https://learn.microsoft.com/en-us/cpp/build/x64-calling-convention?view=msvc-170#return-values).

Pointer fields do not exhibit this behaviour.

---
Full diff: https://github.com/llvm/llvm-project/pull/90547.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/lib/CodeGen/MicrosoftCXXABI.cpp (+31-6) 
- (added) clang/test/CodeGen/x64-microsoft-arguments.cpp (+64) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4aedfafcb26aea..8f228bb4d296e0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -63,6 +63,9 @@ ABI Changes in This Version
   MSVC uses a different mangling for these objects, compatibility is not affected.
   (#GH85423).
 
+- Fixed Microsoft calling convention when returning classes that have a reference type
+  as a field. Such a class should be returned indirectly.
+
 AST Dumping Potentially Breaking Changes
 ----------------------------------------
 
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index d38a26940a3cb6..b0798550ba9775 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1103,8 +1103,27 @@ bool MicrosoftCXXABI::hasMostDerivedReturn(GlobalDecl GD) const {
   return isDeletingDtor(GD);
 }
 
+static bool fieldIsTrivialForMSVC(const FieldDecl *Field,
+                                  const ASTContext &Context) {
+  if (Field->getType()->isReferenceType())
+    return false;
+
+  const RecordType *RT =
+      Context.getBaseElementType(Field->getType())->getAs<RecordType>();
+  if (!RT)
+    return true;
+
+  CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
+
+  for (const FieldDecl *RDField : RD->fields())
+    if (!fieldIsTrivialForMSVC(RDField, Context))
+      return false;
+
+  return true;
+}
+
 static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
-                             CodeGenModule &CGM) {
+                             CodeGenModule &CGM, const ASTContext &Context) {
   // 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().)
@@ -1122,7 +1141,8 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
   //   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.
+  // operator, a trivial destructor, no user-provided constructors and no
+  // non static data members of reference type.
   if (RD->hasProtectedFields() || RD->hasPrivateFields())
     return false;
   if (RD->getNumBases() > 0)
@@ -1136,6 +1156,9 @@ static bool isTrivialForMSVC(const CXXRecordDecl *RD, QualType Ty,
       return false;
   if (RD->hasNonTrivialDestructor())
     return false;
+  for (const FieldDecl *Field : RD->fields())
+    if (!fieldIsTrivialForMSVC(Field, Context))
+      return false;
   return true;
 }
 
@@ -1144,11 +1167,13 @@ bool MicrosoftCXXABI::classifyReturnType(CGFunctionInfo &FI) const {
   if (!RD)
     return false;
 
-  bool isTrivialForABI = RD->canPassInRegisters() &&
-                         isTrivialForMSVC(RD, FI.getReturnType(), CGM);
-
   // MSVC always returns structs indirectly from C++ instance methods.
-  bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
+  bool isIndirectReturn = FI.isInstanceMethod();
+  if (!isIndirectReturn) {
+    isIndirectReturn =
+        !(RD->canPassInRegisters() &&
+          isTrivialForMSVC(RD, FI.getReturnType(), CGM, getContext()));
+  }
 
   if (isIndirectReturn) {
     CharUnits Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
diff --git a/clang/test/CodeGen/x64-microsoft-arguments.cpp b/clang/test/CodeGen/x64-microsoft-arguments.cpp
new file mode 100644
index 00000000000000..9e89b59924ae49
--- /dev/null
+++ b/clang/test/CodeGen/x64-microsoft-arguments.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -ffreestanding -emit-llvm -O0 \
+// RUN: -x c++ -o - %s | FileCheck %s
+
+int global_i = 0;
+
+// Pass and return object with a reference type (pass directly, return indirectly).
+// CHECK: define dso_local void @"?f1@@YA?AUS1@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S1) align 8 {{.*}})
+// CHECK: call void @"?func1@@YA?AUS1@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S1) align 8 {{.*}}, i64 {{.*}})
+struct S1 {
+  int& r;
+};
+
+S1 func1(S1 x);
+S1 f1() {
+  S1 x{ global_i };
+  return func1(x);
+}
+
+// Pass and return object with a reference type within an inner struct (pass directly, return indirectly).
+// CHECK: define dso_local void @"?f2@@YA?AUS2@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S2) align 8 {{.*}})
+// CHECK: call void @"?func2@@YA?AUS2@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S2) align 8 {{.*}}, i64 {{.*}})
+struct Inner {
+  int& r;
+};
+
+struct S2 {
+  Inner i;
+};
+
+S2 func2(S2 x);
+S2 f2() {
+  S2 x{ { global_i } };
+  return func2(x);
+}
+
+// Pass and return object with a reference type (pass directly, return indirectly).
+// CHECK: define dso_local void @"?f3@@YA?AUS3@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S3) align 8 {{.*}})
+// CHECK: call void @"?func3@@YA?AUS3@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S3) align 8 {{.*}}, i64 {{.*}})
+struct S3 {
+  const int& r;
+};
+
+S3 func3(S3 x);
+S3 f3() {
+  S3 x{ global_i };
+  return func3(x);
+}
+
+// Pass and return object with a reference type within an inner struct (pass directly, return indirectly).
+// CHECK: define dso_local void @"?f4@@YA?AUS4@@XZ"(ptr dead_on_unwind noalias writable sret(%struct.S4) align 8 {{.*}})
+// CHECK: call void @"?func4@@YA?AUS4@@U1@@Z"(ptr dead_on_unwind writable sret(%struct.S4) align 8 {{.*}}, i64 {{.*}})
+struct InnerConst {
+  const int& r;
+};
+
+struct S4 {
+  InnerConst i;
+};
+
+S4 func4(S4 x);
+S4 f4() {
+  S4 x{ { global_i } };
+  return func4(x);
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/90547