| Message ID | 8d249ad0-2df9-7939-c216-5e2f1d8d4035@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 17, 2016 at 4:14 AM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > On 11/16/16 9:08 AM, Richard Biener wrote: > >> On Tue, Nov 15, 2016 at 9:03 PM, Bill Schmidt >> <wschmidt@linux.vnet.ibm.com> wrote: >>> - if ((any_pred_load_store || any_complicated_phi) >>> - && !version_loop_for_if_conversion (loop)) >>> + /* Since we have no cost model, always version loops if vectorization >>> + is enabled. Either version this loop, or if the pattern is right >>> + for outer-loop vectorization, version the outer loop. In the >>> + latter case we will still if-convert the original inner loop. */ >>> + /* FIXME: When SLP vectorization can handle if-conversion on its own, >>> + predicate all of if-conversion on flag_tree_loop_vectorize. */ >>> + if ((any_pred_load_store || any_complicated_phi || flag_tree_loop_vectorize) >> I'd say given fun->has_force_vectorize_loops this should be >> >> if (flag_tree_loop_if_convert != 1 >>> + && !version_loop_for_if_conversion >>> + (versionable_outer_loop_p (loop_outer (loop)) >>> + ? loop_outer (loop) : loop)) >>> goto cleanup; >> and thus always version if the user didnt' specify -ftree-loop-if-convert >> (-ftree-loop-if-convert-stores is dead, I forgot to remove uses). >> >> Can you as a first patch (after fixing the minor things above) commit >> the patch w/o changing the condition under which we version >> (but _do_ version the outer loop if possible?). This should be a strict >> improvement enabling more outer loop vectorization. > > Done and committed. Thanks! > >> >> The 2nd patch can then fix the PR and change the condition. >> >> Thus, ok with the nits fixed and the condition unchanged. >> >> Can you re-test the 2nd part with my suggested changed predicate? > > Yes, the new predicate works fine. New patch below, bootstrapped and tested > on powerpc64le-unknown-linux-gnu, with only the bb-slp-cond-1.c regressions > previously discussed. Is this ok for trunk? Yes. Please open a bug for the bb-slp-cond-1.c testsuite FAIL (so we have an excuse to either improve loop vectorization data dependence analysis or BB vectorization in stage3). Thanks, Richard. > Thanks, > Bill > >> >> Thanks, >> Richard. >> > [gcc] > > 2016-11-16 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > Richard Biener <rguenther@suse.de> > > PR tree-optimization/77848 > * tree-if-conv.c (tree_if_conversion): Always version loops unless > the user specified -ftree-loop-if-convert. > > [gcc/testsuite] > > 2016-11-16 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > Richard Biener <rguenther@suse.de> > > PR tree-optimization/77848 > * gfortran.dg/vect/pr77848.f: New test. > > > Index: gcc/testsuite/gfortran.dg/vect/pr77848.f > =================================================================== > --- gcc/testsuite/gfortran.dg/vect/pr77848.f (revision 0) > +++ gcc/testsuite/gfortran.dg/vect/pr77848.f (working copy) > @@ -0,0 +1,24 @@ > +! PR 77848: Verify versioning is on when vectorization fails > +! { dg-do compile } > +! { dg-options "-O3 -ffast-math -fdump-tree-ifcvt -fdump-tree-vect-details" } > + > + subroutine sub(x,a,n,m) > + implicit none > + real*8 x(*),a(*),atemp > + integer i,j,k,m,n > + real*8 s,t,u,v > + do j=1,m > + atemp=0.d0 > + do i=1,n > + if (abs(a(i)).gt.atemp) then > + atemp=a(i) > + k = i > + end if > + enddo > + call dummy(atemp,k) > + enddo > + return > + end > + > +! { dg-final { scan-tree-dump "LOOP_VECTORIZED" "ifcvt" } } > +! { dg-final { scan-tree-dump "vectorized 0 loops in function" "vect" } } > Index: gcc/tree-if-conv.c > =================================================================== > --- gcc/tree-if-conv.c (revision 242521) > +++ gcc/tree-if-conv.c (working copy) > @@ -2803,10 +2803,12 @@ tree_if_conversion (struct loop *loop) > || loop->dont_vectorize)) > goto cleanup; > > - /* Either version this loop, or if the pattern is right for outer-loop > - vectorization, version the outer loop. In the latter case we will > - still if-convert the original inner loop. */ > - if ((any_pred_load_store || any_complicated_phi) > + /* Since we have no cost model, always version loops unless the user > + specified -ftree-loop-if-convert. Either version this loop, or if > + the pattern is right for outer-loop vectorization, version the > + outer loop. In the latter case we will still if-convert the > + original inner loop. */ > + if (flag_tree_loop_if_convert != 1 > && !version_loop_for_if_conversion > (versionable_outer_loop_p (loop_outer (loop)) > ? loop_outer (loop) : loop)) >
> On Nov 17, 2016, at 3:42 AM, Richard Biener <richard.guenther@gmail.com> wrote: > > On Thu, Nov 17, 2016 at 4:14 AM, Bill Schmidt > <wschmidt@linux.vnet.ibm.com> wrote: >> >> Yes, the new predicate works fine. New patch below, bootstrapped and tested >> on powerpc64le-unknown-linux-gnu, with only the bb-slp-cond-1.c regressions >> previously discussed. Is this ok for trunk? > > Yes. Please open a bug for the bb-slp-cond-1.c testsuite FAIL (so we have an > excuse to either improve loop vectorization data dependence analysis or > BB vectorization in stage3). Fix committed as r242550, new bug open at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78396. Thanks! Bill > > Thanks, > Richard.
Index: gcc/testsuite/gfortran.dg/vect/pr77848.f =================================================================== --- gcc/testsuite/gfortran.dg/vect/pr77848.f (revision 0) +++ gcc/testsuite/gfortran.dg/vect/pr77848.f (working copy) @@ -0,0 +1,24 @@ +! PR 77848: Verify versioning is on when vectorization fails +! { dg-do compile } +! { dg-options "-O3 -ffast-math -fdump-tree-ifcvt -fdump-tree-vect-details" } + + subroutine sub(x,a,n,m) + implicit none + real*8 x(*),a(*),atemp + integer i,j,k,m,n + real*8 s,t,u,v + do j=1,m + atemp=0.d0 + do i=1,n + if (abs(a(i)).gt.atemp) then + atemp=a(i) + k = i + end if + enddo + call dummy(atemp,k) + enddo + return + end + +! { dg-final { scan-tree-dump "LOOP_VECTORIZED" "ifcvt" } } +! { dg-final { scan-tree-dump "vectorized 0 loops in function" "vect" } } Index: gcc/tree-if-conv.c =================================================================== --- gcc/tree-if-conv.c (revision 242521) +++ gcc/tree-if-conv.c (working copy) @@ -2803,10 +2803,12 @@ tree_if_conversion (struct loop *loop) || loop->dont_vectorize)) goto cleanup; - /* Either version this loop, or if the pattern is right for outer-loop - vectorization, version the outer loop. In the latter case we will - still if-convert the original inner loop. */ - if ((any_pred_load_store || any_complicated_phi) + /* Since we have no cost model, always version loops unless the user + specified -ftree-loop-if-convert. Either version this loop, or if + the pattern is right for outer-loop vectorization, version the + outer loop. In the latter case we will still if-convert the + original inner loop. */ + if (flag_tree_loop_if_convert != 1 && !version_loop_for_if_conversion (versionable_outer_loop_p (loop_outer (loop)) ? loop_outer (loop) : loop))