[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 3f7f567dde22f75018225555b83c33eeff8dd596 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/2] [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         | 17 +++++++++++++++++
 clang/test/CodeGen/X86/avx-cxx-record.cpp | 23 +++++++++++++++++++++++
 3 files changed, 41 insertions(+)
 create mode 100644 clang/test/CodeGen/X86/avx-cxx-record.cpp

diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 114a5d34a008bd..31afe808fa1411 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -246,6 +246,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 on x86_64.
     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 5ee5179dd0f3e8..ebd4291cfb49e5 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -1334,6 +1334,16 @@ class X86_64ABIInfo : public ABIInfo {
     return T.isOSLinux() || T.isOSNetBSD();
   }
 
+  bool returnCXXRecordGreaterThan128InMem() const {
+    // Clang <= 19.0 did not do this.
+    if (getContext().getLangOpts().getClangABICompat() <=
+        LangOptions::ClangABI::Ver19)
+      return false;
+
+    const llvm::Triple &T = getTarget().getTriple();
+    return T.isOSLinux() || T.isOSNetBSD();
+  }
+
   X86AVXABILevel AVXLevel;
   // Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers on
   // 64-bit hardware.
@@ -2067,6 +2077,13 @@ 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 > 128 && (Size != getContext().getTypeSize(I.getType()) ||
+                            Size > getNativeVectorSizeForAVXABI(AVXLevel)))) {
+          // 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.
+          Lo = Memory;
+        }
         if (Lo == Memory || Hi == Memory) {
           postMerge(Size, Lo, Hi);
           return;
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..d8863ca4e45f9e
--- /dev/null
+++ b/clang/test/CodeGen/X86/avx-cxx-record.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang %s -S --target=x86_64-unknown-linux-gnu -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 403bcdd97211e200dba39d0ab51909d1e4b59d96 Mon Sep 17 00:00:00 2001
From: Pranav Kant <prka at google.com>
Date: Tue, 28 Jan 2025 22:48:01 +0000
Subject: [PATCH 2/2] Add entry to release notes

---
 clang/docs/ReleaseNotes.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d8a94703bd9c57..f44175f294204b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -253,6 +253,7 @@ ABI Changes in This Version
 - Fixed Microsoft name mangling of placeholder, auto and decltype(auto), return types for MSVC 1920+. This change resolves incompatibilities with code compiled by MSVC 1920+ but will introduce incompatibilities with code compiled by earlier versions of Clang unless such code is built with the compiler option -fms-compatibility-version=19.14 to imitate the MSVC 1914 mangling behavior.
 - Fixed the Itanium mangling of the construction vtable name. This change will introduce incompatibilities with code compiled by Clang 19 and earlier versions, unless the -fclang-abi-compat=19 option is used. (#GH108015)
 - Mangle member-like friend function templates as members of the enclosing class. (#GH110247, #GH110503)
+- Return larger CXX records in memory instead of using AVX registers. Code compiled with older clang will be incompatible with newer version of the clang unless -fclang-abi-compat=19 is provided. (#GH120670)
 
 AST Dumping Potentially Breaking Changes
 ----------------------------------------