This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000, testsuite, PR65456] Changes for unaligned vector load/store support on POWER8
- From: Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- To: Vidya Praveen <vidyapraveen at arm dot com>
- Cc: "Bin.Cheng" <amker dot cheng at gmail dot com>, gcc-patches List <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 16 Jun 2015 08:58:20 -0500
- Subject: Re: [PATCH, rs6000, testsuite, PR65456] Changes for unaligned vector load/store support on POWER8
- Authentication-results: sourceware.org; auth=none
- References: <1427650979 dot 2939 dot 33 dot camel at gnopaine> <CAHFci2_ZYzSr_rkLQJ83u5XmbXwpZJj9D0EdMbino-Pg=kcaXA at mail dot gmail dot com> <1430141211 dot 31097 dot 2 dot camel at gnopaine> <CAHFci29gQiSOdw40oxpQjc1m1WFaoarkHz5er45OVv7C_A+aDg at mail dot gmail dot com> <1430397258 dot 15722 dot 1 dot camel at gnopaine> <20150612163649 dot GI5255 at e103625-lin dot cambridge dot arm dot com> <1434395671 dot 2747 dot 8 dot camel at gnopaine> <20150616133729 dot GB992 at e103625-lin dot cambridge dot arm dot com>
On Tue, 2015-06-16 at 14:37 +0100, Vidya Praveen wrote:
> 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?
It shouldn't vectorize for targets other than powerpc, period
(originally the test was, don't vectorize for absolutely anybody). This
will now allow vectorization only for powerpc targets for which
vect_hw_misalign is true (POWER8 only at this time). The cost function
in that case allows us to vectorize it.
However, I may not get approval to commit this, since changes to 4.8
branch are now under release manager control.
Thanks,
Bill
>
> 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
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > >
> >
> >
>