From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 205E93857C5B for ; Wed, 1 Sep 2021 10:07:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 205E93857C5B Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 08D042251A; Wed, 1 Sep 2021 10:07:22 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id 02C12A3B95; Wed, 1 Sep 2021 10:07:21 +0000 (UTC) Date: Wed, 1 Sep 2021 12:07:21 +0200 (CEST) From: Richard Biener To: Richard Sandiford cc: Richard Biener , GCC Patches Subject: Re: [PATCH] tree-optimization/102139 - fix SLP DR base alignment 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=-4.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Wed, 01 Sep 2021 10:07:33 -0000 On Wed, 1 Sep 2021, Richard Sandiford wrote: > Richard Biener writes: > > On Tue, Aug 31, 2021 at 11:26 AM Richard Biener via Gcc-patches > > wrote: > >> > >> When doing whole-function SLP we have to make sure the recorded > >> base alignments we compute as the maximum alignment seen for a > >> base anywhere in the function is actually valid at the point > >> we want to make use of it. > > Ah, yeah, the danger of optimisations that silently rely on the > then-current restrictions :-( Yeah. > >> To make this work we now record the stmt the alignment was derived > >> from in addition to the DRs innermost behavior and we use a > >> dominance check to verify the recorded info is valid when doing > >> BB vectorization. > >> > >> Note this leaves a small(?) hole for the case where we have sth > >> like > >> > >> unaligned DR > >> call (); // does not return > >> aligned DR > >> > >> since we'll derive an aligned access for the earlier DR but the > >> later DR is never actually reached since the call does not > >> return. To plug this hole one option (for the easy backporting) > >> would be to simply not use the base-alignment recording at all. > >> Alternatively we'd have to store the dataref grouping 'id' somewhere > >> in the DR itself and use that to handle this particular case. > > > > It turns out this isn't too difficult so the following is a patch adjusted > > to cover that case together with a testcase. > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > OK? > > TBH I know nothing about this group id scheme, so I'm not really > in a position to comment. But it LGTM from the (few) bits I do understand. > > I guess we're leaving the same easter egg for loop optimisation if > we ever support early exits, but I'm not sure what to do about that. We're currently not recording alignment from masked DRs (aka DR_IS_CONDITIONAL_IN_STMT), I suppose we'd need to mark all stmts after early exits in this way then since in the end they will have to be masked on the early exit. So we _might_ be fine there ... Pushed. Thanks, Richard. > Thanks, > Richard > > > > > Thanks, > > Richard. > > > > 2021-08-31 Richard Biener > > > > PR tree-optimization/102139 > > * tree-vectorizer.h (vec_base_alignments): Adjust hash-map > > type to record a std::pair of the stmt-info and the innermost > > loop behavior. > > (dr_vec_info::group): New member. > > * tree-vect-data-refs.c (vect_record_base_alignment): Adjust. > > (vect_compute_data_ref_alignment): Verify the recorded > > base alignment can be used. > > (data_ref_pair): Remove. > > (dr_group_sort_cmp): Adjust. > > (vect_analyze_data_ref_accesses): Store the group-ID in the > > dr_vec_info and operate on a vector of dr_vec_infos. > > > > * gcc.dg/torture/pr102139.c: New testcase. > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)