https://github.com/ostannard created
https://github.com/llvm/llvm-project/pull/124760
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.
>From a162ceeec41abe61ba1f3e0cdd069556ea58829f Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Thu, 12 Dec 2024 15:28:56 +0000
Subject: [PATCH 1/4] [AArch64] Fix some misleading comments
---
clang/lib/CodeGen/Targets/AArch64.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 7db67ecba07c8f..43c99cfd270654 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -418,18 +418,20 @@ 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.
+ // In C++ mode, arguments which are empty and have sizeof() == 0 (which are
+ // non-standard C++) are ignored.
if (IsEmpty && 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()));
}
>From 45fa4406b2239722249d9e436167057ce6a15e4c Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Tue, 28 Jan 2025 13:58:58 +0000
Subject: [PATCH 2/4] [AArch64] Fix test which wasn't testing anything
---
clang/test/CodeGen/AArch64/args.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/test/CodeGen/AArch64/args.cpp
b/clang/test/CodeGen/AArch64/args.cpp
index fe1298cc683a40..7acf26b2389efb 100644
--- a/clang/test/CodeGen/AArch64/args.cpp
+++ b/clang/test/CodeGen/AArch64/args.cpp
@@ -54,7 +54,7 @@ struct SortOfEmpty {
// 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) {
+EXTERNC int sort_of_empty_arg(struct SortOfEmpty e, int a) {
return a;
}
>From f1be10473c9c889cb8b74a948fa3803c605b728a Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Thu, 23 Jan 2025 10:54:12 +0000
Subject: [PATCH 3/4] [AArch64] Match GCC behaviour for zero-size structs
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.
---
clang/lib/CodeGen/Targets/AArch64.cpp | 7 ++++---
clang/test/CodeGen/AArch64/args.cpp | 5 +++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 43c99cfd270654..b9261dee54db99 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -426,9 +426,10 @@ ABIArgInfo AArch64ABIInfo::classifyArgumentType(QualType
Ty, bool IsVariadicFn,
if (!getContext().getLangOpts().CPlusPlus || isDarwinPCS())
return ABIArgInfo::getIgnore();
- // In C++ mode, arguments which are empty and have sizeof() == 0 (which are
- // non-standard C++) are ignored.
- 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.
diff --git a/clang/test/CodeGen/AArch64/args.cpp
b/clang/test/CodeGen/AArch64/args.cpp
index 7acf26b2389efb..c95b63c61bba63 100644
--- a/clang/test/CodeGen/AArch64/args.cpp
+++ b/clang/test/CodeGen/AArch64/args.cpp
@@ -45,7 +45,8 @@ 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;
@@ -53,7 +54,7 @@ struct SortOfEmpty {
// 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)
+// CHECK-GNU-CXX: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
EXTERNC int sort_of_empty_arg(struct SortOfEmpty e, int a) {
return a;
}
>From 5a46190f1fe9cced2ee51ea10fc57bcb69d327fa Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stann...@arm.com>
Date: Fri, 13 Dec 2024 10:25:58 +0000
Subject: [PATCH 4/4] [AArch64] Add tests for variadic functions
---
clang/test/CodeGen/AArch64/args.cpp | 80 ++++++++++++++++++++++-------
1 file changed, 62 insertions(+), 18 deletions(-)
diff --git a/clang/test/CodeGen/AArch64/args.cpp
b/clang/test/CodeGen/AArch64/args.cpp
index c95b63c61bba63..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,9 +38,9 @@ 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;
}
@@ -52,17 +52,61 @@ 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(i32 noundef %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;
+}
+
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
