[clang] [clang][CodeGen] Fix MSVC ABI for classes with non static data members of reference type (PR #90547)
Max Winkler via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 29 19:39:00 PDT 2024
https://github.com/MaxEW707 created https://github.com/llvm/llvm-project/pull/90547
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.
>From cafb799de05af9197891de4dbf451b10cc26df65 Mon Sep 17 00:00:00 2001
From: MaxEW707 <max.enrico.winkler at gmail.com>
Date: Mon, 29 Apr 2024 22:09:52 -0400
Subject: [PATCH] [clang][CodeGen] Fix MS ABI for classes with non static data
members of reference type
---
clang/docs/ReleaseNotes.rst | 3 +
clang/lib/CodeGen/MicrosoftCXXABI.cpp | 37 +++++++++--
.../test/CodeGen/x64-microsoft-arguments.cpp | 64 +++++++++++++++++++
3 files changed, 98 insertions(+), 6 deletions(-)
create mode 100644 clang/test/CodeGen/x64-microsoft-arguments.cpp
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);
+}
More information about the cfe-commits
mailing list