[clang] [clang] Return larger CXX records in memory (PR #120670)

[Pranav Kant via cfe-commits]

https://github.com/pranavk updated https://github.com/llvm/llvm-project/pull/120670

>From 4b6839317bcd2a014011cb91b5a3e58d4a47b0b1 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Fri, 20 Dec 2024 02:17:23 +0000
Subject: [PATCH 1/3] [clang] Return larger CXX records in memory

We incorrectly return CXX records in AVX registers when they should be
returned in memory. This is violation of x86-64 psABI.

Detailed discussion is here: https://groups.google.com/g/x86-64-abi/c/BjOOyihHuqg/m/KurXdUcWAgAJ
---
 clang/include/clang/Basic/LangOptions.h |  1 +
 clang/lib/CodeGen/Targets/X86.cpp       | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 949c8f5d448bcf..0dd644eba559b9 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -245,6 +245,7 @@ class LangOptionsBase {
     ///   construction vtable because it hasn't added 'type' as a substitution.
     ///   - Skip mangling enclosing class templates of member-like friend
     ///   function templates.
+    ///   - Incorrectly return C++ records in AVX registers.
     Ver19,
 
     /// Conform to the underlying platform's C and C++ ABIs as closely
diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 7f73bf2a65266e..70d812057d0b01 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -1334,6 +1334,20 @@ class X86_64ABIInfo : public ABIInfo {
     return T.isOSLinux() || T.isOSNetBSD();
   }
 
+  bool returnCXXRecordGreaterThan128InMem(unsigned Size, unsigned TypeSize,
+                                          unsigned NativeSize) const {
+    // Clang <= 19.0 did not do this.
+    if (getContext().getLangOpts().getClangABICompat() <=
+        LangOptions::ClangABI::Ver19)
+      return false;
+
+    // The only case a 256(or 512)-bit wide vector could be used to return
+    // is when CXX record contains a single 256(or 512)-bit element.
+    if (Size > 128 && (Size != TypeSize || Size > NativeSize))
+      return true;
+    return false;
+  }
+
   X86AVXABILevel AVXLevel;
   // Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers on
   // 64-bit hardware.
@@ -2067,6 +2081,10 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo,
         classify(I.getType(), Offset, FieldLo, FieldHi, isNamedArg);
         Lo = merge(Lo, FieldLo);
         Hi = merge(Hi, FieldHi);
+        if (returnCXXRecordGreaterThan128InMem(
+                Size, getContext().getTypeSize(I.getType()),
+                getNativeVectorSizeForAVXABI(AVXLevel)))
+          Lo = Memory;
         if (Lo == Memory || Hi == Memory) {
           postMerge(Size, Lo, Hi);
           return;

>From 6e62d019db05601bb6d4046785b9a219f92cebe8 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Mon, 30 Dec 2024 14:57:31 +0000
Subject: [PATCH 2/3] add a regression test

---
 clang/test/CodeGen/X86/avx-cxx-record.cpp | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 clang/test/CodeGen/X86/avx-cxx-record.cpp

diff --git a/clang/test/CodeGen/X86/avx-cxx-record.cpp b/clang/test/CodeGen/X86/avx-cxx-record.cpp
new file mode 100644
index 00000000000000..0d5f070b7754fb
--- /dev/null
+++ b/clang/test/CodeGen/X86/avx-cxx-record.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang %s -c -S -emit-llvm -O2 -march=x86-64-v3 -o - | FileCheck %s
+
+using UInt64x2 = unsigned long long __attribute__((__vector_size__(16), may_alias));
+
+template<int id>
+struct XMM1 {
+    UInt64x2 x;
+};
+
+struct XMM2 : XMM1<0>, XMM1<1> {
+};
+
+// CHECK: define{{.*}} @_Z3foov({{.*}} [[ARG:%.*]]){{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: store {{.*}}, ptr [[ARG]]{{.*}}
+// CHECK-NEXT: [[TMP1:%.*]] = getelementptr {{.*}}, ptr [[ARG]]{{.*}}
+// CHECK-NEXT: store {{.*}}, ptr [[TMP1]]{{.*}}
+XMM2 foo() {
+  XMM2 result;
+  ((XMM1<0>*)&result)->x = UInt64x2{1, 2};
+  ((XMM1<1>*)&result)->x = UInt64x2{3, 4};
+  return result;
+}

>From 1f01309c237c8daf15be4ac1708e22e841935662 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Mon, 30 Dec 2024 14:59:04 +0000
Subject: [PATCH 3/3] comment

---
 clang/include/clang/Basic/LangOptions.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 0dd644eba559b9..6df2fa414f4df8 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -245,7 +245,7 @@ class LangOptionsBase {
     ///   construction vtable because it hasn't added 'type' as a substitution.
     ///   - Skip mangling enclosing class templates of member-like friend
     ///   function templates.
-    ///   - Incorrectly return C++ records in AVX registers.
+    ///   - Incorrectly return C++ records in AVX registers on x86_64.
     Ver19,
 
     /// Conform to the underlying platform's C and C++ ABIs as closely