From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id F2F5F3858C66 for ; Thu, 12 Jan 2023 13:19:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F2F5F3858C66 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BC36213D5; Thu, 12 Jan 2023 05:20:33 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.99.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3126F3F67D; Thu, 12 Jan 2023 05:19:51 -0800 (PST) From: Richard Sandiford To: Christophe Lyon Mail-Followup-To: Christophe Lyon ,, richard.sandiford@arm.com Cc: Subject: Re: [PATCH v3 2/2] aarch64: Fix bit-field alignment in param passing [PR105549] References: <20230111141806.258233-1-christophe.lyon@arm.com> <20230111141806.258233-2-christophe.lyon@arm.com> Date: Thu, 12 Jan 2023 13:19:49 +0000 In-Reply-To: <20230111141806.258233-2-christophe.lyon@arm.com> (Christophe Lyon's message of "Wed, 11 Jan 2023 15:18:06 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-31.9 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Christophe Lyon writes: > While working on enabling DFP for AArch64, I noticed new failures in > gcc.dg/compat/struct-layout-1.exp (t028) which were not actually > caused by DFP types handling. These tests are generated during 'make > check' and enabling DFP made generation different (not sure if new > non-DFP tests are generated, or if existing ones are generated > differently, the tests in question are huge and difficult to compare). > > Anyway, I reduced the problem to what I attach at the end of the new > gcc.target/aarch64/aapcs64/va_arg-17.c test and rewrote it in the same > scheme as other va_arg* AArch64 tests. Richard Sandiford further > reduced this to a non-vararg function, added as a second testcase. > > This is a tough case mixing bit-fields and alignment, where > aarch64_function_arg_alignment did not follow what its descriptive > comment says: we want to use the natural alignment of the bit-field > type only if the user didn't reduce the alignment for the bit-field > itself. > > The patch also adds a comment and assert that would help someone who > has to look at this area again. > > The fix would be very small, except that this introduces a new ABI > break, and we have to warn about that. Since this actually fixes a > problem introduced in GCC 9.1, we keep the old computation to detect > when we now behave differently. > > This patch adds two new tests (va_arg-17.c and > pr105549.c). va_arg-17.c contains the reduced offending testcase from > struct-layout-1.exp for reference. We update some tests introduced by > the previous patch, where parameters with bit-fields and packed > attribute now emit a different warning. > > v2->v3: testcase update > > 2022-11-28 Christophe Lyon > Richard Sandiford > > gcc/ > PR target/105549 > * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): > Check DECL_PACKED for bitfield. > (aarch64_layout_arg): Warn when parameter passing ABI changes. > (aarch64_function_arg_boundary): Do not warn here. > (aarch64_gimplify_va_arg_expr): Warn when parameter passing ABI > changes. > > gcc/testsuite/ > PR target/105549 > * gcc.target/aarch64/bitfield-abi-warning-align16-O2.c: Update. > * gcc.target/aarch64/bitfield-abi-warning-align16-O2-extra.c: Update. > * gcc.target/aarch64/bitfield-abi-warning-align32-O2.c: Update. > * gcc.target/aarch64/bitfield-abi-warning-align32-O2-extra.c: Update. > * gcc.target/aarch64/aapcs64/va_arg-17.c: New test. > * gcc.target/aarch64/pr105549.c: New test. > * g++.target/aarch64/bitfield-abi-warning-align16-O2.C: Update. > * g++.target/aarch64/bitfield-abi-warning-align16-O2-extra.C: Update. > * g++.target/aarch64/bitfield-abi-warning-align32-O2.C: Update. > * g++.target/aarch64/bitfield-abi-warning-align32-O2-extra.C: Update. > --- > gcc/config/aarch64/aarch64.cc | 148 ++++++++++++++---- > .../bitfield-abi-warning-align16-O2-extra.C | 64 ++++---- > .../aarch64/bitfield-abi-warning-align16-O2.C | 48 +++--- > .../bitfield-abi-warning-align32-O2-extra.C | 131 +++++++--------- > .../aarch64/bitfield-abi-warning-align32-O2.C | 132 ++++++++-------- > .../gcc.target/aarch64/aapcs64/va_arg-17.c | 105 +++++++++++++ > .../bitfield-abi-warning-align16-O2-extra.c | 64 ++++---- > .../aarch64/bitfield-abi-warning-align16-O2.c | 48 +++--- > .../bitfield-abi-warning-align32-O2-extra.c | 131 +++++++--------- > .../aarch64/bitfield-abi-warning-align32-O2.c | 132 ++++++++-------- > gcc/testsuite/gcc.target/aarch64/pr105549.c | 12 ++ > 11 files changed, 587 insertions(+), 428 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/aarch64/aapcs64/va_arg-17.c > create mode 100644 gcc/testsuite/gcc.target/aarch64/pr105549.c > [...] > @@ -68,14 +62,14 @@ > /* { dg-note {parameter passing for argument of type 'S4' changed in GCC 9.1} "" { target *-*-* } 103 } f4_stdarg */ > /* { dg-note {parameter passing for argument of type 'S8' changed in GCC 9.1} "" { target *-*-* } 104 } f8_stdarg */ > > -/* Parameter passing for these should not have changed in GCC 9.1 (PR 105549). > +/* FIXME Parameter passing for these should not have changed in GCC 9.1 (PR 105549). > Fortunately we warn. Note the discrepancy with lines 120-124 below: we warn > in the callee, but not in the caller. */ Looks like a stray change. Same for the C test. OK otherwise, thanks. Richard