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 8130C3858C66 for ; Thu, 12 Jan 2023 15:32:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8130C3858C66 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 51A2913D5; Thu, 12 Jan 2023 07:33:04 -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 BA87E3F587; Thu, 12 Jan 2023 07:32:21 -0800 (PST) From: Richard Sandiford To: Prathamesh Kulkarni Mail-Followup-To: Prathamesh Kulkarni ,gcc Patches , richard.sandiford@arm.com Cc: gcc Patches Subject: Re: Missed lowering to ld1rq from svld1rq for memory operand References: Date: Thu, 12 Jan 2023 15:32:20 +0000 In-Reply-To: (Prathamesh Kulkarni's message of "Tue, 10 Jan 2023 23:04:31 +0530") 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=-37.8 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 autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Prathamesh Kulkarni writes: > On Fri, 5 Aug 2022 at 17:49, Richard Sandiford > wrote: >> >> Prathamesh Kulkarni writes: >> > Hi Richard, >> > Following from off-list discussion, in the attached patch, I wrote pattern >> > similar to vec_duplicate_reg, which seems to work for the svld1rq tests. >> > Does it look OK ? >> > >> > Sorry, I didn't fully understand your suggestion on integrating with >> > vec_duplicate_reg >> > pattern. For vec_duplicate_reg, the operand to vec_duplicate expects >> > mode to be , while the pattern in patch expects operand of >> > vec_duplicate to have mode . >> > How do we write a pattern so an operand can accept either of the 2 modes ? >> >> I quoted the wrong one, sorry, should have been >> aarch64_vec_duplicate_vq_le. >> >> > Also it seems cannot be used with SVE_ALL ? >> >> Yeah, these would be SVE_FULL only. > Hi Richard, > Sorry for the very late reply. I have attached patch, to integrate > with vec_duplicate_vq_le. > Bootstrapped+tested on aarch64-linux-gnu. > OK to commit ? > > Thanks, > Prathamesh >> >> Richard >> > > gcc/ > * config/aarch64/aarch64-sve.md (aarch64_vec_duplicate_vq_le): > Change to define_insn_and_split to fold ldr+dup to ld1rq. > * config/aarch64/predicates.md (aarch64_sve_dup_ld1rq_operand): New. > > testsuite/ > * gcc.target/aarch64/sve/acle/general/pr96463-2.c: Adjust. > > diff --git a/gcc/config/aarch64/aarch64-sve.md b/gcc/config/aarch64/aarch64-sve.md > index b8cc47ef5fc..4548375b8d6 100644 > --- a/gcc/config/aarch64/aarch64-sve.md > +++ b/gcc/config/aarch64/aarch64-sve.md > @@ -2533,14 +2533,34 @@ > ) > > ;; Duplicate an Advanced SIMD vector to fill an SVE vector (LE version). > -(define_insn "@aarch64_vec_duplicate_vq_le" > - [(set (match_operand:SVE_FULL 0 "register_operand" "=w") > + > +(define_insn_and_split "@aarch64_vec_duplicate_vq_le" > + [(set (match_operand:SVE_FULL 0 "register_operand" "=w, w") > (vec_duplicate:SVE_FULL > - (match_operand: 1 "register_operand" "w")))] > + (match_operand: 1 "aarch64_sve_dup_ld1rq_operand" "w, UtQ"))) > + (clobber (match_scratch:VNx16BI 2 "=X, Upl"))] > "TARGET_SVE && !BYTES_BIG_ENDIAN" > { > - operands[1] = gen_rtx_REG (mode, REGNO (operands[1])); > - return "dup\t%0.q, %1.q[0]"; > + switch (which_alternative) > + { > + case 0: > + operands[1] = gen_rtx_REG (mode, REGNO (operands[1])); > + return "dup\t%0.q, %1.q[0]"; > + case 1: > + return "#"; > + default: > + gcc_unreachable (); > + } > + } > + "&& MEM_P (operands[1])" > + [(const_int 0)] > + { > + if (GET_CODE (operands[2]) == SCRATCH) > + operands[2] = gen_reg_rtx (VNx16BImode); > + emit_move_insn (operands[2], CONSTM1_RTX (VNx16BImode)); > + rtx gp = gen_lowpart (mode, operands[2]); > + emit_insn (gen_aarch64_sve_ld1rq (operands[0], operands[1], gp)); > + DONE; > } > ) > > diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md > index ff7f73d3f30..6062f37025e 100644 > --- a/gcc/config/aarch64/predicates.md > +++ b/gcc/config/aarch64/predicates.md > @@ -676,6 +676,10 @@ > (ior (match_operand 0 "register_operand") > (match_operand 0 "aarch64_sve_ld1r_operand"))) > > +(define_predicate "aarch64_sve_dup_ld1rq_operand" > + (ior (match_operand 0 "register_operand") > + (match_operand 0 "aarch64_sve_ld1rq_operand"))) > + > (define_predicate "aarch64_sve_ptrue_svpattern_immediate" > (and (match_code "const") > (match_test "aarch64_sve_ptrue_svpattern_p (op, NULL)"))) > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463-2.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463-2.c > index 196de3f5e0a..c38204e6874 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463-2.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr96463-2.c > @@ -26,4 +26,4 @@ TEST(svfloat64_t, float64_t, f64) > > TEST(svbfloat16_t, bfloat16_t, bf16) > > -/* { dg-final { scan-assembler-times {\tdup\tz[0-9]+\.q, z[0-9]+\.q\[0\]} 12 { target aarch64_little_endian } } } */ > +/* { dg-final { scan-assembler-not {\tdup\t} } } */ It would be good to add something like: /* { dg-final { scan-assembler-times {\tld1rq\t} 12 } } */ (I assume it'll pass for both endiannesses, but please check!), in addition to the scan-assembler-not. OK with that change, thanks. Richard