[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-21 Thread via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-21 Thread via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-21 Thread Eli Friedman via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-21 Thread Eli Friedman via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-20 Thread via cfe-commits
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,

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-20 Thread via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-20 Thread Eli Friedman via cfe-commits
@@ -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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-20 Thread Eli Friedman via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-20 Thread Eli Friedman via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-17 Thread via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread John McCall via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread John McCall via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread John McCall via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread John McCall via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread Eli Friedman via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-16 Thread via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread John McCall via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread John McCall via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread John McCall via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Aaron Ballman via 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)) {

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Aaron Ballman via cfe-commits
@@ -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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Aaron Ballman via cfe-commits
@@ -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; }; -

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Aaron Ballman via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Aaron Ballman via 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)) { --

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Aaron Ballman via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread Erich Keane via 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)) {

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-15 Thread via cfe-commits
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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-13 Thread via 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

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-13 Thread via 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/

[clang] fix: compatible C++ empty record with align UB with gcc (PR #72197)

2023-11-13 Thread via cfe-commits
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