llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-arm Author: Oliver Stannard (ostannard) <details> <summary>Changes</summary> Backport 97b066f4e92a0df46b9d10721e988210f0d1afb6 Add release note. --- Full diff: https://github.com/llvm/llvm-project/pull/125194.diff 4 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+5) - (modified) clang/include/clang/Basic/LangOptions.h (+2) - (modified) clang/lib/CodeGen/Targets/ARM.cpp (+40-5) - (added) clang/test/CodeGen/arm-empty-args.cpp (+131) ``````````diff 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 ^^^^^^^^^^^^^^^ 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; +} `````````` </details> https://github.com/llvm/llvm-project/pull/125194 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits