| Message ID | 000001ceec01$77bc13a0$67343ae0$@arm.com |
|---|---|
| State | New |
| Headers | show |
On 28/11/13 06:17, Zhenqiang Chen wrote: > > >> -----Original Message----- >> From: Jeff Law [mailto:law@redhat.com] >> Sent: Thursday, November 28, 2013 12:43 AM >> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org >> Cc: Ramana Radhakrishnan; Richard Earnshaw >> Subject: Re: [PING] [PATCH, ARM, testcase] Skip target arm-neon for >> lp1243022.c >> >> On 11/27/13 02:05, Zhenqiang Chen wrote: >>> Ping? >> Thanks for including the actual patch you're pinging, it helps :-) >> >>>>> Hi, >>>> >>>> lp1243022.c will fail with options: -mfpu=neon -mfloat-abi=hard. >>>> >>>> Logs show it does not generate auto-incremental instruction in pass >>>> auto_inc_dec. In this case, the check of REG_INC note at subreg2 will >>>> be invalid. So skip the check for target arm-neon. >>>> >>>> All PASS with the following options: >>>> >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=hard >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=soft >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=softfp >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=soft/-mfpu=vfpv3 >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=softfp/-mfpu=vfpv3 >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=hard/-mfpu=vfpv3 >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=soft/-mfpu=neon >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=softfp/-mfpu=neon >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=hard/-mfpu=neon >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=hard >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=soft >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=softfp >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=soft/-mfpu=vfpv4 >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=softfp/-mfpu=vfpv4 >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=hard/-mfpu=vfpv4 >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=soft/-mfpu=neon >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=softfp/-mfpu=neon >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=hard/-mfpu=neon >>>> >>>> Is it OK? >>>> >>>> Thanks! >>>> -Zhenqiang >>>> >>>> testsuite/ChangeLog: >>>> 2013-11-08 Zhenqiang Chen <zhenqiang.chen@linaro.org> >>>> >>>> * gcc.target/arm/lp1243022.c: Skip target arm-neon. >> It seems to me you should be xfailing arm-neon, not skipping the test. >> Unless there is some fundamental reason why we can not generate auto-inc >> instructions on the neon. > > Thanks for the comments. Update the test case as xfail. > > diff --git a/gcc/testsuite/gcc.target/arm/lp1243022.c > b/gcc/testsuite/gcc.target/arm/lp1243022.c > index 91a544d..b2ebe7e 100644 > --- a/gcc/testsuite/gcc.target/arm/lp1243022.c > +++ b/gcc/testsuite/gcc.target/arm/lp1243022.c > @@ -1,7 +1,7 @@ > /* { dg-do compile { target arm_thumb2 } } */ > /* { dg-options "-O2 -fdump-rtl-subreg2" } */ > > -/* { dg-final { scan-rtl-dump "REG_INC" "subreg2" } } */ > +/* { dg-final { scan-rtl-dump "REG_INC" "subreg2" { xfail arm_neon } } } */ > /* { dg-final { cleanup-rtl-dump "subreg2" } } */ > struct device; > typedef unsigned int __u32; > > This test looks horribly fragile, since it's taking a large chunk of code and expecting a specific optimization to have occurred in exactly one place. The particular instruction was a large pre-modify offset, which isn't supported Looking back through the original bug report, the problem was that the subreg2 pass was losing a REG_INC note that had previously been created. Of course it's not a bug if it was never created before, but there's no easy way to tell that. On that basis, I think the original patch is the correct one, please install that. I must say that I do wonder what the value of some of these tests are in the absence of a proper unit test environment. R.
> -----Original Message----- > From: Richard Earnshaw > Sent: Friday, November 29, 2013 1:01 AM > To: Zhenqiang Chen > Cc: 'Jeff Law'; gcc-patches@gcc.gnu.org; Ramana Radhakrishnan > Subject: Re: [PING] [PATCH, ARM, testcase] Skip target arm-neon for > lp1243022.c > > On 28/11/13 06:17, Zhenqiang Chen wrote: > > > > > >> -----Original Message----- > >> From: Jeff Law [mailto:law@redhat.com] > >> Sent: Thursday, November 28, 2013 12:43 AM > >> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org > >> Cc: Ramana Radhakrishnan; Richard Earnshaw > >> Subject: Re: [PING] [PATCH, ARM, testcase] Skip target arm-neon for > >> lp1243022.c > >> > >> On 11/27/13 02:05, Zhenqiang Chen wrote: > >>> Ping? > >> Thanks for including the actual patch you're pinging, it helps :-) > >> > >>>>> Hi, > >>>> > >>>> lp1243022.c will fail with options: -mfpu=neon -mfloat-abi=hard. > >>>> > >>>> Logs show it does not generate auto-incremental instruction in pass > >>>> auto_inc_dec. In this case, the check of REG_INC note at subreg2 > >>>> will be invalid. So skip the check for target arm-neon. > >>>> > >>>> All PASS with the following options: > >>>> > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=hard > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=soft > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=softfp > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=soft/-mfpu=vfpv3 > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=softfp/-mfpu=vfpv3 > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=hard/-mfpu=vfpv3 > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=soft/-mfpu=neon > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=softfp/-mfpu=neon > >>>> -mthumb/-mcpu=cortex-a9/-mfloat-abi=hard/-mfpu=neon > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=hard > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=soft > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=softfp > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=soft/-mfpu=vfpv4 > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=softfp/-mfpu=vfpv4 > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=hard/-mfpu=vfpv4 > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=soft/-mfpu=neon > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=softfp/-mfpu=neon > >>>> -mthumb/-mcpu=cortex-a15/-mfloat-abi=hard/-mfpu=neon > >>>> > >>>> Is it OK? > >>>> > >>>> Thanks! > >>>> -Zhenqiang > >>>> > >>>> testsuite/ChangeLog: > >>>> 2013-11-08 Zhenqiang Chen <zhenqiang.chen@linaro.org> > >>>> > >>>> * gcc.target/arm/lp1243022.c: Skip target arm-neon. > >> It seems to me you should be xfailing arm-neon, not skipping the test. > >> Unless there is some fundamental reason why we can not generate > >> auto-inc instructions on the neon. > > > > Thanks for the comments. Update the test case as xfail. > > > > diff --git a/gcc/testsuite/gcc.target/arm/lp1243022.c > > b/gcc/testsuite/gcc.target/arm/lp1243022.c > > index 91a544d..b2ebe7e 100644 > > --- a/gcc/testsuite/gcc.target/arm/lp1243022.c > > +++ b/gcc/testsuite/gcc.target/arm/lp1243022.c > > @@ -1,7 +1,7 @@ > > /* { dg-do compile { target arm_thumb2 } } */ > > /* { dg-options "-O2 -fdump-rtl-subreg2" } */ > > > > -/* { dg-final { scan-rtl-dump "REG_INC" "subreg2" } } */ > > +/* { dg-final { scan-rtl-dump "REG_INC" "subreg2" { xfail arm_neon } > > +} } */ > > /* { dg-final { cleanup-rtl-dump "subreg2" } } */ struct device; > > typedef unsigned int __u32; > > > > > > This test looks horribly fragile, since it's taking a large chunk of code and > expecting a specific optimization to have occurred in exactly one place. The > particular instruction was a large pre-modify offset, which isn't supported > > Looking back through the original bug report, the problem was that the > subreg2 pass was losing a REG_INC note that had previously been created. > Of course it's not a bug if it was never created before, but there's no easy > way to tell that. > > On that basis, I think the original patch is the correct one, please install that. Thanks. The original patch was committed @r205509. -Zhenqiang > I must say that I do wonder what the value of some of these tests are in the > absence of a proper unit test environment. > > R.
diff --git a/gcc/testsuite/gcc.target/arm/lp1243022.c b/gcc/testsuite/gcc.target/arm/lp1243022.c index 91a544d..b2ebe7e 100644 --- a/gcc/testsuite/gcc.target/arm/lp1243022.c +++ b/gcc/testsuite/gcc.target/arm/lp1243022.c @@ -1,7 +1,7 @@ /* { dg-do compile { target arm_thumb2 } } */ /* { dg-options "-O2 -fdump-rtl-subreg2" } */ -/* { dg-final { scan-rtl-dump "REG_INC" "subreg2" } } */ +/* { dg-final { scan-rtl-dump "REG_INC" "subreg2" { xfail arm_neon } } } */ /* { dg-final { cleanup-rtl-dump "subreg2" } } */ struct device; typedef unsigned int __u32;