[ARM] Empty structs are 1-byte for C++ ABI #124762
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-arm @llvm/pr-subscribers-clang Author: Oliver Stannard (ostannard) ChangesFor 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: 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. Full diff: https://github.com/llvm/llvm-project/pull/124762.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
index 2d858fa2f3c3a3..b243ccacc2155d 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,26 @@ 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;
+
+ // 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 +387,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 +615,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,10 +780,13 @@ 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);
+ CharUnits TySize =
+ std::max(getContext().getTypeSizeInChars(Ty), CharUnits::fromQuantity(1));
CharUnits TyAlignForABI = getContext().getTypeUnadjustedAlignInChars(Ty);
// Use indirect if size of the illegal vector is bigger than 16 bytes.
diff --git a/clang/test/CodeGen/arm-empty-args.cpp b/clang/test/CodeGen/arm-empty-args.cpp
new file mode 100644
index 00000000000000..5770ce160749f7
--- /dev/null
+++ b/clang/test/CodeGen/arm-empty-args.cpp
@@ -0,0 +1,119 @@
+// 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 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)
+// 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()
+// 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)
+// 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)
+// 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()
+// 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
+// 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 }}
+// 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 }}
+// 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;
+}
|
|
@llvm/pr-subscribers-clang-codegen Author: Oliver Stannard (ostannard) ChangesFor 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: 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. Full diff: https://github.com/llvm/llvm-project/pull/124762.diff 2 Files Affected:
diff --git a/clang/lib/CodeGen/Targets/ARM.cpp b/clang/lib/CodeGen/Targets/ARM.cpp
index 2d858fa2f3c3a3..b243ccacc2155d 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,26 @@ 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;
+
+ // 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 +387,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 +615,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,10 +780,13 @@ 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);
+ CharUnits TySize =
+ std::max(getContext().getTypeSizeInChars(Ty), CharUnits::fromQuantity(1));
CharUnits TyAlignForABI = getContext().getTypeUnadjustedAlignInChars(Ty);
// Use indirect if size of the illegal vector is bigger than 16 bytes.
diff --git a/clang/test/CodeGen/arm-empty-args.cpp b/clang/test/CodeGen/arm-empty-args.cpp
new file mode 100644
index 00000000000000..5770ce160749f7
--- /dev/null
+++ b/clang/test/CodeGen/arm-empty-args.cpp
@@ -0,0 +1,119 @@
+// 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 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)
+// 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()
+// 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)
+// 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)
+// 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()
+// 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
+// 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 }}
+// 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 }}
+// 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;
+}
|
Comment on lines
+618
to
+619
| if (isEmptyRecord(getContext(), RetTy, true) || | ||
| getContext().getTypeSize(RetTy) == 0) |
There was a problem hiding this comment.
Does this change actually have any effect?
There was a problem hiding this comment.
This is needed for the SortOfEmpty case, which isn't considered empty, but does have size 0.
clang/lib/CodeGen/Targets/ARM.cpp
Outdated
| return Slot.asRValue(); | ||
|
|
||
| CharUnits TySize = getContext().getTypeSizeInChars(Ty); | ||
| CharUnits TySize = | ||
| std::max(getContext().getTypeSizeInChars(Ty), CharUnits::fromQuantity(1)); |
There was a problem hiding this comment.
If we get here, we know the size isn't zero, I think?
There was a problem hiding this comment.
This was left over from a previous version, I'll remove it.
| // 1-byte object, depending on the ABI and language standard. | ||
| if (isEmptyRecord(getContext(), Ty, true) || | ||
| getContext().getTypeSize(Ty) == 0) { | ||
| if (shouldIgnoreEmptyArg(Ty)) |
There was a problem hiding this comment.
I think I'd prefer to write this something like:
if (size == zero)
return getIgnore();
if (isempty() && isWatchOS())
return getIgnore();
This should be equivalent because an empty struct in C is size zero anyway.
There was a problem hiding this comment.
Actually, hmm... do we want to add fclang-abi-compat support for this change, or do we think it's too niche to matter?
There was a problem hiding this comment.
I wasn't aware of the -fclang-abi-compat option, but I think that would be worth adding for this. I'll also add a release note as suggested in #124760.
llvmbot
added
the
clang:frontend
clang/docs/ReleaseNotes.rst
Outdated
| - The AArch64 calling convention for empty structs in C++ mode was changed to | ||
| pass them as if they have a size of 1 byte, matching the AAPCS64 | ||
| specification and GCC's implementation. The previous behaviour of ignoring | ||
| the argument can be restored using the -fclang-abi-compat=20 (or earlier) |
There was a problem hiding this comment.
20? You're not planning to cherry-pick this to the release branch?
|
That last commit message should have been "llvm-20", not 19. |
clang/docs/ReleaseNotes.rst
Outdated
| @@ -1144,6 +1144,12 @@ Arm and AArch64 Support | |||
|
|
|||
| * FUJITSU-MONAKA (fujitsu-monaka) | |||
|
|
|||
| - The AArch64 calling convention for empty structs in C++ mode was changed to | |||
There was a problem hiding this comment.
I assume this is supposed to mention AArch32/AAPCS32?
|
The release notes have now been cleared on main, so I'll re-add them in the llvm-20 cherry pick. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/14992 Here is the relevant piece of the build log for the reference |
|
/cherry-pick 97b066f |
Error: Command failed due to missing milestone. |
|
/cherry-pick 97b066f |
|
/pull-request #125191 |
ostannard
added a commit
to ostannard/llvm-project
that referenced
this pull request
Jan 31, 2025
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.
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 10, 2025
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.