Re: [PATCH, rs6000, testsuite, PR65456] Changes for unaligned vector load/store support on POWER8

[Vidya Praveen <vidyapraveen at arm dot com>]

On Mon, Jun 15, 2015 at 08:14:31PM +0100, Bill Schmidt wrote:
> On Fri, 2015-06-12 at 17:36 +0100, Vidya Praveen wrote:
> > On Thu, Apr 30, 2015 at 01:34:18PM +0100, Bill Schmidt wrote:
> > > On Thu, 2015-04-30 at 18:26 +0800, Bin.Cheng wrote:
> > > > On Mon, Apr 27, 2015 at 9:26 PM, Bill Schmidt
> > > > <wschmidt@linux.vnet.ibm.com> wrote:
> > > > > On Mon, 2015-04-27 at 14:23 +0800, Bin.Cheng wrote:
> > > > >> On Mon, Mar 30, 2015 at 1:42 AM, Bill Schmidt
> > > > >> <wschmidt@linux.vnet.ibm.com> wrote:
> > > > >
> > > > >>
> > > > >> > Index: gcc/testsuite/gcc.dg/vect/vect-33.c
> > > > >> > ===================================================================
> > > > >> > --- gcc/testsuite/gcc.dg/vect/vect-33.c (revision 221118)
> > > > >> > +++ gcc/testsuite/gcc.dg/vect/vect-33.c (working copy)
> > > > >> > @@ -36,9 +36,10 @@ int main (void)
> > > > >> >    return main1 ();
> > > > >> >  }
> > > > >> >
> > > > >> > +/* vect_hw_misalign && { ! vect64 } */
> > > > >> >
> > > > >> >  /* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect"  } } */
> > > > >> > -/* { dg-final { scan-tree-dump "Vectorizing an unaligned access" "vect" { target { vect_hw_misalign && { {! vect64} || vect_multiple_sizes } } } } } */
> > > > >> > +/* { dg-final { scan-tree-dump "Vectorizing an unaligned access" "vect" { target { { { ! powerpc*-*-* } && vect_hw_misalign } && { { ! vect64 } || vect_multiple_sizes } } } } }  */
> > > > >> >  /* { dg-final { scan-tree-dump "Alignment of access forced using peeling" "vect" { target { vector_alignment_reachable && { vect64 && {! vect_multiple_sizes} } } } } } */
> > > > >> >  /* { dg-final { scan-tree-dump-times "Alignment of access forced using versioning" 1 "vect" { target { { {! vector_alignment_reachable} || {! vect64} } && {! vect_hw_misalign} } } } } */
> > > > >> >  /* { dg-final { cleanup-tree-dump "vect" } } */
> > > > >>
> > > > >> Hi Bill,
> > > > >> With this change, the test case is skipped on aarch64 now.  Since it
> > > > >> passed before, Is it expected to act like this on 64bit platforms?
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > No, that's a mistake on my part -- thanks for the report!  That first
> > > > > added line was not intended to be part of the patch:
> > > > >
> > > > > +/* vect_hw_misalign && { ! vect64 } */
> > > > >
> > > > > Please try removing that line and verify that the patch succeeds again
> > > > > for ARM.  Assuming so, I'll prepare a patch to fix this.
> > > > >
> > > > > It looks like this mistake was introduced only in this particular test,
> > > > > but please let me know if you see any other anomalies.
> > > > Hi Bill,
> > > > I chased the wrong branch.  The test disappeared on fsf-48 branch in
> > > > out build, rather than trunk.  I guess it's not your patch's fault.
> > > > Will follow up and get back to you later.
> > > > Sorry for the inconvenience.
> > > 
> > > OK, thanks for letting me know!  There was still a bad line in this
> > > patch, although it was only introduced in 5.1 and trunk, so I guess that
> > > wasn't responsible in this case.  Thanks for checking!
> > 
> > 
> > Hi Bill,
> > 
> > In 4.8 branch, you have changed:
> > 
> > -/* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" } } */
> > +/* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" { target { ! vect_hw_misalign } } } } */
> > 
> > Whereas your comment says:
> > 
> >    2015-04-24  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >     
> >         Backport from mainline r222349
> >         2015-04-22  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> >     
> >         PR target/65456
> > [...]
> >         * gcc.dg/vect/vect-33.c: Exclude unaligned access test for
> >         POWER8.
> > [...]
> > 
> > There wasn't an unaligned access test in the first place. But if you wanted to
> > introduce it and exclude it for POWER8 then it should've been:
> > 
> >  ...   { { ! powerpc*-*-* } && vect_hw_misalign } ...
> > 
> > like you have done for the trunk. At the moment, this change has made the test
> > to be skipped for AArch64. It should've been skipped for x86_64-*-* and i*86-*-*
> > as well.
> > 
> > I believe it wasn't intended to be skipped so?
> 
> Right, wasn't intended to be skipped.  This test changed substantially
> between 4.8 and 4.9, so when I did the backport I tried (and failed) to
> adjust it properly.
> 
> Because the sense of the test has been reversed, I believe the correct
> change is
> 
> /* { dg-final { scan-tree-dump-times "Vectorizing an unaligned access" 0 "vect" { target { { ! powerpc*-*-* } || { ! vect_hw_misalign } } } } } */

Makes sense. If I understand it right, it shouldn't vectorize for targets
(except powerpc) that support vector misalign access?

Regards
VP.






> 
> I'll give that a quick test.
> 
> Bill
> 
> > 
> > Regards
> > VP.
> > 
> > 
> > 
> > 
> > 
> > > 
> > > Bill
> > > 
> > > > 
> > > > Thanks,
> > > > bin
> > > > >
> > > > > Thanks very much!
> > > > >
> > > > > Bill
> > > > >>
> > > > >> PASS->NA: gcc.dg/vect/vect-33.c -flto -ffat-lto-objects
> > > > >> scan-tree-dump-times vect "Vectorizing an unaligned access" 0
> > > > >> PASS->NA: gcc.dg/vect/vect-33.c scan-tree-dump-times vect "Vectorizing
> > > > >> an unaligned access" 0
> > > > >>
> > > > >> Thanks,
> > > > >> bin
> > > > >>
> > > > >
> > > > >
> > > > 
> > > 
> > > 
> > 
> 
>