From patchwork Wed May 3 16:48:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 89783 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1469368vqo; Wed, 3 May 2023 09:49:24 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6wWGCBLQabHQj/C+HAqqhsKyWeWwnHGAVgvvtEiwl+EOR+O1zlbxaAll9A+WTAPD7ygniq X-Received: by 2002:aa7:cfd6:0:b0:506:7385:9653 with SMTP id r22-20020aa7cfd6000000b0050673859653mr11584778edy.39.1683132564326; Wed, 03 May 2023 09:49:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683132564; cv=none; d=google.com; s=arc-20160816; b=N7Fqf9XGmtcZcWAoUAZakGqagWx7itCfVNq7oGkgSyArWisy2m7Cqb6BgGVYG0A5zK LcYpRq0LWvjC4T/mjZ40S6gIQTYG5lDGOhplXvHXgFgftGbHiT8QM4JCSkW4Sw86cXiz yRbVOYWSpMzXwHUGDhLVQ6Yb5QS1cjxEJTueVN7irgE79WNiMKukobC8f/nSZKaDB4es Vz0kFE1OWklrxV8qG03WjpOseivExBPoJ0YkdYaUFdjHfKE5+zhQVyVqwWLucJldkicD FYwIEq9r80HThyYzxUAe90f6m/zMMHRUW/IOcNzfHONILFRivKIbx6FIYbIael20Lizy Tsow== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:mime-version :user-agent:message-id:date:subject:mail-followup-to:to:dmarc-filter :delivered-to:dkim-signature:dkim-filter; bh=ggvhsl5UlpQseEk7XWicTDdaJ+L3O9Dp02bUb/ryAVA=; b=eyBKXvkm2EoBbouvremSkD0yBv8aZy8bMigY8yt5XSC88eKqDtGarLFwR7mhsXdLAA tPoLPdG7aeNEzKbhOpO7IVF7jOSBFSyOJ2RLFAPGvxoc4+IJ5BU2kp3uVfqDe9ond6l0 fdhVnuRtclzPcSVjfKKPbnw1ZZgQBRWOQb5p7ctdU/XLcfGLv1pDlyAcYyKPoCqAdKgm 0kPWq4aZS2D1c0pP62xl+9H/k1I1VlLEjWD7i+qtCzT9s9eGIriqY9Foy23jACG/R9U2 S6H2Zz+x3ZyOAZwGbo/evlvDg2q2/+x5LapXsxJaQKa88YKy3GzJdV6idUg5Qw4jFoyR hdSw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="okPF/UR8"; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id s11-20020a056402014b00b004fe84497e17si1254094edu.26.2023.05.03.09.49.24 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 May 2023 09:49:24 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="okPF/UR8"; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8A4E43856DFE for ; Wed, 3 May 2023 16:49:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8A4E43856DFE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1683132553; bh=ggvhsl5UlpQseEk7XWicTDdaJ+L3O9Dp02bUb/ryAVA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=okPF/UR87IF8bKlt/RV8LTcAd97nA9bDXVlNtLVubjvhXKV3rYMb0VuFVe7J/9oRR CGntPo+ikbXyFQkkyvLvyL8vJbacGAMe4oV34sQDb97x9+a6DJPvWpy3MVkr20hxMY 65iuOaGNIfGupUodVCnLmsxqoEEkg32h07rJce88= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 35CC73858C50 for ; Wed, 3 May 2023 16:48:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 35CC73858C50 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 087F12F4 for ; Wed, 3 May 2023 09:49:11 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6BBF03F64C for ; Wed, 3 May 2023 09:48:26 -0700 (PDT) To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: [PATCH 1/2] aarch64: Rename abi_break parameters [PR109661] Date: Wed, 03 May 2023 17:48:25 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-30.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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-Patchwork-Original-From: Richard Sandiford via Gcc-patches From: Richard Sandiford Reply-To: Richard Sandiford Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764892411565250139?= X-GMAIL-MSGID: =?utf-8?q?1764892411565250139?= aarch64_function_arg_alignment has two related abi_break parameters: abi_break for a change in GCC 9, and abi_break_packed for a related follow-on change in GCC 13. In a sense, abi_break_packed is a "subfix" of abi_break. PR109661 now requires a third ABI break that is independent of the other two. Having abi_break for the GCC 9 break and abi_break_ for the GCC 13 and GCC 14 breaks might give the impression that they're all related, and that the GCC 14 fix (like the GCC 13 fix) is a "subfix" of the GCC 9 one. It therefore seemed like a good idea to rename the existing variables first. It would be difficult to choose names that describe briefly and precisely what went wrong in each case. The next best thing seemed to be to name them after the relevant GCC version. (Of course, this might break down in future if we need two independent fixes in the same version. Let's hope not.) I wondered about putting all the variables in a structure, but one advantage of using independent variables is that it's harder to forget to update a caller. Maybe a fourth parameter would be a tipping point. Tested on aarch64-linux-gnu & pushed. Richard gcc/ PR target/109661 * config/aarch64/aarch64.cc (aarch64_function_arg_alignment): Rename ABI break variables to abi_break_gcc_9 and abi_break_gcc_13. (aarch64_layout_arg, aarch64_function_arg_boundary): Likewise. (aarch64_gimplify_va_arg_expr): Likewise. --- gcc/config/aarch64/aarch64.cc | 70 ++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc index 2b0de7ca038..70916ad63d2 100644 --- a/gcc/config/aarch64/aarch64.cc +++ b/gcc/config/aarch64/aarch64.cc @@ -7464,19 +7464,20 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode, /* Given MODE and TYPE of a function argument, return the alignment in bits. The idea is to suppress any stronger alignment requested by the user and opt for the natural alignment (specified in AAPCS64 \S - 4.1). ABI_BREAK is set to the old alignment if the alignment was - incorrectly calculated in versions of GCC prior to GCC-9. - ABI_BREAK_PACKED is set to the old alignment if it was incorrectly - calculated in versions between GCC-9 and GCC-13. This is a helper - function for local use only. */ + 4.1). ABI_BREAK_GCC_9 is set to the old alignment if the alignment + was incorrectly calculated in versions of GCC prior to GCC 9. + ABI_BREAK_GCC_13 is set to the old alignment if it was incorrectly + calculated in versions between GCC 9 and GCC 13. + + This is a helper function for local use only. */ static unsigned int aarch64_function_arg_alignment (machine_mode mode, const_tree type, - unsigned int *abi_break, - unsigned int *abi_break_packed) + unsigned int *abi_break_gcc_9, + unsigned int *abi_break_gcc_13) { - *abi_break = 0; - *abi_break_packed = 0; + *abi_break_gcc_9 = 0; + *abi_break_gcc_13 = 0; if (!type) return GET_MODE_ALIGNMENT (mode); @@ -7547,11 +7548,11 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type, 'packed' attribute into account. */ if (bitfield_alignment != bitfield_alignment_with_packed && bitfield_alignment_with_packed > alignment) - *abi_break_packed = bitfield_alignment_with_packed; + *abi_break_gcc_13 = bitfield_alignment_with_packed; if (bitfield_alignment > alignment) { - *abi_break = alignment; + *abi_break_gcc_9 = alignment; return bitfield_alignment; } @@ -7573,8 +7574,8 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg) int ncrn, nvrn, nregs; bool allocate_ncrn, allocate_nvrn; HOST_WIDE_INT size; - unsigned int abi_break; - unsigned int abi_break_packed; + unsigned int abi_break_gcc_9; + unsigned int abi_break_gcc_13; /* We need to do this once per argument. */ if (pcum->aapcs_arg_processed) @@ -7612,7 +7613,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg) Versions prior to GCC 9.1 ignored a bitfield's underlying type and so could calculate an alignment that was too small. If this - happened for TYPE then ABI_BREAK is this older, too-small alignment. + happened for TYPE then ABI_BREAK_GCC_9 is this older, too-small alignment. Although GCC 9.1 fixed that bug, it introduced a different one: it would consider the alignment of a bitfield's underlying type even @@ -7620,7 +7621,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg) the alignment of the underlying type). This was fixed in GCC 13.1. As a result of this bug, GCC 9 to GCC 12 could calculate an alignment - that was too big. If this happened for TYPE, ABI_BREAK_PACKED is + that was too big. If this happened for TYPE, ABI_BREAK_GCC_13 is this older, too-big alignment. Also, the fact that GCC 9 to GCC 12 considered irrelevant @@ -7713,12 +7714,12 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg) gcc_assert (!sve_p || !allocate_nvrn); unsigned int alignment - = aarch64_function_arg_alignment (mode, type, &abi_break, - &abi_break_packed); + = aarch64_function_arg_alignment (mode, type, &abi_break_gcc_9, + &abi_break_gcc_13); gcc_assert ((allocate_nvrn || alignment <= 16 * BITS_PER_UNIT) - && (!alignment || abi_break < alignment) - && (!abi_break_packed || alignment < abi_break_packed)); + && (!alignment || abi_break_gcc_9 < alignment) + && (!abi_break_gcc_13 || alignment < abi_break_gcc_13)); /* allocate_ncrn may be false-positive, but allocate_nvrn is quite reliable. The following code thus handles passing by SIMD/FP registers first. */ @@ -7792,8 +7793,8 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg) /* Emit a warning if the alignment changed when taking the 'packed' attribute into account. */ if (warn_pcs_change - && abi_break_packed - && ((abi_break_packed == 16 * BITS_PER_UNIT) + && abi_break_gcc_13 + && ((abi_break_gcc_13 == 16 * BITS_PER_UNIT) != (alignment == 16 * BITS_PER_UNIT))) inform (input_location, "parameter passing for argument of type " "%qT changed in GCC 13.1", type); @@ -7804,7 +7805,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, const function_arg_info &arg) passed by reference rather than value. */ if (alignment == 16 * BITS_PER_UNIT) { - if (warn_pcs_change && abi_break) + if (warn_pcs_change && abi_break_gcc_9) inform (input_location, "parameter passing for argument of type " "%qT changed in GCC 9.1", type); ++ncrn; @@ -7863,8 +7864,8 @@ on_stack: pcum->aapcs_stack_words = size / UNITS_PER_WORD; if (warn_pcs_change - && abi_break_packed - && ((abi_break_packed >= 16 * BITS_PER_UNIT) + && abi_break_gcc_13 + && ((abi_break_gcc_13 >= 16 * BITS_PER_UNIT) != (alignment >= 16 * BITS_PER_UNIT))) inform (input_location, "parameter passing for argument of type " "%qT changed in GCC 13.1", type); @@ -7874,7 +7875,7 @@ on_stack: int new_size = ROUND_UP (pcum->aapcs_stack_size, 16 / UNITS_PER_WORD); if (pcum->aapcs_stack_size != new_size) { - if (warn_pcs_change && abi_break) + if (warn_pcs_change && abi_break_gcc_9) inform (input_location, "parameter passing for argument of type " "%qT changed in GCC 9.1", type); pcum->aapcs_stack_size = new_size; @@ -7991,11 +7992,11 @@ aarch64_function_arg_regno_p (unsigned regno) static unsigned int aarch64_function_arg_boundary (machine_mode mode, const_tree type) { - unsigned int abi_break; - unsigned int abi_break_packed; + unsigned int abi_break_gcc_9; + unsigned int abi_break_gcc_13; unsigned int alignment = aarch64_function_arg_alignment (mode, type, - &abi_break, - &abi_break_packed); + &abi_break_gcc_9, + &abi_break_gcc_13); /* We rely on aarch64_layout_arg and aarch64_gimplify_va_arg_expr to emit warnings about ABI incompatibility. */ alignment = MIN (MAX (alignment, PARM_BOUNDARY), STACK_BOUNDARY); @@ -19762,10 +19763,11 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, f_stack, NULL_TREE); size = int_size_in_bytes (type); - unsigned int abi_break; - unsigned int abi_break_packed; + unsigned int abi_break_gcc_9; + unsigned int abi_break_gcc_13; align - = aarch64_function_arg_alignment (mode, type, &abi_break, &abi_break_packed) + = aarch64_function_arg_alignment (mode, type, &abi_break_gcc_9, + &abi_break_gcc_13) / BITS_PER_UNIT; dw_align = false; @@ -19809,13 +19811,13 @@ aarch64_gimplify_va_arg_expr (tree valist, tree type, gimple_seq *pre_p, rsize = ROUND_UP (size, UNITS_PER_WORD); nregs = rsize / UNITS_PER_WORD; - if (align <= 8 && abi_break_packed && warn_psabi) + if (align <= 8 && abi_break_gcc_13 && warn_psabi) inform (input_location, "parameter passing for argument of type " "%qT changed in GCC 13.1", type); if (align > 8) { - if (abi_break && warn_psabi) + if (abi_break_gcc_9 && warn_psabi) inform (input_location, "parameter passing for argument of type " "%qT changed in GCC 9.1", type); dw_align = true;