| Message ID | 908e586f-650e-ecae-af55-6b75b2cc782a@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 26, 2017 at 03:14:47PM -0600, Bill Schmidt wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65484 identifies an > issue whether g++.dg/vect/pr36648.cc fails on older POWER hardware. > This is due to a decision made in November 2010 to include the > flag -mno-allow-movmisalign in check_vect_support_and_set_flags, > which governs the vectorizer tests in that directory. This flag > sometimes inhibits vectorization when to vectorize the code > requires that misaligned loads and stores be used. This flag is > not added to the command line for POWER8 hardware and later. > > pr36648.cc is an example of the kind of vectorization that > requires misaligned memory accesses, so it is vectorized on > POWER8 and later hardware, but not on POWER7 or earlier with > the default testsuite flags. This patch modifies the dg-final > checks in pr36648.cc to be consistent with this behavior. I've > added commentary to explain what might otherwise seem to be a > somewhat arcane choice of tests. > > Tested on trunk and GCC 6 for POWER8 LE and POWER7 BE systems. > Is this ok for trunk? > 2017-01-26 Bill Schmidt <wschmidt@linux.vnet.ibm.com> > > PR target/65484 > * g++.dg/vect/pr36648.cc: Modify to reflect that the loop is not > vectorized on POWER unless hardware misaligned loads are > available. > --- gcc/testsuite/g++.dg/vect/pr36648.cc (revision 244811) > +++ gcc/testsuite/g++.dg/vect/pr36648.cc (working copy) > @@ -19,7 +19,12 @@ Foo foo; > > int main() { } > > -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { ! vect_no_align } } } } */ > -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { ! vect_no_align } } } } */ > +/* On older powerpc hardware (POWER7 and earlier), the default flag > + -mno-allow-movmisalign prevents vectorization. On POWER8 and later, > + when vect_hw_misalign is true, vectorization occurs. For other > + targets, ! vect_no_align is a sufficient test. */ > > +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ > +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ > What does this do if no_align and powerpc and vect_hw_misalign? Or can that not happen? Segher
> On Jan 26, 2017, at 4:29 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > > On Thu, Jan 26, 2017 at 03:14:47PM -0600, Bill Schmidt wrote: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65484 identifies an >> issue whether g++.dg/vect/pr36648.cc fails on older POWER hardware. >> This is due to a decision made in November 2010 to include the >> flag -mno-allow-movmisalign in check_vect_support_and_set_flags, >> which governs the vectorizer tests in that directory. This flag >> sometimes inhibits vectorization when to vectorize the code >> requires that misaligned loads and stores be used. This flag is >> not added to the command line for POWER8 hardware and later. >> >> pr36648.cc is an example of the kind of vectorization that >> requires misaligned memory accesses, so it is vectorized on >> POWER8 and later hardware, but not on POWER7 or earlier with >> the default testsuite flags. This patch modifies the dg-final >> checks in pr36648.cc to be consistent with this behavior. I've >> added commentary to explain what might otherwise seem to be a >> somewhat arcane choice of tests. >> >> Tested on trunk and GCC 6 for POWER8 LE and POWER7 BE systems. >> Is this ok for trunk? > >> 2017-01-26 Bill Schmidt <wschmidt@linux.vnet.ibm.com> >> >> PR target/65484 >> * g++.dg/vect/pr36648.cc: Modify to reflect that the loop is not >> vectorized on POWER unless hardware misaligned loads are >> available. > >> --- gcc/testsuite/g++.dg/vect/pr36648.cc (revision 244811) >> +++ gcc/testsuite/g++.dg/vect/pr36648.cc (working copy) >> @@ -19,7 +19,12 @@ Foo foo; >> >> int main() { } >> >> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { ! vect_no_align } } } } */ >> -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { ! vect_no_align } } } } */ >> +/* On older powerpc hardware (POWER7 and earlier), the default flag >> + -mno-allow-movmisalign prevents vectorization. On POWER8 and later, >> + when vect_hw_misalign is true, vectorization occurs. For other >> + targets, ! vect_no_align is a sufficient test. */ >> >> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ >> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ >> > > What does this do if no_align and powerpc and vect_hw_misalign? Or can that > not happen? > That's the usual case. Both vect_no_align and vect_hw_misalign are 1 for POWER8 or later, and 0 otherwise. If you're asking what happens with !vect_no_align && powerpc*-*-* && vect_hw_misalign, the answer is that it can't happen. I used vect_hw_misalign here because it better documents what's going on. The first clause maintains the existing condition for non-powerpc targets, while the second expects vectorization for POWER8 and later, but not for earlier hardware. Bill > > Segher >
On Thu, Jan 26, 2017 at 04:36:31PM -0600, Bill Schmidt wrote: > >> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { ! vect_no_align } } } } */ > >> -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { ! vect_no_align } } } } */ > >> +/* On older powerpc hardware (POWER7 and earlier), the default flag > >> + -mno-allow-movmisalign prevents vectorization. On POWER8 and later, > >> + when vect_hw_misalign is true, vectorization occurs. For other > >> + targets, ! vect_no_align is a sufficient test. */ > >> > >> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ > >> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ > >> > > > > What does this do if no_align and powerpc and vect_hw_misalign? Or can that > > not happen? > > > That's the usual case. Both vect_no_align and vect_hw_misalign are 1 for > POWER8 or later, and 0 otherwise. So for older targets it used to run the final, but not after the patch; and for newer targets it used to not run it, but it does after the patch? So it is meant to be two changes? Segher
> On Jan 26, 2017, at 5:15 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > > On Thu, Jan 26, 2017 at 04:36:31PM -0600, Bill Schmidt wrote: >>>> -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { ! vect_no_align } } } } */ >>>> -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { ! vect_no_align } } } } */ >>>> +/* On older powerpc hardware (POWER7 and earlier), the default flag >>>> + -mno-allow-movmisalign prevents vectorization. On POWER8 and later, >>>> + when vect_hw_misalign is true, vectorization occurs. For other >>>> + targets, ! vect_no_align is a sufficient test. */ >>>> >>>> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ >>>> +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ >>>> >>> >>> What does this do if no_align and powerpc and vect_hw_misalign? Or can that >>> not happen? >>> >> That's the usual case. Both vect_no_align and vect_hw_misalign are 1 for >> POWER8 or later, and 0 otherwise. > > So for older targets it used to run the final, but not after the patch; and > for newer targets it used to not run it, but it does after the patch? So > it is meant to be two changes? Correct, I forgot to point out that for newer hardware we are vectorizing the loop, but the way the test was written before we weren't testing it. Sorry for the confusion! Bill > > > Segher >
On Thu, Jan 26, 2017 at 05:31:16PM -0600, Bill Schmidt wrote: > > So for older targets it used to run the final, but not after the patch; and > > for newer targets it used to not run it, but it does after the patch? So > > it is meant to be two changes? > > Correct, I forgot to point out that for newer hardware we are vectorizing > the loop, but the way the test was written before we weren't testing it. > Sorry for the confusion! For the record: approved for trunk. Thanks! Segher
> On Jan 26, 2017, at 6:40 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote: > > On Thu, Jan 26, 2017 at 05:31:16PM -0600, Bill Schmidt wrote: >>> So for older targets it used to run the final, but not after the patch; and >>> for newer targets it used to not run it, but it does after the patch? So >>> it is meant to be two changes? >> >> Correct, I forgot to point out that for newer hardware we are vectorizing >> the loop, but the way the test was written before we weren't testing it. >> Sorry for the confusion! > > For the record: approved for trunk. Thanks! > Would it be all right to back port this to GCC 5 and 6 after some burn-in? I see now that the bug was reported against GCC 5. Bill > > Segher >
On Fri, Jan 27, 2017 at 10:02:41AM -0600, Bill Schmidt wrote: > > For the record: approved for trunk. Thanks! > > > Would it be all right to back port this to GCC 5 and 6 after some burn-in? > I see now that the bug was reported against GCC 5. Sure. It only fixes a testcase so it won't really help much, but it is totally safe for the same reason ;-) Segher
Index: gcc/testsuite/g++.dg/vect/pr36648.cc =================================================================== --- gcc/testsuite/g++.dg/vect/pr36648.cc (revision 244811) +++ gcc/testsuite/g++.dg/vect/pr36648.cc (working copy) @@ -19,7 +19,12 @@ Foo foo; int main() { } -/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { ! vect_no_align } } } } */ -/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { ! vect_no_align } } } } */ +/* On older powerpc hardware (POWER7 and earlier), the default flag + -mno-allow-movmisalign prevents vectorization. On POWER8 and later, + when vect_hw_misalign is true, vectorization occurs. For other + targets, ! vect_no_align is a sufficient test. */ +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ +/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 1 "vect" { target { { { ! vect_no_align } && { ! powerpc*-*-* } } || { powerpc*-*-* && vect_hw_misalign } } } } } */ +