llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Oliver Stannard (ostannard) <details> <summary>Changes</summary> We had a test claiming that this empty struct type consumes a register slot when passing it to a function with GCC, but that does not appear to be the case, at least with GCC versions going back to 4.8. This also caused a miscompilation when passing one of these structs to a variadic function, but it turned out that our implementation of `va_arg` matches GCC's ABI, so the one change fixes both bugs. --- Full diff: https://github.com/llvm/llvm-project/pull/124760.diff 2 Files Affected: - (modified) clang/lib/CodeGen/Targets/AArch64.cpp (+8-5) - (modified) clang/test/CodeGen/AArch64/args.cpp (+65-20) ``````````diff diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp index 7db67ecba07c8f..b9261dee54db99 100644 --- a/clang/lib/CodeGen/Targets/AArch64.cpp +++ b/clang/lib/CodeGen/Targets/AArch64.cpp @@ -418,18 +418,21 @@ ABIArgInfo AArch64ABIInfo::classifyArgumentType(QualType Ty, bool IsVariadicFn, CGCXXABI::RAA_DirectInMemory); } - // Empty records are always ignored on Darwin, but actually passed in C++ mode - // elsewhere for GNU compatibility. + // Empty records: uint64_t Size = getContext().getTypeSize(Ty); bool IsEmpty = isEmptyRecord(getContext(), Ty, true); if (!Ty->isSVESizelessBuiltinType() && (IsEmpty || Size == 0)) { + // Empty records are ignored in C mode, and in C++ on Darwin. if (!getContext().getLangOpts().CPlusPlus || isDarwinPCS()) return ABIArgInfo::getIgnore(); - // GNU C mode. The only argument that gets ignored is an empty one with size - // 0. - if (IsEmpty && Size == 0) + // In C++ mode, arguments which have sizeof() == 0 (which are non-standard + // C++) are ignored. This isn't defined by any standard, so we copy GCC's + // behaviour here. + if (Size == 0) return ABIArgInfo::getIgnore(); + + // Otherwise, they are passed as if they have a size of 1 byte. return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext())); } diff --git a/clang/test/CodeGen/AArch64/args.cpp b/clang/test/CodeGen/AArch64/args.cpp index fe1298cc683a40..6f1f3d5e2a062b 100644 --- a/clang/test/CodeGen/AArch64/args.cpp +++ b/clang/test/CodeGen/AArch64/args.cpp @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -triple arm64-apple-ios7.0 -target-abi darwinpcs -emit-llvm -o - %s | FileCheck %s -// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - -x c %s | FileCheck %s --check-prefix=CHECK-GNU-C -// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-GNU-CXX +// RUN: %clang_cc1 -triple arm64-apple-ios7.0 -target-abi darwinpcs -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN +// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - -x c %s | FileCheck %s --check-prefixes=CHECK,C +// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,CXX // Empty structs are ignored for PCS purposes on Darwin and in C mode elsewhere. // In C++ mode on ELF they consume a register slot though. Functions are @@ -15,16 +15,16 @@ struct Empty {}; -// CHECK: define{{.*}} i32 @empty_arg(i32 noundef %a) -// CHECK-GNU-C: define{{.*}} i32 @empty_arg(i32 noundef %a) -// CHECK-GNU-CXX: define{{.*}} i32 @empty_arg(i8 %e.coerce, i32 noundef %a) +// DARWIN: define{{.*}} i32 @empty_arg(i32 noundef %a) +// C: define{{.*}} i32 @empty_arg(i32 noundef %a) +// CXX: define{{.*}} i32 @empty_arg(i8 %e.coerce, i32 noundef %a) EXTERNC int empty_arg(struct Empty e, int a) { return a; } -// CHECK: define{{.*}} void @empty_ret() -// CHECK-GNU-C: define{{.*}} void @empty_ret() -// CHECK-GNU-CXX: define{{.*}} void @empty_ret() +// DARWIN: define{{.*}} void @empty_ret() +// C: define{{.*}} void @empty_ret() +// CXX: define{{.*}} void @empty_ret() EXTERNC struct Empty empty_ret(void) { struct Empty e; return e; @@ -38,30 +38,75 @@ struct SuperEmpty { int arr[0]; }; -// CHECK: define{{.*}} i32 @super_empty_arg(i32 noundef %a) -// CHECK-GNU-C: define{{.*}} i32 @super_empty_arg(i32 noundef %a) -// CHECK-GNU-CXX: define{{.*}} i32 @super_empty_arg(i32 noundef %a) +// DARWIN: define{{.*}} i32 @super_empty_arg(i32 noundef %a) +// C: define{{.*}} i32 @super_empty_arg(i32 noundef %a) +// CXX: define{{.*}} i32 @super_empty_arg(i32 noundef %a) EXTERNC int super_empty_arg(struct SuperEmpty e, int a) { return a; } -// This is not empty. It has 0 size but consumes a register slot for GCC. +// This is also not empty, and non-standard. We previously considered it to +// consume a register slot, but GCC does not, so we match that. struct SortOfEmpty { struct SuperEmpty e; }; -// CHECK: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a) -// CHECK-GNU-C: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a) -// CHECK-GNU-CXX: define{{.*}} i32 @sort_of_empty_arg(i8 %e.coerce, i32 noundef %a) -EXTERNC int sort_of_empty_arg(struct Empty e, int a) { +// DARWIN: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a) +// C: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a) +// CXX: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a) +EXTERNC int sort_of_empty_arg(struct SortOfEmpty e, int a) { return a; } -// CHECK: define{{.*}} void @sort_of_empty_ret() -// CHECK-GNU-C: define{{.*}} void @sort_of_empty_ret() -// CHECK-GNU-CXX: define{{.*}} void @sort_of_empty_ret() +// DARWIN: define{{.*}} void @sort_of_empty_ret() +// C: define{{.*}} void @sort_of_empty_ret() +// CXX: 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 struct Empty empty_arg_variadic(int a, ...) { +// CHECK-LABEL: @empty_arg_variadic( +// DARWIN-NOT: {{ getelementptr }} +// C-NOT: {{ getelementptr }} +// CXX: %new_reg_offs = add i32 %gr_offs, 8 +// CXX: %new_stack = getelementptr inbounds i8, ptr %stack, i64 8 + va_list vl; + va_start(vl, a); + struct Empty b = va_arg(vl, struct Empty); + va_end(vl); + return b; +} + +EXTERNC struct SuperEmpty super_empty_arg_variadic(int a, ...) { +// CHECK-LABEL: @super_empty_arg_variadic( +// DARWIN-NOT: {{ getelementptr }} +// C-NOT: {{ getelementptr }} +// CXX-NOT: {{ getelementptr }} + va_list vl; + va_start(vl, a); + struct SuperEmpty b = va_arg(vl, struct SuperEmpty); + va_end(vl); + return b; +} + +EXTERNC struct SortOfEmpty sort_of_empty_arg_variadic(int a, ...) { +// CHECK-LABEL: @sort_of_empty_arg_variadic( +// DARWIN: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i64 0 +// C-NOT: {{ getelementptr }} +// CXX-NOT: {{ getelementptr }} + va_list vl; + va_start(vl, a); + struct SortOfEmpty b = va_arg(vl, struct SortOfEmpty); + va_end(vl); + return b; +} + `````````` </details> https://github.com/llvm/llvm-project/pull/124760 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
