[llvm-branch-commits] [clang] release/20.x: [ARM] Empty structs are 1-byte for C++ ABI (#124762) (PR #125194)

Oliver Stannard via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Fri Jan 31 02:08:07 PST 2025


https://github.com/ostannard created https://github.com/llvm/llvm-project/pull/125194

Backport 97b066f4e92a0df46b9d10721e988210f0d1afb6

Add release note.

>From f486aca315b8af06e050fab2f7d4f31675607b07 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Fri, 31 Jan 2025 09:03:01 +0000
Subject: [PATCH 1/2] [ARM] Empty structs are 1-byte for C++ ABI (#124762)

For C++ (but not C), empty structs should be passed to functions as if
they are a 1 byte object with 1 byte alignment.

This is defined in Arm's CPPABI32:
  https://github.com/ARM-software/abi-aa/blob/main/cppabi32/cppabi32.rst
  For the purposes of parameter passing in AAPCS32, a parameter whose
  type is an empty class shall be treated as if its type were an
  aggregate with a single member of type unsigned byte.

The AArch64 equivalent of this has an exception for structs containing
an array of size zero, I've kept that logic for ARM. I've not found a
reason for this exception, but I've checked that GCC does have the same
behaviour for ARM as it does for AArch64.

The AArch64 version has an Apple ABI with different rules, which ignores
empty structs in both C and C++. This is documented at
https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms.
The ARM equivalent of that appears to be AAPCS16_VFP, used for WatchOS,
but I can't find any documentation for that ABI, so I'm not sure what
rules it should follow. For now I've left it following the AArch64 Apple
rules.
---
 clang/include/clang/Basic/LangOptions.h |   2 +
 clang/lib/CodeGen/Targets/ARM.cpp       |  45 +++++++-
 clang/test/CodeGen/arm-empty-args.cpp   | 131 ++++++++++++++++++++++++
 3 files changed, 173 insertions(+), 5 deletions(-)
 create mode 100644 clang/test/CodeGen/arm-empty-args.cpp

diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 114a5d34a008bd..16c35bcf49339c 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -246,6 +246,8 @@ class LangOptionsBase {
     ///   construction vtable because it hasn't added 'type' as a substitution.
     ///   - Skip mangling enclosing class templates of member-like friend
     ///   function templates.
+    ///   - Ignore empty struct arguments in C++ mode for ARM, instead of
+    ///   passing them as if they had a size of 1 byte.
     Ver19,
 
     /// Conform to the underlying platform's C and C++ ABIs as closely
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
index 2d858fa2f3c3a3..47e31ceeaf2943 100644
--- a/clang/lib/CodeGen/Targets/ARM.cpp
+++ b/clang/lib/CodeGen/Targets/ARM.cpp
@@ -71,6 +71,7 @@ class ARMABIInfo : public ABIInfo {
                                   unsigned functionCallConv) const;
   ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base,
                                           uint64_t Members) const;
+  bool shouldIgnoreEmptyArg(QualType Ty) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
   bool isIllegalVectorType(QualType Ty) const;
   bool containsAnyFP16Vectors(QualType Ty) const;
@@ -328,6 +329,31 @@ ABIArgInfo ARMABIInfo::classifyHomogeneousAggregate(QualType Ty,
   return ABIArgInfo::getDirect(nullptr, 0, nullptr, false, Align);
 }
 
+bool ARMABIInfo::shouldIgnoreEmptyArg(QualType Ty) const {
+  uint64_t Size = getContext().getTypeSize(Ty);
+  assert((isEmptyRecord(getContext(), Ty, true) || Size == 0) &&
+         "Arg is not empty");
+
+  // Empty records are ignored in C mode, and in C++ on WatchOS.
+  if (!getContext().getLangOpts().CPlusPlus ||
+      getABIKind() == ARMABIKind::AAPCS16_VFP)
+    return true;
+
+  // In C++ mode, arguments which have sizeof() == 0 are ignored. This is not a
+  // situation which is defined by any C++ standard or ABI, but this matches
+  // GCC's de facto ABI.
+  if (Size == 0)
+    return true;
+
+  // Clang 19.0 and earlier always ignored empty struct arguments in C++ mode.
+  if (getContext().getLangOpts().getClangABICompat() <=
+      LangOptions::ClangABI::Ver19)
+    return true;
+
+  // Otherwise, they are passed as if they have a size of 1 byte.
+  return false;
+}
+
 ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic,
                                             unsigned functionCallConv) const {
   // 6.1.2.1 The following argument types are VFP CPRCs:
@@ -366,9 +392,15 @@ ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic,
     return getNaturalAlignIndirect(Ty, RAA == CGCXXABI::RAA_DirectInMemory);
   }
 
-  // Ignore empty records.
-  if (isEmptyRecord(getContext(), Ty, true))
-    return ABIArgInfo::getIgnore();
+  // Empty records are either ignored completely or passed as if they were a
+  // 1-byte object, depending on the ABI and language standard.
+  if (isEmptyRecord(getContext(), Ty, true) ||
+      getContext().getTypeSize(Ty) == 0) {
+    if (shouldIgnoreEmptyArg(Ty))
+      return ABIArgInfo::getIgnore();
+    else
+      return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
+  }
 
   if (IsAAPCS_VFP) {
     // Homogeneous Aggregates need to be expanded when we can fit the aggregate
@@ -588,7 +620,8 @@ ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy, bool isVariadic,
 
   // Otherwise this is an AAPCS variant.
 
-  if (isEmptyRecord(getContext(), RetTy, true))
+  if (isEmptyRecord(getContext(), RetTy, true) ||
+      getContext().getTypeSize(RetTy) == 0)
     return ABIArgInfo::getIgnore();
 
   // Check for homogeneous aggregates with AAPCS-VFP.
@@ -752,7 +785,9 @@ RValue ARMABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
   CharUnits SlotSize = CharUnits::fromQuantity(4);
 
   // Empty records are ignored for parameter passing purposes.
-  if (isEmptyRecord(getContext(), Ty, true))
+  uint64_t Size = getContext().getTypeSize(Ty);
+  bool IsEmpty = isEmptyRecord(getContext(), Ty, true);
+  if ((IsEmpty || Size == 0) && shouldIgnoreEmptyArg(Ty))
     return Slot.asRValue();
 
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);
diff --git a/clang/test/CodeGen/arm-empty-args.cpp b/clang/test/CodeGen/arm-empty-args.cpp
new file mode 100644
index 00000000000000..4e61c78b73ab92
--- /dev/null
+++ b/clang/test/CodeGen/arm-empty-args.cpp
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 -triple armv7a-linux-gnueabi -emit-llvm -o - -x c %s | FileCheck %s --check-prefixes=CHECK,C
+// RUN: %clang_cc1 -triple armv7a-linux-gnueabi -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,CXX
+// RUN: %clang_cc1 -triple armv7a-linux-gnueabi -emit-llvm -o - %s -fclang-abi-compat=19 | FileCheck %s --check-prefixes=CHECK,CXXCLANG19
+// RUN: %clang_cc1 -triple thumbv7k-apple-watchos2.0 -target-abi aapcs16 -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,WATCHOS
+
+// Empty structs are ignored for PCS purposes on WatchOS and in C mode
+// elsewhere.  In C++ mode they consume a register slot though. Functions are
+// slightly bigger than minimal to make confirmation against actual GCC
+// behaviour easier.
+
+#if __cplusplus
+#define EXTERNC extern "C"
+#else
+#define EXTERNC
+#endif
+
+struct Empty {};
+
+// C: define{{.*}} i32 @empty_arg(i32 noundef %a)
+// CXX: define{{.*}} i32 @empty_arg(i8 %e.coerce, i32 noundef %a)
+// CXXCLANG19: define{{.*}} i32 @empty_arg(i32 noundef %a)
+// WATCHOS: define{{.*}} i32 @empty_arg(i32 noundef %a)
+EXTERNC int empty_arg(struct Empty e, int a) {
+  return a;
+}
+
+// C: define{{.*}} void @empty_ret()
+// CXX: define{{.*}} void @empty_ret()
+// CXXCLANG19: define{{.*}} void @empty_ret()
+// WATCHOS: define{{.*}} void @empty_ret()
+EXTERNC struct Empty empty_ret(void) {
+  struct Empty e;
+  return e;
+}
+
+// However, what counts as "empty" is a baroque mess. This is super-empty, it's
+// ignored even in C++ mode. It also has sizeof == 0, violating C++, but that's
+// legacy for you:
+
+struct SuperEmpty {
+  int arr[0];
+};
+
+// C: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+// CXX: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+// CXXCLANG19: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+// WATCHOS: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+EXTERNC int super_empty_arg(struct SuperEmpty e, int a) {
+  return a;
+}
+
+struct SortOfEmpty {
+  struct SuperEmpty e;
+};
+
+// C: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
+// CXX: define{{.*}} i32 @sort_of_empty_arg(i8 %e.coerce, i32 noundef %a)
+// CXXCLANG19: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
+// WATCHOS: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
+EXTERNC int sort_of_empty_arg(struct Empty e, int a) {
+  return a;
+}
+
+// C: define{{.*}} void @sort_of_empty_ret()
+// CXX: define{{.*}} void @sort_of_empty_ret()
+// CXXCLANG19: define{{.*}} void @sort_of_empty_ret()
+// WATCHOS: define{{.*}} void @sort_of_empty_ret()
+EXTERNC struct SortOfEmpty sort_of_empty_ret(void) {
+  struct SortOfEmpty e;
+  return e;
+}
+
+#include <stdarg.h>
+
+// va_arg matches the above rules, consuming an incoming argument in cases
+// where one would be passed, and not doing so when the argument should be
+// ignored.
+
+EXTERNC int empty_arg_variadic(int a, ...) {
+// CHECK-LABEL: @empty_arg_variadic(
+// C: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// C-NOT: {{ getelementptr }}
+// CXX: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// CXX: %argp.next2 = getelementptr inbounds i8, ptr %argp.cur1, i32 4
+// CXXCLANG19: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// CXXCLANG19-NOT: {{ getelementptr }}
+// WATCHOS: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// WATCHOS-NOT: {{ getelementptr }}
+  va_list vl;
+  va_start(vl, a);
+  struct Empty b = va_arg(vl, struct Empty);
+  int c = va_arg(vl, int);
+  va_end(vl);
+  return c;
+}
+
+EXTERNC int super_empty_arg_variadic(int a, ...) {
+// CHECK-LABEL: @super_empty_arg_variadic(
+// C: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// C-NOT: {{ getelementptr }}
+// CXX: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// CXX-NOT: {{ getelementptr }}
+// CXXCLANG19: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// CXXCLANG19-NOT: {{ getelementptr }}
+// WATCHOS: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// WATCHOS-NOT: {{ getelementptr }}
+  va_list vl;
+  va_start(vl, a);
+  struct SuperEmpty b = va_arg(vl, struct SuperEmpty);
+  int c = va_arg(vl, int);
+  va_end(vl);
+  return c;
+}
+
+EXTERNC int sort_of_empty_arg_variadic(int a, ...) {
+// CHECK-LABEL: @sort_of_empty_arg_variadic(
+// C: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// C-NOT: {{ getelementptr }}
+// CXX: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// CXX-NOT: {{ getelementptr }}
+// CXXCLANG19: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// CXXCLANG19-NOT: {{ getelementptr }}
+// WATCHOS: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i32 4
+// WATCHOS-NOT: {{ getelementptr }}
+  va_list vl;
+  va_start(vl, a);
+  struct SortOfEmpty b = va_arg(vl, struct SortOfEmpty);
+  int c = va_arg(vl, int);
+  va_end(vl);
+  return c;
+}

>From ed55b4dfb3625e3c6176f5e8aa312911dee5c04a Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Fri, 31 Jan 2025 09:52:29 +0000
Subject: [PATCH 2/2] Add release note

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

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d8a94703bd9c57..6df7a178387601 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1186,6 +1186,11 @@ Arm and AArch64 Support
 
   * FUJITSU-MONAKA (fujitsu-monaka)
 
+- The ARM calling convention for empty structs in C++ mode was changed to pass
+  them as if they have a size of 1 byte, matching the AAPCS32 specification and
+  GCC's implementation. The previous behaviour of ignoring the argument can be
+  restored using the -fclang-abi-compat=19 (or earlier) option.
+
 Android Support
 ^^^^^^^^^^^^^^^
 



More information about the llvm-branch-commits mailing list