From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1337 invoked by alias); 7 May 2015 16:07:30 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 1326 invoked by uid 89); 7 May 2015 16:07:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx2.suse.de Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Thu, 07 May 2015 16:07:22 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 776E6ACC2; Thu, 7 May 2015 16:07:19 +0000 (UTC) Date: Thu, 07 May 2015 16:07:00 -0000 From: Michael Matz To: Alan Lawrence cc: "gcc-patches@gcc.gnu.org" Subject: Re: Vectorize stores with unknown stride In-Reply-To: <554B77D7.40505@arm.com> Message-ID: References: <554B77D7.40505@arm.com> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-IsSubscribed: yes X-SW-Source: 2015-05/txt/msg00566.txt.bz2 On Thu, 7 May 2015, Alan Lawrence wrote: > Also update comment? (5 identical cases) > > Also update comment? Obviously a good idea, thanks :) (s/loads/accesses/ for the commit) > > @@ -5013,7 +5025,7 @@ vectorizable_store (gimple stmt, gimple_stmt_iterator > > *gsi, gimple *vec_stmt, > > tree dataref_ptr = NULL_TREE; > > tree dataref_offset = NULL_TREE; > > gimple ptr_incr = NULL; > > - int nunits = TYPE_VECTOR_SUBPARTS (vectype); > > + unsigned int nunits = TYPE_VECTOR_SUBPARTS (vectype); > > Unrelated? (though I don't object to it!) No, the patch introduces for a first time in this function a loop over unsigned i with nunits as bound, so the compare would warn if I weren't to change either the type of i (with a cascade of more such changes) or nuinits (with only this one place). The whole file could use an overhaul of signedness (after all nunits in all functions will always be non-negative), but as a separate patch. > > + if (STMT_VINFO_STRIDED_P (stmt_info)) > > + ; > > + else > > This is not a long chain of ifs, so is there a reason not to have > if (!STMT_VINFO_STRIDED_P (stmt_info)) > ? Err, right. Probably I had some code in there initially; as now it looks a bit mannered :) > > -#define STMT_VINFO_STRIDE_LOAD_P(S) (S)->stride_load_p > > +#define STMT_VINFO_STRIDED_P(S) (S)->strided_p > > Spacing (?) No, it's using correct tabs, indentation by the diff '+' prefix deceives us. (Your mailer must be playing games, I've sent it out with TABs intact). > > + float src[] = {1, 2, 3, 4, 5, 6, 7, 8}; > > + float dest[8]; > > + check_vect (); > > + sumit (dest, src, src, 1, 8); > > + for (i = 0; i < 8; i++) > > + if (2*src[i] != dest[i]) > > + abort (); > > + return 0; > > +} > > + > > +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */ > > +/* { dg-final { cleanup-tree-dump "vect" } } */ > > While I appreciate the vectorizer doesn't know the invariant stride is going > to be '1' when the loop executes...IMVHO I'd feel more reassured if we passed > in stride>1 at runtime ;). In fact I'd feel more reassured still, if we did > the thing with a few different values of stride... Sensible. I'll use the test case below (hey! even stride zero! :) ) Regstrapping again with the changes as per above. Committing tomorrow. Many thanks for the review. Ciao, Michael. /* { dg-require-effective-target vect_float } */ #include #include "tree-vect.h" void __attribute__((noinline)) sumit (float * __restrict dest, float * __restrict src, float * __restrict src2, int stride, int n) { int i; for (i = 0; i < n; i++) dest[i*stride] = src[i] + src2[i]; } int main() { int i, stride; float src[] = {1, 2, 3, 4, 5, 6, 7, 8}; float dest[64]; check_vect (); for (stride = 0; stride < 8; stride++) { sumit (dest, src, src, stride, 8); if (!stride && dest[0] != 16) abort(); else if (stride) for (i = 0; i < 8; i++) if (2*src[i] != dest[i*stride]) abort (); } return 0; } /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" } } */ /* { dg-final { cleanup-tree-dump "vect" } } */