From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id CC844396ECF4 for ; Thu, 10 Sep 2020 09:14:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CC844396ECF4 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 898BAAF47; Thu, 10 Sep 2020 09:14:21 +0000 (UTC) Date: Thu, 10 Sep 2020 11:14:05 +0200 (CEST) From: Richard Biener To: Richard Sandiford cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] fix useless unsharing of SLP tree In-Reply-To: Message-ID: References: User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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: Thu, 10 Sep 2020 09:14:08 -0000 On Thu, 10 Sep 2020, Richard Sandiford wrote: > Richard Biener writes: > > This avoids unsharing the SLP tree when optimizing load permutations > > for reductions but there is no actual permute taking place. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. > > > > 2020-09-09 Richard Biener > > > > * tree-vect-slp.c (vect_attempt_slp_rearrange_stmts): Do > > nothing when the permutation doesn't permute. > > Drive-by comment, sorry for not looking in more detail first, but: > is there a difference in semantics between an identity load_permutation > and a null load_permutation? Seems like it would be good to have > a canonical form. Well, yes - an identity load_permutation will become non-identity if there's a GROUP_GAP and we end up with a VF > 1. We're simply postponing figuring this out until we have VF fixed (and vect_attempt_slp_rearrange_stmts is quite legacy now and at an odd place - sth I hope to address). So with GROUP_GAP == 1 and an identity permute the permute should be NULL. I've run into this during development of other stuff but decided to push this fix since it might as well happen for the SLP reduction case. It's probably a regression from when I pushed permute validity check to vectorizable_load. Richard. > Thanks, > Richard > > > --- > > gcc/tree-vect-slp.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c > > index 03b11058bd5..15d57890b6f 100644 > > --- a/gcc/tree-vect-slp.c > > +++ b/gcc/tree-vect-slp.c > > @@ -1905,11 +1905,14 @@ vect_attempt_slp_rearrange_stmts (slp_instance slp_instn) > > } > > > > /* Check that the loads in the first sequence are different and there > > - are no gaps between them. */ > > + are no gaps between them and that there is an actual permutation. */ > > + bool any_permute = false; > > auto_sbitmap load_index (group_size); > > bitmap_clear (load_index); > > FOR_EACH_VEC_ELT (node->load_permutation, i, lidx) > > { > > + if (lidx != i) > > + any_permute = true; > > if (lidx >= group_size) > > return false; > > if (bitmap_bit_p (load_index, lidx)) > > @@ -1917,6 +1920,8 @@ vect_attempt_slp_rearrange_stmts (slp_instance slp_instn) > > > > bitmap_set_bit (load_index, lidx); > > } > > + if (!any_permute) > > + return false; > > for (i = 0; i < group_size; i++) > > if (!bitmap_bit_p (load_index, i)) > > return false; > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)