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 0B71F3858407 for ; Fri, 10 Mar 2023 18:08:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0B71F3858407 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 08B0C4B3; Fri, 10 Mar 2023 10:09:40 -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 E13073F5A1; Fri, 10 Mar 2023 10:08:55 -0800 (PST) From: Richard Sandiford To: Prathamesh Kulkarni Mail-Followup-To: Prathamesh Kulkarni ,gcc Patches , rguenther@suse.de, richard.sandiford@arm.com Cc: gcc Patches , rguenther@suse.de Subject: Re: [aarch64] Use dup and zip1 for interleaving elements in initializing vector References: Date: Fri, 10 Mar 2023 18:08:54 +0000 In-Reply-To: (Prathamesh Kulkarni's message of "Sat, 11 Feb 2023 14:42:50 +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=-27.4 required=5.0 tests=BAYES_00,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Sorry for the slow reply. Prathamesh Kulkarni writes: > Unfortunately it regresses code-gen for the following case: > > svint32_t f(int32x4_t x) > { > return svdupq_s32 (x[0], x[1], x[2], x[3]); > } > > -O2 code-gen with trunk: > f: > dup z0.q, z0.q[0] > ret > > -O2 code-gen with patch: > f: > dup s1, v0.s[1] > mov v2.8b, v0.8b > ins v1.s[1], v0.s[3] > ins v2.s[1], v0.s[2] > zip1 v0.4s, v2.4s, v1.4s > dup z0.q, z0.q[0] > ret > > IIUC, svdupq_impl::expand uses aarch64_expand_vector_init > to initialize the "base 128-bit vector" and then use dupq to replicate it. > > Without patch, aarch64_expand_vector_init generates fallback code, and then > combine optimizes a sequence of vec_merge/vec_select pairs into an assignment: > > (insn 7 3 8 2 (set (reg:SI 99) > (vec_select:SI (reg/v:V4SI 97 [ x ]) > (parallel [ > (const_int 1 [0x1]) > ]))) "bar.c":6:10 2592 {aarch64_get_lanev4si} > (nil)) > > (insn 13 9 15 2 (set (reg:V4SI 102) > (vec_merge:V4SI (vec_duplicate:V4SI (reg:SI 99)) > (reg/v:V4SI 97 [ x ]) > (const_int 2 [0x2]))) "bar.c":6:10 1794 {aarch64_simd_vec_setv4si} > (expr_list:REG_DEAD (reg:SI 99) > (expr_list:REG_DEAD (reg/v:V4SI 97 [ x ]) > (nil)))) > > into: > Trying 7 -> 13: > 7: r99:SI=vec_select(r97:V4SI,parallel) > 13: r102:V4SI=vec_merge(vec_duplicate(r99:SI),r97:V4SI,0x2) > REG_DEAD r99:SI > REG_DEAD r97:V4SI > Successfully matched this instruction: > (set (reg:V4SI 102) > (reg/v:V4SI 97 [ x ])) > > which eventually results into: > (note 2 25 3 2 NOTE_INSN_DELETED) > (note 3 2 7 2 NOTE_INSN_FUNCTION_BEG) > (note 7 3 8 2 NOTE_INSN_DELETED) > (note 8 7 9 2 NOTE_INSN_DELETED) > (note 9 8 13 2 NOTE_INSN_DELETED) > (note 13 9 15 2 NOTE_INSN_DELETED) > (note 15 13 17 2 NOTE_INSN_DELETED) > (note 17 15 18 2 NOTE_INSN_DELETED) > (note 18 17 22 2 NOTE_INSN_DELETED) > (insn 22 18 23 2 (parallel [ > (set (reg/i:VNx4SI 32 v0) > (vec_duplicate:VNx4SI (reg:V4SI 108))) > (clobber (scratch:VNx16BI)) > ]) "bar.c":7:1 5202 {aarch64_vec_duplicate_vqvnx4si_le} > (expr_list:REG_DEAD (reg:V4SI 108) > (nil))) > (insn 23 22 0 2 (use (reg/i:VNx4SI 32 v0)) "bar.c":7:1 -1 > (nil)) > > I was wondering if we should add the above special case, of assigning > target = vec in aarch64_expand_vector_init, if initializer is { > vec[0], vec[1], ... } ? I'm not sure it will be easy to detect that. Won't the inputs to aarch64_expand_vector_init just be plain registers? It's not a good idea in general to search for definitions of registers during expansion. It would be nice to fix this by lowering svdupq into: (a) a constructor for a 128-bit vector (b) a duplication of the 128-bit vector to fill an SVE vector But I'm not sure what the best way of doing (b) would be. In RTL we can use vec_duplicate, but I don't think gimple has an equivalent construct. Maybe Richi has some ideas. We're planning to implement the ACLE's Neon-SVE bridge: https://github.com/ARM-software/acle/blob/main/main/acle.md#neon-sve-bridge and so we'll need (b) to implement the svdup_neonq functions. Thanks, Richard