| Message ID | 1501266127.28549.118.camel@cavium.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jul 28, 2017 at 8:22 PM, Steve Ellcey <sellcey@cavium.com> wrote: > On Fri, 2017-07-28 at 09:47 +0200, Richard Biener wrote: >> On Fri, Jul 28, 2017 at 12:16 AM, Steve Ellcey <sellcey@cavium.com> wrote: >> > >> > Any comments from the power and/or vectorizer folks? >> On one side I'm inclined to simplify the testsuite by adding >> --param vect-max-peeling-for-alignment=0 in addition to >> -fno-vect-cost-model we already pass and override that in the >> tests that specifically exercise peeling for alignment (do we have >> any?). >> OTOH that would remove quite some testing coverage of prologue >> peeling. >> >> So ideally testresults would be clean with both no such --param >> and that --param added... >> >> I think most of the testcases you needed to adjust have nothing >> to do with peeling for alignment thus adding this --param just for >> those (and simplifying their dump scanning accordingly) is another >> pragmatic option. >> >> Adding yet another target (vect_peel_align) is IMHO not good, >> especially as this one depends on cost tuning and not HW >> features, so it's impossible(?) to dynamically compute it >> with a test compile for example (we _do_ want a clean >> vect.exp with any vector HW / tuning switch you add). > > How about something like the following. I only fixed two of the tests, > I can follow up with more if this approach seems reasonable. I tested > this on aarch64 and x86_64. Looks good to me. Richard. > Steve Ellcey > sellcey@cavium.com > > > 2017-07-28 Steve Ellcey <sellcey@cavium.com> > > PR tree-optimization/80925 > * gcc.dg/vect/no-section-anchors-vect-69.c: Add > --param vect-max-peeling-for-alignment=0 option. > Remove unaligned access and peeling checks. > * gcc.dg/vect/section-anchors-vect-69.c: Ditto.
Richard Biener <richard.guenther@gmail.com> writes: > On Fri, Jul 28, 2017 at 8:22 PM, Steve Ellcey <sellcey@cavium.com> wrote: >> On Fri, 2017-07-28 at 09:47 +0200, Richard Biener wrote: >>> On Fri, Jul 28, 2017 at 12:16 AM, Steve Ellcey <sellcey@cavium.com> wrote: >>> > >>> > Any comments from the power and/or vectorizer folks? >>> On one side I'm inclined to simplify the testsuite by adding >>> --param vect-max-peeling-for-alignment=0 in addition to >>> -fno-vect-cost-model we already pass and override that in the >>> tests that specifically exercise peeling for alignment (do we have >>> any?). >>> OTOH that would remove quite some testing coverage of prologue >>> peeling. >>> >>> So ideally testresults would be clean with both no such --param >>> and that --param added... >>> >>> I think most of the testcases you needed to adjust have nothing >>> to do with peeling for alignment thus adding this --param just for >>> those (and simplifying their dump scanning accordingly) is another >>> pragmatic option. >>> >>> Adding yet another target (vect_peel_align) is IMHO not good, >>> especially as this one depends on cost tuning and not HW >>> features, so it's impossible(?) to dynamically compute it >>> with a test compile for example (we _do_ want a clean >>> vect.exp with any vector HW / tuning switch you add). >> >> How about something like the following. I only fixed two of the tests, >> I can follow up with more if this approach seems reasonable. I tested >> this on aarch64 and x86_64. > > Looks good to me. > > Richard. > >> Steve Ellcey >> sellcey@cavium.com >> >> >> 2017-07-28 Steve Ellcey <sellcey@cavium.com> >> >> PR tree-optimization/80925 >> * gcc.dg/vect/no-section-anchors-vect-69.c: Add >> --param vect-max-peeling-for-alignment=0 option. >> Remove unaligned access and peeling checks. >> * gcc.dg/vect/section-anchors-vect-69.c: Ditto. This broke the test on Solaris/SPARC: +FAIL: gcc.dg/vect/no-section-anchors-vect-69.c scan-tree-dump-times vect "vectorized 4 loops" 1 both 32 and 64-bit. The dump now has no-section-anchors-vect-69.c:39:5: note: vectorized 3 loops in function. and (compared to the gcc-7 branch) no-section-anchors-vect-69.c:44:3: note: not vectorized: unsupported unaligned store.tmp1[2].a.n[1][2][i_74] Rainer
I am not sure why this is failing on Solaris/SPARC, do you have the vector dump file from the test so we can see what it says about the loop it stopped vectorizing? I don't have a Solaris/SPARC system here, I tried to build an 'initial' gcc for sparc-sun-solaris2.11 but I could not reproduce the error using that. I am not sure why. Steve Ellcey sellcey@cavium.com On Fri, 2017-08-04 at 16:27 +0200, Rainer Orth wrote: > Richard Biener <richard.guenther@gmail.com> writes: > > > > > On Fri, Jul 28, 2017 at 8:22 PM, Steve Ellcey <sellcey@cavium.com> > > wrote: > > > > > > On Fri, 2017-07-28 at 09:47 +0200, Richard Biener wrote: > > > > > > > > On Fri, Jul 28, 2017 at 12:16 AM, Steve Ellcey <sellcey@cavium. > > > > com> wrote: > > > > > > > > > > > > > > > Any comments from the power and/or vectorizer folks? > > > > On one side I'm inclined to simplify the testsuite by adding > > > > --param vect-max-peeling-for-alignment=0 in addition to > > > > -fno-vect-cost-model we already pass and override that in the > > > > tests that specifically exercise peeling for alignment (do we > > > > have > > > > any?). > > > > OTOH that would remove quite some testing coverage of prologue > > > > peeling. > > > > > > > > So ideally testresults would be clean with both no such --param > > > > and that --param added... > > > > > > > > I think most of the testcases you needed to adjust have nothing > > > > to do with peeling for alignment thus adding this --param just > > > > for > > > > those (and simplifying their dump scanning accordingly) is > > > > another > > > > pragmatic option. > > > > > > > > Adding yet another target (vect_peel_align) is IMHO not good, > > > > especially as this one depends on cost tuning and not HW > > > > features, so it's impossible(?) to dynamically compute it > > > > with a test compile for example (we _do_ want a clean > > > > vect.exp with any vector HW / tuning switch you add). > > > How about something like the following. I only fixed two of the > > > tests, > > > I can follow up with more if this approach seems reasonable. I > > > tested > > > this on aarch64 and x86_64. > > Looks good to me. > > > > Richard. > > > > > > > > Steve Ellcey > > > sellcey@cavium.com > > > > > > > > > 2017-07-28 Steve Ellcey <sellcey@cavium.com> > > > > > > PR tree-optimization/80925 > > > * gcc.dg/vect/no-section-anchors-vect-69.c: Add > > > --param vect-max-peeling-for-alignment=0 option. > > > Remove unaligned access and peeling checks. > > > * gcc.dg/vect/section-anchors-vect-69.c: Ditto. > This broke the test on Solaris/SPARC: > > +FAIL: gcc.dg/vect/no-section-anchors-vect-69.c scan-tree-dump-times > vect "vectorized 4 loops" 1 > > both 32 and 64-bit. The dump now has > > no-section-anchors-vect-69.c:39:5: note: vectorized 3 loops in > function. > > and (compared to the gcc-7 branch) > > no-section-anchors-vect-69.c:44:3: note: not vectorized: unsupported > unaligned store.tmp1[2].a.n[1][2][i_74] > > Rainer >
diff --git a/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-69.c b/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-69.c index fe968de..1458ba6 100644 --- a/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-69.c +++ b/gcc/testsuite/gcc.dg/vect/no-section-anchors-vect-69.c @@ -1,5 +1,6 @@ /* { dg-require-effective-target vect_int } */ /* { dg-add-options bind_pic_locally } */ +/* { dg-additional-options "--param vect-max-peeling-for-alignment=0" } */ #include <stdarg.h> #include "tree-vect.h" @@ -114,7 +115,4 @@ int main (void) } /* { dg-final { scan-tree-dump-times "vectorized 4 loops" 1 "vect" } } */ -/* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */ -/* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { xfail { {! vector_alignment_reachable} || { vect_sizes_32B_16B} } } } } */ /* { dg-final { scan-tree-dump-times "Alignment of access forced using versioning" 1 "vect" { target { {! vector_alignment_reachable} && {! vect_hw_misalign} } } } } */ -/* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 1 "vect" { target { {! vector_alignment_reachable} && {! vect_hw_misalign} } } } } */ diff --git a/gcc/testsuite/gcc.dg/vect/section-anchors-vect-69.c b/gcc/testsuite/gcc.dg/vect/section-anchors-vect-69.c index 8c88e5f..26bcf4b0 100644 --- a/gcc/testsuite/gcc.dg/vect/section-anchors-vect-69.c +++ b/gcc/testsuite/gcc.dg/vect/section-anchors-vect-69.c @@ -1,4 +1,5 @@ /* { dg-require-effective-target section_anchors } */ +/* { dg-additional-options "--param vect-max-peeling-for-alignment=0" } */ #include <stdarg.h> #include "tree-vect.h" @@ -112,8 +113,6 @@ int main (void) } /* { dg-final { scan-tree-dump-times "vectorized 4 loops" 1 "vect" { target vect_int } } } */ -/* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */ /* Alignment forced using versioning until the pass that increases alignment is extended to handle structs. */ -/* { dg-final { scan-tree-dump-times "Alignment of access forced using peeling" 2 "vect" { target {vect_int && vector_alignment_reachable } } } } */ /* { dg-final { scan-tree-dump-times "Alignment of access forced using versioning" 4 "vect" { target {vect_int && {! vector_alignment_reachable} } } } } */