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 6769F3857C54 for ; Mon, 21 Sep 2020 10:58:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 6769F3857C54 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=richard.sandiford@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 0AF3831B; Mon, 21 Sep 2020 03:58:31 -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 538043F70D; Mon, 21 Sep 2020 03:58:30 -0700 (PDT) From: Richard Sandiford To: Andrea Corallo Mail-Followup-To: Andrea Corallo , gcc-patches@gcc.gnu.org, nd@arm.com, richard.earnshaw@arm.com, richard.sandiford@arm.com Cc: gcc-patches@gcc.gnu.org, nd@arm.com, richard.earnshaw@arm.com Subject: Re: [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt References: Date: Mon, 21 Sep 2020 11:58:28 +0100 In-Reply-To: (Andrea Corallo's message of "Mon, 21 Sep 2020 12:31:29 +0200") 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=-12.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, 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: Mon, 21 Sep 2020 10:58:32 -0000 Andrea Corallo writes: > Hi all, > > From the `force_reg` description comment I see the returned register > should not be modified, thus IIUC should not be used as a GEN_FCN > target. > > Assuming my interpretation is correct this fix this case inside > `aarch64_general_expand_builtin` while expanding expanding the > `__jcvt` intrinsic. If is not the case please discard. Good catch. > Regtested and bootsraped on aarch64-linux-gnu. > > Andrea > > From 403ad66b8f9c108d7f38b406ed1afcb603b7e25f Mon Sep 17 00:00:00 2001 > From: Andrea Corallo > Date: Thu, 17 Sep 2020 17:17:52 +0100 > Subject: [PATCH] aarch64: Do not alter value on a force_reg returned rtx > expanding __jcvt > > 2020-09-17 Andrea Corallo > > * config/aarch64/aarch64-builtins.c > (aarch64_general_expand_builtin): Use expand machinery not to > alter the value of an rtx returned by force_reg. OK, thanks. Richard > --- > gcc/config/aarch64/aarch64-builtins.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c > index 4f33dd936c7..b787719cf5e 100644 > --- a/gcc/config/aarch64/aarch64-builtins.c > +++ b/gcc/config/aarch64/aarch64-builtins.c > @@ -2128,14 +2128,14 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target, > return target; > > case AARCH64_JSCVT: > - arg0 = CALL_EXPR_ARG (exp, 0); > - op0 = force_reg (DFmode, expand_normal (arg0)); > - if (!target) > - target = gen_reg_rtx (SImode); > - else > - target = force_reg (SImode, target); > - emit_insn (GEN_FCN (CODE_FOR_aarch64_fjcvtzs) (target, op0)); > - return target; > + { > + expand_operand ops[2]; > + create_output_operand (&ops[0], target, SImode); > + op0 = expand_normal (CALL_EXPR_ARG (exp, 0)); > + create_input_operand (&ops[1], op0, DFmode); > + expand_insn (CODE_FOR_aarch64_fjcvtzs, 2, ops); > + return ops[0].value; > + } > > case AARCH64_SIMD_BUILTIN_FCMLA_LANEQ0_V2SF: > case AARCH64_SIMD_BUILTIN_FCMLA_LANEQ90_V2SF: