@@ -307,7 +307,12 @@ AArch64ABIInfo::classifyArgumentType(QualType Ty, bool
IsVariadic,
// 0.
if (IsEmpty && Size == 0)
return ABIArgInfo::getIgnore();
-return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
+// An empty struct can have size
https://github.com/hstk30-hw edited
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/efriedma-quic edited
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -307,7 +307,12 @@ AArch64ABIInfo::classifyArgumentType(QualType Ty, bool
IsVariadic,
// 0.
if (IsEmpty && Size == 0)
return ABIArgInfo::getIgnore();
-return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
+// An empty struct can have size
hstk30-hw wrote:
For now,
empty record with size <= 64, still use `i8` in IR (before empty record without
align always use `i8`), and lowering to a general purpose register.
empty record with size <= 128, use `i128` in IR, and lowering to two
gr-registers (C.10).
empty record with size > 128,
https://github.com/hstk30-hw updated
https://github.com/llvm/llvm-project/pull/72197
>From 9a00824d0ac164d94c9cabfb4544eb6ef2e81802 Mon Sep 17 00:00:00 2001
From: hstk30-hw
Date: Sat, 18 Nov 2023 11:00:29 +
Subject: [PATCH] fix: empty record size > 64 with align let va_list get out of
sync
@@ -307,7 +307,13 @@ AArch64ABIInfo::classifyArgumentType(QualType Ty, bool
IsVariadic,
// 0.
if (IsEmpty && Size == 0)
return ABIArgInfo::getIgnore();
-return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
+// An empty struct can have size
https://github.com/efriedma-quic commented:
Good to see this works without any unexpected side-effects.
Please also add a test for 32-byte alignment (since that generates different
code)
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits
https://github.com/efriedma-quic edited
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hstk30-hw updated
https://github.com/llvm/llvm-project/pull/72197
>From 76b86014cb4af9fe5b163d61a96597217d6e843c Mon Sep 17 00:00:00 2001
From: hstk30-hw
Date: Sat, 18 Nov 2023 11:00:29 +
Subject: [PATCH] fix: empty record size > 64 with align let va_list get out of
sync
https://github.com/rjmccall reopened
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall wrote:
> The code you noted is supposed to handle two cases, neither of which are
> relevant to your testcase:
>
> * Darwin-specific calling convention rules.
> * GNU extensions for zero-size structs (which aren't allowed according to
> either C or C++ standards, but GNU invented a bu
https://github.com/rjmccall closed
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rjmccall wrote:
> This is the code I debug located. Seem the comment is out of date?
That seems like the right place to fix the issue, specifically the place below
where it always passes a single `i8`. The right rule is to just fall through
to the normal conventions for passing the argument i
efriedma-quic wrote:
The proper fix here is probably to just delete the `return
ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));` from the empty
struct codepath on aarch64.
Alignment shouldn't affect whether a class is empty. The issue here is just
that according to aarch64 AAPC
hstk30-hw wrote:
This is the code I debug located. Seem the comment is out of date?
https://github.com/llvm/llvm-project/blob/1b82cc1186b8bf716205c9b86b851db667792a64/clang/lib/CodeGen/Targets/AArch64.cpp#L298C1-L311C4
This issue
https://github.com/llvm/llvm-project/commit/1711cc930bda8d27e8
https://github.com/hstk30-hw reopened
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/hstk30-hw closed
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/rjmccall requested changes to this pull request.
Marking as changes requested so that this doesn't get committed.
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.l
rjmccall wrote:
Oh, the [Apple AArch64 calling
convention](https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms#Pass-arguments-to-functions-correctly)
diverges from AAPCS and ignores empty classes as parameters. We appear to
consider this an empty class regar
rjmccall wrote:
First off, the change here actually applies to all over-aligned empty structs,
not just to those with aligned bit-fields. Maybe we can say that aligned
zero-width bit-fields are ill-formed, but I don't think we can say that all
aligned empty classes are ill-formed, among other
@@ -296,10 +296,16 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType
T, bool AllowArrays,
return false;
// If this is a C++ record, check the bases first.
- if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+ if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
@@ -65,3 +65,9 @@ EXTERNC struct SortOfEmpty sort_of_empty_ret(void) {
struct SortOfEmpty e;
return e;
}
+
+// CHECK-GNU-CXX: define{{.*}} i32 @empty_align_arg(i128 %a.coerce, i32
noundef %b)
+struct EmptyAlign { long long int __attribute__((aligned)) : 0; };
+EXTERNC int
@@ -65,3 +65,9 @@ EXTERNC struct SortOfEmpty sort_of_empty_ret(void) {
struct SortOfEmpty e;
return e;
}
+
+// CHECK-GNU-CXX: define{{.*}} i32 @empty_align_arg(i128 %a.coerce, i32
noundef %b)
+struct EmptyAlign { long long int __attribute__((aligned)) : 0; };
-
https://github.com/AaronBallman commented:
Thank you for this! Please be sure to add a release note to
`clang/docs/ReleaseNotes.rst` as well so users know about the change in
behavior.
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits m
@@ -296,10 +296,16 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType
T, bool AllowArrays,
return false;
// If this is a C++ record, check the bases first.
- if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+ if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
--
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -296,10 +296,16 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType
T, bool AllowArrays,
return false;
// If this is a C++ record, check the bases first.
- if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+ if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
https://github.com/hstk30-hw edited
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hstk30-hw wrote:
@AaronBallman @erichkeane Check it plz.
https://github.com/llvm/llvm-project/pull/72197
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: None (hstk30-hw)
Changes
About https://github.com/llvm/llvm-project/issues/69872 , just for compatible
C++ empty record with align UB with gcc
https://godbolt.org/z/qsze8fqra
---
Full diff: https://github.com/llvm/llvm-project/
https://github.com/hstk30-hw created
https://github.com/llvm/llvm-project/pull/72197
About https://github.com/llvm/llvm-project/issues/69872 , just for compatible
C++ empty record with align UB with gcc
https://godbolt.org/z/qsze8fqra
>From 84472eea164fe78e061e6c85a4f3a7874c5957a3 Mon Sep 1
32 matches
Mail list logo