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 80F233857025 for ; Fri, 19 Mar 2021 13:04:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 80F233857025 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 D24EB31B; Fri, 19 Mar 2021 06:04:21 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0A3A03F718; Fri, 19 Mar 2021 06:04:20 -0700 (PDT) From: Richard Sandiford To: Jakub Jelinek Mail-Followup-To: Jakub Jelinek , Richard Earnshaw , Marcus Shawcroft , Kyrylo Tkachov , gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: Richard Earnshaw , Marcus Shawcroft , Kyrylo Tkachov , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] aarch64: Don't emit -Wpsabi note when ABI was never affected [PR91710] References: <20210319094659.GF231854@tucnak> Date: Fri, 19 Mar 2021 13:04:19 +0000 In-Reply-To: <20210319094659.GF231854@tucnak> (Jakub Jelinek's message of "Fri, 19 Mar 2021 10:46:59 +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=-6.6 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Mar 2021 13:04:24 -0000 Jakub Jelinek writes: > Hi! > > As the following testcase shows, we emit a -Wpsabi note about argument > passing change since GCC 9, but in reality the ABI didn't change. > The alignment is 8 bits in GCC < 9 and 32 bits in GCC >= 9 and > the aarch64_function_arg_alignment returns in that case: > return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY); > so when both the old and new alignment are smaller or equal to PARM_BOUNDARY > (or both are larger than STACK_BOUNDARY, just in theory), even when the new > one is bigger, it doesn't change the argument passing. > > So, the following patch changes aarch64_function_arg_alignment to tell the > callers the exact old alignmentm so that they can test it if needed. > The other aarch64_function_arg_alignment callers either check the > alignment for equality against 16-byte alignment (when old alignment was > smaller than that and the new one is 16-byte, we want to emit -Wpsabi > in all the cases) or the va_arg case which I think is ok now too. > > Bootstrapped/regtested on aarch64-linux, ok for trunk? Looks good to me. Richard E knows this code better than I do though, so I think he should have the final say. He's currently on holiday but will be back next week. Thanks, Richard > > 2021-03-18 Jakub Jelinek > > PR target/91710 > * config/aarch64/aarch64.c (aarch64_function_arg_alignment): Change > abi_break argument from bool * to unsigned *, store there the pre-GCC 9 > alignment. > (aarch64_layout_arg, aarch64_gimplify_va_arg_expr): Adjust callers. > (aarch64_function_arg_regno_p): Likewise. Only emit -Wpsabi note if > the old and new alignment after applying MIN/MAX to it is different. > > * gcc.target/aarch64/pr91710.c: New test. > > --- gcc/config/aarch64/aarch64.c.jj 2021-03-18 15:14:51.721425223 +0100 > +++ gcc/config/aarch64/aarch64.c 2021-03-18 16:35:04.437115447 +0100 > @@ -5938,9 +5938,9 @@ aarch64_vfp_is_call_candidate (cumulativ > > static unsigned int > aarch64_function_arg_alignment (machine_mode mode, const_tree type, > - bool *abi_break) > + unsigned int *abi_break) > { > - *abi_break = false; > + *abi_break = 0; > if (!type) > return GET_MODE_ALIGNMENT (mode); > > @@ -5982,7 +5982,7 @@ aarch64_function_arg_alignment (machine_ > > if (bitfield_alignment > alignment) > { > - *abi_break = true; > + *abi_break = alignment; > return bitfield_alignment; > } > > @@ -6004,7 +6004,7 @@ aarch64_layout_arg (cumulative_args_t pc > int ncrn, nvrn, nregs; > bool allocate_ncrn, allocate_nvrn; > HOST_WIDE_INT size; > - bool abi_break; > + unsigned int abi_break; > > /* We need to do this once per argument. */ > if (pcum->aapcs_arg_processed) > @@ -6322,14 +6322,19 @@ aarch64_function_arg_regno_p (unsigned r > static unsigned int > aarch64_function_arg_boundary (machine_mode mode, const_tree type) > { > - bool abi_break; > + unsigned int abi_break; > unsigned int alignment = aarch64_function_arg_alignment (mode, type, > &abi_break); > + alignment = MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY); > if (abi_break & warn_psabi) > - inform (input_location, "parameter passing for argument of type " > - "%qT changed in GCC 9.1", type); > + { > + abi_break = MIN (MAX (abi_break, PARM_BOUNDARY), STACK_BOUNDARY); > + if (alignment != abi_break) > + inform (input_location, "parameter passing for argument of type " > + "%qT changed in GCC 9.1", type); > + } > > - return MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY); > + return alignment; > } > > /* Implement TARGET_GET_RAW_RESULT_MODE and TARGET_GET_RAW_ARG_MODE. */ > @@ -16616,7 +16621,7 @@ aarch64_gimplify_va_arg_expr (tree valis > f_stack, NULL_TREE); > size = int_size_in_bytes (type); > > - bool abi_break; > + unsigned int abi_break; > align > = aarch64_function_arg_alignment (mode, type, &abi_break) / BITS_PER_UNIT; > > --- gcc/testsuite/gcc.target/aarch64/pr91710.c.jj 2021-03-18 16:42:33.529232710 +0100 > +++ gcc/testsuite/gcc.target/aarch64/pr91710.c 2021-03-18 16:42:07.271518121 +0100 > @@ -0,0 +1,16 @@ > +/* PR target/91710 */ > +/* { dg-do compile } */ > + > +struct S { unsigned int i:4; }; > + > +unsigned int test1(struct S s) { /* { dg-bogus "parameter passing for argument of type" } */ > + return s.i; > +} > + > +unsigned int test2(unsigned x, struct S s) { /* { dg-bogus "parameter passing for argument of type" } */ > + return x - s.i; > +} > + > +unsigned int test3(unsigned x, unsigned y, struct S s) { /* { dg-bogus "parameter passing for argument of type" } */ > + return x - y - s.i; > +} > > Jakub