| Message ID | 87tw8tofoa.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, 20 Jan 2017, Nick Clifton wrote: > Hi Guys, > > [I have been asked to look at this PR in the hopes that it can be > fixed soon and so no longer act as a blocker for the gcc 7 branch]. > > It seems to me that Richard's proposed patch does work: > > https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00909.html > > The only problem is that the check_effective_target_vect_hw_misalign > proc is always returning 0 (or false) for ARM, even when unaligned > vectors are supported. This is why Richard's patch introduces a new > failure for the arm-* targets. > > So what I would like to suggest is an extended patch (attached) which > also updates the check_effective_target_vect_hw_misalign proc to use > the check_effective_target_arm_vect_no_misalign proc. With this patch > applied not only does the gcc.dg/vect/vect-strided-a-u8-i2-gap.c test > for both big-endian and little-endian arm targets, but there is also a > significant reduction in the number of failures in the gcc.dg/vect > tests overall: > > Little Endian ARM: > < # of expected passes 3275 > < # of unexpected failures 63 > < # of unexpected successes 125 > < # of expected failures 123 > < # of unsupported tests 153 > --- > > # of expected passes 3448 > > # of unexpected failures 2 > > # of unexpected successes 14 > > # of expected failures 131 > > # of unsupported tests 151 > > Big Endian ARM: > < # of expected passes 2995 > < # of unexpected failures 269 > < # of unexpected successes 21 > < # of expected failures 128 > --- > > # of expected passes 3037 > > # of unexpected failures 127 > > # of unexpected successes 24 > > # of expected failures 228 > > Which looks like a win to me. So - any objections to my applying this > patch and then closing the PR ? Ok. Thanks, Richard. > Cheers > Nick > > gcc/ChangeLog > 2017-01-20 Richard Biener <rguenther@suse.de> > Nick Clifton <nickc@redhat.com> > > PR testsuite/78421 > * lib/target-supports.exp (check_effective_target_vect_hw_misalign): > If the target is ARM return the result of the > check_effective_target_arm_vect_no_misalign proc. > * gcc.dg/vect/vect-strided-a-u8-i2-gap.c: If the target does not > support unaligned vectors then only expect one of the loops to be > unrolled. > >
Hi Nick, On 23 January 2017 at 10:04, Richard Biener <rguenther@suse.de> wrote: > On Fri, 20 Jan 2017, Nick Clifton wrote: > >> Hi Guys, >> >> [I have been asked to look at this PR in the hopes that it can be >> fixed soon and so no longer act as a blocker for the gcc 7 branch]. >> >> It seems to me that Richard's proposed patch does work: >> >> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00909.html >> >> The only problem is that the check_effective_target_vect_hw_misalign >> proc is always returning 0 (or false) for ARM, even when unaligned >> vectors are supported. This is why Richard's patch introduces a new >> failure for the arm-* targets. >> >> So what I would like to suggest is an extended patch (attached) which >> also updates the check_effective_target_vect_hw_misalign proc to use >> the check_effective_target_arm_vect_no_misalign proc. With this patch >> applied not only does the gcc.dg/vect/vect-strided-a-u8-i2-gap.c test >> for both big-endian and little-endian arm targets, but there is also a >> significant reduction in the number of failures in the gcc.dg/vect >> tests overall: >> >> Little Endian ARM: >> < # of expected passes 3275 >> < # of unexpected failures 63 >> < # of unexpected successes 125 >> < # of expected failures 123 >> < # of unsupported tests 153 >> --- >> > # of expected passes 3448 >> > # of unexpected failures 2 >> > # of unexpected successes 14 >> > # of expected failures 131 >> > # of unsupported tests 151 >> >> Big Endian ARM: >> < # of expected passes 2995 >> < # of unexpected failures 269 >> < # of unexpected successes 21 >> < # of expected failures 128 >> --- >> > # of expected passes 3037 >> > # of unexpected failures 127 >> > # of unexpected successes 24 >> > # of expected failures 228 >> >> Which looks like a win to me. So - any objections to my applying this >> patch and then closing the PR ? > > Ok. > I must be missing something, but I see many regressions since you committed this patch (r244796). See http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244796/report-build-info.html for more details. In short, on arm-*, gcc.dg/vect/vect-strided-a-u8-i2-gap.c -flto -ffat-lto-objects scan-tree-dump-times vect "vectorized 1 loops" 1 gcc.dg/vect/vect-strided-a-u8-i2-gap.c scan-tree-dump-times vect "vectorized 1 loops" 1 now FAIL instead of PASS. on armeb, there are many more differences. Christophe > Thanks, > Richard. > >> Cheers >> Nick >> >> gcc/ChangeLog >> 2017-01-20 Richard Biener <rguenther@suse.de> >> Nick Clifton <nickc@redhat.com> >> >> PR testsuite/78421 >> * lib/target-supports.exp (check_effective_target_vect_hw_misalign): >> If the target is ARM return the result of the >> check_effective_target_arm_vect_no_misalign proc. >> * gcc.dg/vect/vect-strided-a-u8-i2-gap.c: If the target does not >> support unaligned vectors then only expect one of the loops to be >> unrolled. >> >> > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
On 23/01/17 19:26, Christophe Lyon wrote: > Hi Nick, > > On 23 January 2017 at 10:04, Richard Biener <rguenther@suse.de> wrote: >> On Fri, 20 Jan 2017, Nick Clifton wrote: >> >>> Hi Guys, >>> >>> [I have been asked to look at this PR in the hopes that it can be >>> fixed soon and so no longer act as a blocker for the gcc 7 branch]. >>> >>> It seems to me that Richard's proposed patch does work: >>> >>> https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00909.html >>> >>> The only problem is that the check_effective_target_vect_hw_misalign >>> proc is always returning 0 (or false) for ARM, even when unaligned >>> vectors are supported. This is why Richard's patch introduces a new >>> failure for the arm-* targets. >>> >>> So what I would like to suggest is an extended patch (attached) which >>> also updates the check_effective_target_vect_hw_misalign proc to use >>> the check_effective_target_arm_vect_no_misalign proc. With this patch >>> applied not only does the gcc.dg/vect/vect-strided-a-u8-i2-gap.c test >>> for both big-endian and little-endian arm targets, but there is also a >>> significant reduction in the number of failures in the gcc.dg/vect >>> tests overall: >>> >>> Little Endian ARM: >>> < # of expected passes 3275 >>> < # of unexpected failures 63 >>> < # of unexpected successes 125 >>> < # of expected failures 123 >>> < # of unsupported tests 153 >>> --- >>>> # of expected passes 3448 >>>> # of unexpected failures 2 >>>> # of unexpected successes 14 >>>> # of expected failures 131 >>>> # of unsupported tests 151 >>> Big Endian ARM: >>> < # of expected passes 2995 >>> < # of unexpected failures 269 >>> < # of unexpected successes 21 >>> < # of expected failures 128 >>> --- >>>> # of expected passes 3037 >>>> # of unexpected failures 127 >>>> # of unexpected successes 24 >>>> # of expected failures 228 >>> Which looks like a win to me. So - any objections to my applying this >>> patch and then closing the PR ? >> Ok. >> > I must be missing something, but I see many regressions since you > committed this patch (r244796). > See > http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/244796/report-build-info.html > for more details. > > In short, on arm-*, > gcc.dg/vect/vect-strided-a-u8-i2-gap.c -flto -ffat-lto-objects > scan-tree-dump-times vect "vectorized 1 loops" 1 > gcc.dg/vect/vect-strided-a-u8-i2-gap.c scan-tree-dump-times vect > "vectorized 1 loops" 1 > now FAIL instead of PASS. I also see this when testing -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard. Thanks, Kyrill > on armeb, there are many more differences. > > Christophe > > >> Thanks, >> Richard. >> >>> Cheers >>> Nick >>> >>> gcc/ChangeLog >>> 2017-01-20 Richard Biener <rguenther@suse.de> >>> Nick Clifton <nickc@redhat.com> >>> >>> PR testsuite/78421 >>> * lib/target-supports.exp (check_effective_target_vect_hw_misalign): >>> If the target is ARM return the result of the >>> check_effective_target_arm_vect_no_misalign proc. >>> * gcc.dg/vect/vect-strided-a-u8-i2-gap.c: If the target does not >>> support unaligned vectors then only expect one of the loops to be >>> unrolled. >>> >>> >> -- >> Richard Biener <rguenther@suse.de> >> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Index: gcc/testsuite/gcc.dg/vect/vect-strided-a-u8-i2-gap.c =================================================================== --- gcc/testsuite/gcc.dg/vect/vect-strided-a-u8-i2-gap.c (revision 244691) +++ gcc/testsuite/gcc.dg/vect/vect-strided-a-u8-i2-gap.c (working copy) @@ -71,5 +71,6 @@ return 0; } -/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect" { target vect_strided2 } } } */ +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 1 "vect" { target { vect_strided2 && { ! vect_hw_misalign } } } } } */ +/* { dg-final { scan-tree-dump-times "vectorized 2 loops" 1 "vect" { target { vect_strided2 && vect_hw_misalign } } } } */ Index: gcc/testsuite/lib/target-supports.exp =================================================================== --- gcc/testsuite/lib/target-supports.exp (revision 244691) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -5732,6 +5732,9 @@ || ([istarget mips*-*-*] && [et-is-effective-target mips_msa]) } { set et_vect_hw_misalign_saved($et_index) 1 } + if { [istarget arm*-*-*] } { + set et_vect_hw_misalign_saved($et_index) [check_effective_target_arm_vect_no_misalign] + } } verbose "check_effective_target_vect_hw_misalign:\ returning $et_vect_hw_misalign_saved($et_index)" 2