From patchwork Mon Dec 11 15:23:02 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 176742 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp7129369vqy; Mon, 11 Dec 2023 07:23:30 -0800 (PST) X-Google-Smtp-Source: AGHT+IEW/yZZF8abXD2+cDYXKPH+00o56BRZHrUOH8JHagQBNFZMvuDrToU6eahqlArFSyhKoLa9 X-Received: by 2002:a05:6122:4684:b0:4b2:d41f:e457 with SMTP id di4-20020a056122468400b004b2d41fe457mr3251221vkb.1.1702308209751; Mon, 11 Dec 2023 07:23:29 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1702308209; cv=pass; d=google.com; s=arc-20160816; b=xPY3jBsPD2OGhta/aj0JmzWY2Nx1fR19hKIaO4A3DDM2TlKNiumC7L9L0vvq1/559d LzykEoydFQinfaZ+WzjqO5BMnP6ozLf2tGTxbLZifyUeBEqP89y+YmUTf5jt2de2h19I 8mqs3p89sc1GdwQAkQuaoqMG4u5AzUrVz01LQbbe2MvfKamxtlhTRIoH6zPauHvOwwk1 9BFpUtb0zp3frxD8oiGFaUGzZR2qY8gHWC9uHRy+2wdbiNOsnvtw/F0fRZNADOiW5ZPF ZAUBpJ9qq2P5n9JkKhEYHajzAwW51l2B3G4hRdKvHZK8LMhlSQ4gfDw3cP+4cVSR0AEg o/8A== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:mime-version:user-agent :message-id:in-reply-to:date:references:subject:mail-followup-to:to :from:arc-filter:dmarc-filter:delivered-to; bh=Xuyy+AeABluCKphwoEEkzsWLnow1kXUyw0hTwkYX850=; fh=hPrbWPhweUx4V0GV9uXJqbyAzg2ABmTz7kczrAQqMmM=; b=JOFQeaV6xwEhwgnTNFMiTOEho0veqJ+APtOLtnB6vJmTmzP6eB4qt9DPEbVJxzQj5p 1aBDojpt0/0PHbK1NxeSdqVij43bf6eJ+AmKuAA4Zi84RX4nXqj9WwL9h/TB7cDG+7Ic dn80VvlSrpqwSJRyqb02yTL9qaxTAOgt6fOUxi2uBjmJVqkUo4SfwEBfcFwUZM1gUF7F QUdlVDFQv8bFE3PqK3hoa9z/TaaQzdnlPTbzc3qxjqUsxLfZswnBYOitaiohZqYYTGwG TXasnMzVEUYHl9sAHTm/NHP1DF6TJUWEM4tmaxa2bFHt+lzwCmv7UN17JOmTzNykb7IK S8Xg== ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id fg11-20020a0561224c8b00b004b2dab697d6si1618293vkb.43.2023.12.11.07.23.29 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Dec 2023 07:23:29 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 71067385800E for ; Mon, 11 Dec 2023 15:23:29 +0000 (GMT) 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 4D6C33858D28 for ; Mon, 11 Dec 2023 15:23:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4D6C33858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4D6C33858D28 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=217.140.110.172 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702308186; cv=none; b=MZjiX/6AADjkRgd3Mt/TyK+cny08yz0rf6c/MCxPWwwVCtkPERe4gh+xBnRqI47/5PXVBGxSmAR8/FcU84RWuettAFJ2YcUXLiRFblo4jvX5kEaC0wSTo1XQaYPNFGHdDj0Cv2AacSG676yZ722hJAd2jAiDoWot8gbjw79DOyw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702308186; c=relaxed/simple; bh=oPm5khI3WyCYJd/OBvkx1aO4a8vOWUNYps+SIODZ35M=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=qWWyOLis8YUL/vWP4+UD/hsojXkHLpXf01udUBbdRTTkzx5dPtd0L5CH1ymicLcxEPRFNInkoQaEPnpeqd9d90hV7hGE3Sm7szlH0g+vw4hEpRTtaDf1fwi9z2h7XZuOSPdLUHCk6nAXZ+ey4cdkloodJi47DhQSdTetkzTFWec= ARC-Authentication-Results: i=1; server2.sourceware.org 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 5AC07FEC for ; Mon, 11 Dec 2023 07:23:50 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 6D0F43F738 for ; Mon, 11 Dec 2023 07:23:03 -0800 (PST) From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: Ping: [PATCH] Treat "p" in asms as addressing VOIDmode References: Date: Mon, 11 Dec 2023 15:23:02 +0000 In-Reply-To: (Richard Sandiford's message of "Mon, 27 Nov 2023 12:12:11 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-21.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784999533480650061 X-GMAIL-MSGID: 1784999533480650061 Ping --- check_asm_operands was inconsistent about how it handled "p" after RA compared to before RA. Before RA it tested the address with a void (unknown) memory mode: case CT_ADDRESS: /* Every address operand can be reloaded to fit. */ result = result || address_operand (op, VOIDmode); break; After RA it deferred to constrain_operands, which used the mode of the operand: if ((GET_MODE (op) == VOIDmode || SCALAR_INT_MODE_P (GET_MODE (op))) && (strict <= 0 || (strict_memory_address_p (recog_data.operand_mode[opno], op)))) win = true; Using the mode of the operand matches reload's behaviour: else if (insn_extra_address_constraint (lookup_constraint (constraints[i]))) { address_operand_reloaded[i] = find_reloads_address (recog_data.operand_mode[i], (rtx*) 0, recog_data.operand[i], recog_data.operand_loc[i], i, operand_type[i], ind_levels, insn); It allowed the special predicate address_operand to be used, with the mode of the operand being the mode of the addressed memory, rather than the mode of the address itself. For example, vax has: (define_insn "*movaddr" [(set (match_operand:SI 0 "nonimmediate_operand" "=g") (match_operand:VAXfp 1 "address_operand" "p")) (clobber (reg:CC VAX_PSL_REGNUM))] "reload_completed" "mova %a1,%0") where operand 1 is an SImode expression that can address memory of mode VAXfp. GET_MODE (recog_data.operand[1]) is SImode (or VOIDmode), but recog_data.operand_mode[1] is mode. But AFAICT, ira and lra (like pre-reload check_asm_operands) do not do this, and instead pass VOIDmode. So I think this traditional use of address_operand is effectively an old-reload-only feature. And it seems like no modern port cares. I think ports have generally moved to using different address constraints instead, rather than relying on "p" with different operand modes. Target-specific address constraints post-date the code above. The big advantage of using different constraints is that it works for asms too. And that (to finally get to the point) is the problem fixed in this patch. For the aarch64 test: void f(char *p) { asm("prfm pldl1keep, %a0\n" :: "p" (p + 6)); } everything up to and including RA required the operand to be a valid VOIDmode address. But post-RA check_asm_operands and constrain_operands instead required it to be valid for recog_data.operand_mode[0]. Since asms have no syntax for specifying an operand mode that's separate from the operand itself, operand_mode[0] is simply Pmode (i.e. DImode). This meant that we required one mode before RA and a different mode after RA. On AArch64, VOIDmode is treated as a wildcard and so has a more conservative/restricted range than DImode. So if a post-RA pass tried to form a new address, it would use a laxer condition than the pre-RA passes. This happened with the late-combine pass that I posted in October: https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634166.html which in turn triggered an error from aarch64_print_operand_address. This patch takes the (hopefully) conservative fix of using VOIDmode for asms but continuing to use the operand mode for .md insns, so as not to break ports that still use reload. Fixing this made me realise that recog_level2 was doing duplicate work for asms after RA. Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? Richard gcc/ * recog.cc (constrain_operands): Pass VOIDmode to strict_memory_address_p for 'p' constraints in asms. * rtl-ssa/changes.cc (recog_level2): Skip redundant constrain_operands for asms. gcc/testsuite/ * gcc.target/aarch64/prfm_imm_offset_2.c: New test. --- gcc/recog.cc | 18 +++++++++++------- gcc/rtl-ssa/changes.cc | 4 +++- .../gcc.target/aarch64/prfm_imm_offset_2.c | 2 ++ 3 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/prfm_imm_offset_2.c diff --git a/gcc/recog.cc b/gcc/recog.cc index eaab79c25d7..bff7be1aec1 100644 --- a/gcc/recog.cc +++ b/gcc/recog.cc @@ -3191,13 +3191,17 @@ constrain_operands (int strict, alternative_mask alternatives) strictly valid, i.e., that all pseudos requiring hard regs have gotten them. We also want to make sure we have a valid mode. */ - if ((GET_MODE (op) == VOIDmode - || SCALAR_INT_MODE_P (GET_MODE (op))) - && (strict <= 0 - || (strict_memory_address_p - (recog_data.operand_mode[opno], op)))) - win = true; - break; + { + auto mem_mode = (recog_data.is_asm + ? VOIDmode + : recog_data.operand_mode[opno]); + if ((GET_MODE (op) == VOIDmode + || SCALAR_INT_MODE_P (GET_MODE (op))) + && (strict <= 0 + || strict_memory_address_p (mem_mode, op))) + win = true; + break; + } /* No need to check general_operand again; it was done in insn-recog.cc. Well, except that reload diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc index 2f2d12d5f30..443d0575df5 100644 --- a/gcc/rtl-ssa/changes.cc +++ b/gcc/rtl-ssa/changes.cc @@ -986,8 +986,10 @@ recog_level2 (insn_change &change, add_regno_clobber_fn add_regno_clobber) pat = newpat; } + // check_asm_operands checks the constraints after RA, so we don't + // need to do it again. INSN_CODE (rtl) = icode; - if (reload_completed) + if (reload_completed && !asm_p) { extract_insn (rtl); if (!constrain_operands (1, get_preferred_alternatives (rtl))) diff --git a/gcc/testsuite/gcc.target/aarch64/prfm_imm_offset_2.c b/gcc/testsuite/gcc.target/aarch64/prfm_imm_offset_2.c new file mode 100644 index 00000000000..04e3fb72c45 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/prfm_imm_offset_2.c @@ -0,0 +1,2 @@ +/* { dg-options "-O2" } */ +void f(char *p) { asm("prfm pldl1keep, %a0\n" :: "p" (p + 6)); }