Message ID | 06fd34d8-8f90-0b62-8cf5-c58ee67e9ab3@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [testsuite] Fix PR94019 to allow one vector char when !vect_hw_misalign | expand |
Hi! On Wed, Mar 04, 2020 at 03:13:51PM +0800, Kewen.Lin wrote: > As PR94019 shows, without misaligned vector access support but with > realign load, the vectorized loop will end up with realign scheme. > It generates mask (control vector) with return type vector signed > char which breaks the not check. > > The fix is to differentiate powerpc vect_hw_misalign and powerpc > !vect_hw_misalign, permit one vector char occurance for powerpc > !vect_hw_misalign and keep other targets same as before. > > Verified it on ppc64-redhat-linux (Power7 BE). > > Is it ok for trunk, and backport to GCC 9 after some burn-in time? Why is any of this Power-specific? Does it work on other targets if !vect_hw_misalign? The patch is fine everywhere as far as the rs6000 port is concerned, but I'd like someone else who actually understands this stuff to have a look at it as well ;-) Segher
"Kewen.Lin" <linkw@linux.ibm.com> writes: > Hi, > > As PR94019 shows, without misaligned vector access support but with > realign load, the vectorized loop will end up with realign scheme. > It generates mask (control vector) with return type vector signed > char which breaks the not check. > > The fix is to differentiate powerpc vect_hw_misalign and powerpc > !vect_hw_misalign, permit one vector char occurance for powerpc > !vect_hw_misalign and keep other targets same as before. > > Verified it on ppc64-redhat-linux (Power7 BE). > > Is it ok for trunk, and backport to GCC 9 after some burn-in time? > > > BR, > Kewen > -------- > > gcc/testsuite/ChangeLog > > 2020-03-04 Kewen Lin <linkw@gcc.gnu.org> > > PR testsuite/94019 > * gcc.dg/vect/vect-over-widen-17.c: Expect one vector char if it's on > POWER and without misaligned vector access support. > > -------- > > --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c > +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c > @@ -41,6 +41,10 @@ main (void) > } > > /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */ > -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */ > +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } } > +/* On Power, if there is no vect_hw_misalign support, unaligned vector access > + adopts realign_load scheme. It requires rs6000_builtin_mask_for_load to > + generate mask whose return type is vector char. */ > +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */ Thanks for looking at this. The patch is OK as-is. However, since vect-over-widen-17.c is a negative test for generic code, there probably isn't much need for the new scan-tree-dump-times line, and it could start failing if we make different optimisation decisions in future. So the patch is also OK with just the change to the scan-tree-dump-not line, if you prefer that. (Please keep the comment either way though -- it's really helpful.) Richard > /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */ > /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */
Hi Segher, on 2020/3/5 上午2:44, Segher Boessenkool wrote: > Hi! > > On Wed, Mar 04, 2020 at 03:13:51PM +0800, Kewen.Lin wrote: >> As PR94019 shows, without misaligned vector access support but with >> realign load, the vectorized loop will end up with realign scheme. >> It generates mask (control vector) with return type vector signed >> char which breaks the not check. >> >> The fix is to differentiate powerpc vect_hw_misalign and powerpc >> !vect_hw_misalign, permit one vector char occurance for powerpc >> !vect_hw_misalign and keep other targets same as before. >> >> Verified it on ppc64-redhat-linux (Power7 BE). >> >> Is it ok for trunk, and backport to GCC 9 after some burn-in time? > > Why is any of this Power-specific? Does it work on other targets if > !vect_hw_misalign? > Good question, the initial draft was not to take power as specific and just guard it with !vect_hw_misalign. But the root cause is due to builtin_mask_for_load return type, currently rs6000 port is the only user providing that hook, I thought it's better to use "times" to keep the coverage somehow. To avoid any possible new ports with that hook but different return type and then leads to unexpected vector char occurrence times, I further guarded it with powerpc specific. Considering Richard's comments, I think it's fine to remove that particularity now. BR, Kewen
Hi Richard, on 2020/3/5 上午3:09, Richard Sandiford wrote: > "Kewen.Lin" <linkw@linux.ibm.com> writes: >> Hi, >> >> >> --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c >> +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c >> @@ -41,6 +41,10 @@ main (void) >> } >> >> /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */ >> -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */ >> +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } } >> +/* On Power, if there is no vect_hw_misalign support, unaligned vector access >> + adopts realign_load scheme. It requires rs6000_builtin_mask_for_load to >> + generate mask whose return type is vector char. */ >> +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */ > > Thanks for looking at this. The patch is OK as-is. However, since > vect-over-widen-17.c is a negative test for generic code, there probably > isn't much need for the new scan-tree-dump-times line, and it could start > failing if we make different optimisation decisions in future. So the > patch is also OK with just the change to the scan-tree-dump-not line, > if you prefer that. (Please keep the comment either way though -- > it's really helpful.) > Thanks for your suggestion! The new patch is updated as below. I removed the scan-tree-dump-times, as well as powerpc specific requirement. Does it look good to you especially the later? Thanks in advance! BR, Kewen gcc/testsuite/ChangeLog 2020-03-05 Kewen Lin <linkw@gcc.gnu.org> PR testsuite/94019 * gcc.dg/vect/vect-over-widen-17.c: Don't expect vector char if it's without misaligned vector access support. ------ diff --git a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c index 0448260..333d74a 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c @@ -41,6 +41,9 @@ main (void) } /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */ -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */ +/* On Power, if there is no vect_hw_misalign support, unaligned vector access + adopts realign_load scheme. It requires rs6000_builtin_mask_for_load to + generate mask whose return type is vector char. */ +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target vect_hw_misalign } } } */ /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */ /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */
"Kewen.Lin" <linkw@linux.ibm.com> writes: > on 2020/3/5 上午3:09, Richard Sandiford wrote: >> "Kewen.Lin" <linkw@linux.ibm.com> writes: >>> Hi, >>> >>> >>> --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c >>> +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c >>> @@ -41,6 +41,10 @@ main (void) >>> } >>> >>> /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */ >>> -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */ >>> +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } } >>> +/* On Power, if there is no vect_hw_misalign support, unaligned vector access >>> + adopts realign_load scheme. It requires rs6000_builtin_mask_for_load to >>> + generate mask whose return type is vector char. */ >>> +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */ >> >> Thanks for looking at this. The patch is OK as-is. However, since >> vect-over-widen-17.c is a negative test for generic code, there probably >> isn't much need for the new scan-tree-dump-times line, and it could start >> failing if we make different optimisation decisions in future. So the >> patch is also OK with just the change to the scan-tree-dump-not line, >> if you prefer that. (Please keep the comment either way though -- >> it's really helpful.) >> > > Thanks for your suggestion! The new patch is updated as below. I removed > the scan-tree-dump-times, as well as powerpc specific requirement. > Does it look good to you especially the later? Thanks in advance! > > BR, > Kewen > > gcc/testsuite/ChangeLog > > 2020-03-05 Kewen Lin <linkw@gcc.gnu.org> > > PR testsuite/94019 > * gcc.dg/vect/vect-over-widen-17.c: Don't expect vector char if it's > without misaligned vector access support. OK, thanks. Richard > > ------ > > diff --git a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c > index 0448260..333d74a 100644 > --- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c > +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c > @@ -41,6 +41,9 @@ main (void) > } > > /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */ > -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */ > +/* On Power, if there is no vect_hw_misalign support, unaligned vector access > + adopts realign_load scheme. It requires rs6000_builtin_mask_for_load to > + generate mask whose return type is vector char. */ > +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target vect_hw_misalign } } } */ > /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */ > /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */
--- a/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c +++ b/gcc/testsuite/gcc.dg/vect/vect-over-widen-17.c @@ -41,6 +41,10 @@ main (void) } /* { dg-final { scan-tree-dump-not {vect_recog_over_widening_pattern: detected} "vect" } } */ -/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" } } */ +/* { dg-final { scan-tree-dump-not {vector[^\n]*char} "vect" { target { { ! powerpc*-*-* } || { powerpc*-*-* && vect_hw_misalign } } } } } +/* On Power, if there is no vect_hw_misalign support, unaligned vector access + adopts realign_load scheme. It requires rs6000_builtin_mask_for_load to + generate mask whose return type is vector char. */ +/* { dg-final { scan-tree-dump-times {vector[^\n]*char} 1 "vect" { target { powerpc*-*-* && { ! vect_hw_misalign } } } } } */ /* { dg-final { scan-tree-dump-not {vector[^ ]* int} "vect" } } */ /* { dg-final { scan-tree-dump-times "vectorized 1 loop" 1 "vect" } } */