* [arm][patch] fix arm_neon_ok check on !arm_arch7 @ 2014-09-13 21:39 Andrew Stubbs 2014-09-15 9:46 ` Richard Earnshaw 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-09-13 21:39 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 414 bytes --] Hi, I get a lot of "vect/*" and "neon-*" test failure in my armv5te testing because the arm_neon_ok test incorrectly detects that NEON is valid on arm926ej-s. It turns out that the reason is that the compiler only disallows NEON for Thumb1 or soft-float configurations. Otherwise it just takes -mfpu=neon at face value, regardless of -march or -mcpu. This patch limits NEON to armv7 or higher. OK? Andrew [-- Attachment #2: arm_neon_ok.patch --] [-- Type: text/x-patch, Size: 559 bytes --] 2014-09-13 Andrew Stubbs <ams@codesourcery.com> gcc/ * config/arm/arm.h (TARGET_NEON): Ensure target is v7 or higher. Index: gcc/config/arm/arm.h =================================================================== --- gcc/config/arm/arm.h (revision 215228) +++ gcc/config/arm/arm.h (working copy) @@ -323,6 +323,7 @@ and TARGET_HARD_FLOAT to ensure that NEON instructions are available. */ #define TARGET_NEON (TARGET_32BIT && TARGET_HARD_FLOAT \ + && arm_arch7 \ && TARGET_VFP && arm_fpu_desc->neon) /* Q-bit is present. */ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-13 21:39 [arm][patch] fix arm_neon_ok check on !arm_arch7 Andrew Stubbs @ 2014-09-15 9:46 ` Richard Earnshaw 2014-09-15 10:56 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Richard Earnshaw @ 2014-09-15 9:46 UTC (permalink / raw) To: Andrew Stubbs, gcc-patches On 13/09/14 22:39, Andrew Stubbs wrote: > Hi, > > I get a lot of "vect/*" and "neon-*" test failure in my armv5te testing > because the arm_neon_ok test incorrectly detects that NEON is valid on > arm926ej-s. > > It turns out that the reason is that the compiler only disallows NEON > for Thumb1 or soft-float configurations. Otherwise it just takes > -mfpu=neon at face value, regardless of -march or -mcpu. > > This patch limits NEON to armv7 or higher. > > OK? > > Andrew > > > arm_neon_ok.patch > > > 2014-09-13 Andrew Stubbs <ams@codesourcery.com> > > gcc/ > * config/arm/arm.h (TARGET_NEON): Ensure target is v7 or higher. > > Index: gcc/config/arm/arm.h > =================================================================== > --- gcc/config/arm/arm.h (revision 215228) > +++ gcc/config/arm/arm.h (working copy) > @@ -323,6 +323,7 @@ > and TARGET_HARD_FLOAT to ensure that NEON instructions are > available. */ > #define TARGET_NEON (TARGET_32BIT && TARGET_HARD_FLOAT \ > + && arm_arch7 \ > && TARGET_VFP && arm_fpu_desc->neon) > > /* Q-bit is present. */ > Hmm, I wonder if arm_override_options should reject neon + (arch < 7). R. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-15 9:46 ` Richard Earnshaw @ 2014-09-15 10:56 ` Andrew Stubbs 2014-09-15 13:30 ` Richard Earnshaw 2014-09-23 8:27 ` James Greenhalgh 0 siblings, 2 replies; 25+ messages in thread From: Andrew Stubbs @ 2014-09-15 10:56 UTC (permalink / raw) To: Richard Earnshaw, gcc-patches [-- Attachment #1: Type: text/plain, Size: 201 bytes --] On 15/09/14 10:46, Richard Earnshaw wrote: > Hmm, I wonder if arm_override_options should reject neon + (arch < 7). Is this more to your taste? Andrew P.S. arm_override_options was renamed in 2010. [-- Attachment #2: arm_neon_ok-2.patch --] [-- Type: text/x-patch, Size: 558 bytes --] 2014-09-15 Andrew Stubbs <ams@codesourcery.com> * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon when architecture is older than ARMv7. Index: gcc/config/arm/arm.c =================================================================== --- gcc/config/arm/arm.c (revision 215228) +++ gcc/config/arm/arm.c (working copy) @@ -2845,6 +2845,9 @@ arm_fpu_desc = &all_fpus[arm_fpu_index]; + if (TARGET_NEON && !arm_arch7) + error ("target CPU does not support NEON"); + switch (arm_fpu_desc->model) { case ARM_FP_MODEL_VFP: ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-15 10:56 ` Andrew Stubbs @ 2014-09-15 13:30 ` Richard Earnshaw 2014-09-17 11:00 ` Andrew Stubbs 2014-09-23 8:27 ` James Greenhalgh 1 sibling, 1 reply; 25+ messages in thread From: Richard Earnshaw @ 2014-09-15 13:30 UTC (permalink / raw) To: Andrew Stubbs, gcc-patches On 15/09/14 11:56, Andrew Stubbs wrote: > On 15/09/14 10:46, Richard Earnshaw wrote: >> Hmm, I wonder if arm_override_options should reject neon + (arch < 7). > > Is this more to your taste? > Yep, that's fine. > Andrew > > P.S. arm_override_options was renamed in 2010. I'm getting old :-( R. > > > arm_neon_ok-2.patch > > > 2014-09-15 Andrew Stubbs <ams@codesourcery.com> > > * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon > when architecture is older than ARMv7. > > Index: gcc/config/arm/arm.c > =================================================================== > --- gcc/config/arm/arm.c (revision 215228) > +++ gcc/config/arm/arm.c (working copy) > @@ -2845,6 +2845,9 @@ > > arm_fpu_desc = &all_fpus[arm_fpu_index]; > > + if (TARGET_NEON && !arm_arch7) > + error ("target CPU does not support NEON"); > + > switch (arm_fpu_desc->model) > { > case ARM_FP_MODEL_VFP: > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-15 13:30 ` Richard Earnshaw @ 2014-09-17 11:00 ` Andrew Stubbs 0 siblings, 0 replies; 25+ messages in thread From: Andrew Stubbs @ 2014-09-17 11:00 UTC (permalink / raw) To: Richard Earnshaw, gcc-patches On 15/09/14 14:29, Richard Earnshaw wrote: > Yep, that's fine. Committed, thanks. Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-15 10:56 ` Andrew Stubbs 2014-09-15 13:30 ` Richard Earnshaw @ 2014-09-23 8:27 ` James Greenhalgh 2014-09-23 15:22 ` Stubbs, Andrew 2014-12-02 14:01 ` Kyrill Tkachov 1 sibling, 2 replies; 25+ messages in thread From: James Greenhalgh @ 2014-09-23 8:27 UTC (permalink / raw) To: Andrew Stubbs; +Cc: Richard Earnshaw, gcc-patches On Mon, Sep 15, 2014 at 11:56:03AM +0100, Andrew Stubbs wrote: > On 15/09/14 10:46, Richard Earnshaw wrote: > > Hmm, I wonder if arm_override_options should reject neon + (arch < 7). > > Is this more to your taste? Is this really such a good idea? It causes carnage throughout the testsuite if you have configured with support for Neon and the testcase is written with dg-options for a pre-armv7-a -march value. For example in: testsuite/gcc.target/arm/di-longlong64-sync-withhelpers.c Which forces -march=armv5. Perhaps you just have to fix the effective-target-ok tests - but then we lose swathes of test coverage. Thanks, James > > Andrew > > P.S. arm_override_options was renamed in 2010. > 2014-09-15 Andrew Stubbs <ams@codesourcery.com> > > * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon > when architecture is older than ARMv7. > > Index: gcc/config/arm/arm.c > =================================================================== > --- gcc/config/arm/arm.c (revision 215228) > +++ gcc/config/arm/arm.c (working copy) > @@ -2845,6 +2845,9 @@ > > arm_fpu_desc = &all_fpus[arm_fpu_index]; > > + if (TARGET_NEON && !arm_arch7) > + error ("target CPU does not support NEON"); > + > switch (arm_fpu_desc->model) > { > case ARM_FP_MODEL_VFP: ^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-23 8:27 ` James Greenhalgh @ 2014-09-23 15:22 ` Stubbs, Andrew 2014-10-15 16:37 ` Jiong Wang 2014-12-02 14:01 ` Kyrill Tkachov 1 sibling, 1 reply; 25+ messages in thread From: Stubbs, Andrew @ 2014-09-23 15:22 UTC (permalink / raw) To: James Greenhalgh; +Cc: Richard Earnshaw, gcc-patches Maybe the original patch is better? Or maybe it should reconfigure the FPU instead of erroring out? But reconfigure it to what? Andrew ________________________________________ From: James Greenhalgh [james.greenhalgh@arm.com] Sent: 23 September 2014 09:27 To: Stubbs, Andrew Cc: Richard Earnshaw; gcc-patches@gcc.gnu.org Subject: Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 On Mon, Sep 15, 2014 at 11:56:03AM +0100, Andrew Stubbs wrote: > On 15/09/14 10:46, Richard Earnshaw wrote: > > Hmm, I wonder if arm_override_options should reject neon + (arch < 7). > > Is this more to your taste? Is this really such a good idea? It causes carnage throughout the testsuite if you have configured with support for Neon and the testcase is written with dg-options for a pre-armv7-a -march value. For example in: testsuite/gcc.target/arm/di-longlong64-sync-withhelpers.c Which forces -march=armv5. Perhaps you just have to fix the effective-target-ok tests - but then we lose swathes of test coverage. Thanks, James > > Andrew > > P.S. arm_override_options was renamed in 2010. > 2014-09-15 Andrew Stubbs <ams@codesourcery.com> > > * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon > when architecture is older than ARMv7. > > Index: gcc/config/arm/arm.c > =================================================================== > --- gcc/config/arm/arm.c (revision 215228) > +++ gcc/config/arm/arm.c (working copy) > @@ -2845,6 +2845,9 @@ > > arm_fpu_desc = &all_fpus[arm_fpu_index]; > > + if (TARGET_NEON && !arm_arch7) > + error ("target CPU does not support NEON"); > + > switch (arm_fpu_desc->model) > { > case ARM_FP_MODEL_VFP: ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-23 15:22 ` Stubbs, Andrew @ 2014-10-15 16:37 ` Jiong Wang 2014-10-15 16:59 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Jiong Wang @ 2014-10-15 16:37 UTC (permalink / raw) To: Stubbs, Andrew, James Greenhalgh; +Cc: Richard Earnshaw, gcc-patches On 23/09/14 16:22, Stubbs, Andrew wrote: > Maybe the original patch is better? Or maybe it should reconfigure the FPU instead of erroring out? But reconfigure it to what? Andrew, are you still working on this? a bunch of tests on my local environment failed because of the reason James mentioned. for example gcc.target/arm/xor-and.c etc. ... gcc.target/arm/xor-and.c:1:0: error: target CPU does not support NEON ... Regards. Jiong > > Andrew > ________________________________________ > From: James Greenhalgh [james.greenhalgh@arm.com] > Sent: 23 September 2014 09:27 > To: Stubbs, Andrew > Cc: Richard Earnshaw; gcc-patches@gcc.gnu.org > Subject: Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 > > On Mon, Sep 15, 2014 at 11:56:03AM +0100, Andrew Stubbs wrote: >> On 15/09/14 10:46, Richard Earnshaw wrote: >>> Hmm, I wonder if arm_override_options should reject neon + (arch < 7). >> Is this more to your taste? > Is this really such a good idea? It causes carnage throughout the > testsuite if you have configured with support for Neon and the testcase > is written with dg-options for a pre-armv7-a -march value. > > For example in: > testsuite/gcc.target/arm/di-longlong64-sync-withhelpers.c > > Which forces -march=armv5. > > Perhaps you just have to fix the effective-target-ok tests - but then > we lose swathes of test coverage. > > Thanks, > James > >> Andrew >> >> P.S. arm_override_options was renamed in 2010. >> 2014-09-15 Andrew Stubbs <ams@codesourcery.com> >> >> * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon >> when architecture is older than ARMv7. >> >> Index: gcc/config/arm/arm.c >> =================================================================== >> --- gcc/config/arm/arm.c (revision 215228) >> +++ gcc/config/arm/arm.c (working copy) >> @@ -2845,6 +2845,9 @@ >> >> arm_fpu_desc = &all_fpus[arm_fpu_index]; >> >> + if (TARGET_NEON && !arm_arch7) >> + error ("target CPU does not support NEON"); >> + >> switch (arm_fpu_desc->model) >> { >> case ARM_FP_MODEL_VFP: > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-10-15 16:37 ` Jiong Wang @ 2014-10-15 16:59 ` Andrew Stubbs 2014-10-16 13:53 ` Jiong Wang 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-10-15 16:59 UTC (permalink / raw) To: Jiong Wang, James Greenhalgh; +Cc: Richard Earnshaw, gcc-patches On 15/10/14 17:34, Jiong Wang wrote: > On 23/09/14 16:22, Stubbs, Andrew wrote: >> Maybe the original patch is better? Or maybe it should reconfigure the >> FPU instead of erroring out? But reconfigure it to what? > > Andrew, > > are you still working on this? > a bunch of tests on my local environment failed because of the reason > James mentioned. for example gcc.target/arm/xor-and.c etc. I can do, but nobody had any suggestions of a better fix. It seems that the testsuite is broken different ways with and without neon. I get it running (and failing) NEON tests on multilibs it should not, and you get it failing non-NEON tests on toolchains configured to have NEON by default. I can apply my original patch, if that works better? Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-10-15 16:59 ` Andrew Stubbs @ 2014-10-16 13:53 ` Jiong Wang 2014-11-07 10:35 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Jiong Wang @ 2014-10-16 13:53 UTC (permalink / raw) To: Andrew Stubbs, James Greenhalgh; +Cc: Richard Earnshaw, gcc-patches On 15/10/14 17:58, Andrew Stubbs wrote: > On 15/10/14 17:34, Jiong Wang wrote: >> On 23/09/14 16:22, Stubbs, Andrew wrote: >>> Maybe the original patch is better? Or maybe it should reconfigure the >>> FPU instead of erroring out? But reconfigure it to what? >> Andrew, >> >> are you still working on this? >> a bunch of tests on my local environment failed because of the reason >> James mentioned. for example gcc.target/arm/xor-and.c etc. > I can do, but nobody had any suggestions of a better fix. > > It seems that the testsuite is broken different ways with and without > neon. I get it running (and failing) NEON tests on multilibs it should > not, and you get it failing non-NEON tests on toolchains configured to > have NEON by default. > > I can apply my original patch, if that works better? Hi Andrew, if armv6 never co-exist with NEON, personally I think your original patch is better because TARGET_NEON generally will be used when all options are processed. any way, this needs gate keeper's approval. thanks. Regards, Jiong > > Andrew > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-10-16 13:53 ` Jiong Wang @ 2014-11-07 10:35 ` Andrew Stubbs 2014-11-14 11:17 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-11-07 10:35 UTC (permalink / raw) To: Jiong Wang, Andrew Stubbs, James Greenhalgh, Richard Earnshaw; +Cc: gcc-patches > if armv6 never co-exist with NEON, personally I think your original > patch is better > because TARGET_NEON generally will be used when all options are > processed. > > any way, this needs gate keeper's approval. Ping, Richard. Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-11-07 10:35 ` Andrew Stubbs @ 2014-11-14 11:17 ` Andrew Stubbs 2014-11-26 13:11 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-11-14 11:17 UTC (permalink / raw) To: Jiong Wang, Andrew Stubbs, James Greenhalgh, Richard Earnshaw; +Cc: gcc-patches On 07/11/14 10:35, Andrew Stubbs wrote: >> if armv6 never co-exist with NEON, personally I think your original >> patch is better >> because TARGET_NEON generally will be used when all options are >> processed. >> >> any way, this needs gate keeper's approval. > > Ping, Richard. Ping. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-11-14 11:17 ` Andrew Stubbs @ 2014-11-26 13:11 ` Andrew Stubbs 2014-11-27 17:28 ` Mike Stump 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-11-26 13:11 UTC (permalink / raw) To: Andrew Stubbs, Jiong Wang, James Greenhalgh, Richard Earnshaw; +Cc: gcc-patches On 14/11/14 11:12, Andrew Stubbs wrote: > On 07/11/14 10:35, Andrew Stubbs wrote: >>> if armv6 never co-exist with NEON, personally I think your original >>> patch is better >>> because TARGET_NEON generally will be used when all options are >>> processed. >>> >>> any way, this needs gate keeper's approval. >> >> Ping, Richard. > > Ping. Ping. Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-11-26 13:11 ` Andrew Stubbs @ 2014-11-27 17:28 ` Mike Stump 2014-11-27 19:29 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Mike Stump @ 2014-11-27 17:28 UTC (permalink / raw) To: Andrew Stubbs Cc: Andrew Stubbs, Jiong Wang, James Greenhalgh, Richard Earnshaw, gcc-patches On Nov 26, 2014, at 4:24 AM, Andrew Stubbs <andrew_stubbs@mentor.com> wrote: > On 14/11/14 11:12, Andrew Stubbs wrote: >> On 07/11/14 10:35, Andrew Stubbs wrote: >>>> if armv6 never co-exist with NEON, personally I think your original >>>> patch is better >>>> because TARGET_NEON generally will be used when all options are >>>> processed. >>>> >>>> any way, this needs gate keeper's approval. >>> >>> Ping, Richard. >> >> Ping. > > Ping. Could you include a link or the patch. If the test suite, I'll review it if no one else steps up. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-11-27 17:28 ` Mike Stump @ 2014-11-27 19:29 ` Andrew Stubbs 2014-11-28 8:42 ` Mike Stump 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-11-27 19:29 UTC (permalink / raw) To: Mike Stump; +Cc: Jiong Wang, James Greenhalgh, Richard Earnshaw, gcc-patches On 27/11/14 17:05, Mike Stump wrote: > Could you include a link or the patch. If the test suite, I'll review it if no one else steps up. The original patch is here: https://gcc.gnu.org/ml/gcc-patches/2014-09/msg01119.html Thanks Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-11-27 19:29 ` Andrew Stubbs @ 2014-11-28 8:42 ` Mike Stump 0 siblings, 0 replies; 25+ messages in thread From: Mike Stump @ 2014-11-28 8:42 UTC (permalink / raw) To: Andrew Stubbs Cc: Mike Stump, Jiong Wang, James Greenhalgh, Richard Earnshaw, gcc-patches On Nov 27, 2014, at 9:28 AM, Andrew Stubbs <ams@codesourcery.com> wrote: > On 27/11/14 17:05, Mike Stump wrote: >> Could you include a link or the patch. If the test suite, I'll review it if no one else steps up. > > The original patch is here: > > https://gcc.gnu.org/ml/gcc-patches/2014-09/msg01119.html Sorry, arm people will have to approve... Many reasonable choices, yet I bet only one is best. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-09-23 8:27 ` James Greenhalgh 2014-09-23 15:22 ` Stubbs, Andrew @ 2014-12-02 14:01 ` Kyrill Tkachov 2014-12-02 21:45 ` Ramana Radhakrishnan 1 sibling, 1 reply; 25+ messages in thread From: Kyrill Tkachov @ 2014-12-02 14:01 UTC (permalink / raw) To: James Greenhalgh, Andrew Stubbs; +Cc: Richard Earnshaw, gcc-patches On 23/09/14 09:27, James Greenhalgh wrote: > On Mon, Sep 15, 2014 at 11:56:03AM +0100, Andrew Stubbs wrote: >> On 15/09/14 10:46, Richard Earnshaw wrote: >>> Hmm, I wonder if arm_override_options should reject neon + (arch < 7). >> Is this more to your taste? > Is this really such a good idea? It causes carnage throughout the > testsuite if you have configured with support for Neon and the testcase > is written with dg-options for a pre-armv7-a -march value. > > For example in: > testsuite/gcc.target/arm/di-longlong64-sync-withhelpers.c > > Which forces -march=armv5. > > Perhaps you just have to fix the effective-target-ok tests - but then > we lose swathes of test coverage. This also causes subtle Linux kernel compile failures. Over there they use make rules where they check if the compiler supports -march=armv5te and if not use -march=armv4t. With this patch if the compiler is configured with something like --with-fpu=neon the test will fail with your error message, even though the compiler supports -march=armv5te. Kyrill > > Thanks, > James > >> Andrew >> >> P.S. arm_override_options was renamed in 2010. >> 2014-09-15 Andrew Stubbs <ams@codesourcery.com> >> >> * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon >> when architecture is older than ARMv7. >> >> Index: gcc/config/arm/arm.c >> =================================================================== >> --- gcc/config/arm/arm.c (revision 215228) >> +++ gcc/config/arm/arm.c (working copy) >> @@ -2845,6 +2845,9 @@ >> >> arm_fpu_desc = &all_fpus[arm_fpu_index]; >> >> + if (TARGET_NEON && !arm_arch7) >> + error ("target CPU does not support NEON"); >> + >> switch (arm_fpu_desc->model) >> { >> case ARM_FP_MODEL_VFP: ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-12-02 14:01 ` Kyrill Tkachov @ 2014-12-02 21:45 ` Ramana Radhakrishnan 2014-12-03 15:03 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Ramana Radhakrishnan @ 2014-12-02 21:45 UTC (permalink / raw) To: Kyrill Tkachov Cc: James Greenhalgh, Andrew Stubbs, Richard Earnshaw, gcc-patches On Tue, Dec 2, 2014 at 2:01 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: > > On 23/09/14 09:27, James Greenhalgh wrote: >> >> On Mon, Sep 15, 2014 at 11:56:03AM +0100, Andrew Stubbs wrote: >>> >>> On 15/09/14 10:46, Richard Earnshaw wrote: >>>> >>>> Hmm, I wonder if arm_override_options should reject neon + (arch < 7). >>> >>> Is this more to your taste? >> >> Is this really such a good idea? It causes carnage throughout the >> testsuite if you have configured with support for Neon and the testcase >> is written with dg-options for a pre-armv7-a -march value. >> >> For example in: >> testsuite/gcc.target/arm/di-longlong64-sync-withhelpers.c >> >> Which forces -march=armv5. >> >> Perhaps you just have to fix the effective-target-ok tests - but then >> we lose swathes of test coverage. > > > This also causes subtle Linux kernel compile failures. > Over there they use make rules where they check if the compiler supports > -march=armv5te and if not use -march=armv4t. > With this patch if the compiler is configured with something like > --with-fpu=neon the test will fail with your error message, > even though the compiler supports -march=armv5te. I've spent some time this evening pondering over your patch. Firstly it appears that the current behaviour is going to cause more breakage than originally expected. If this is to go in we'd have a number of users having to add -mfloat-abi=soft to the command line option to ensure that -march=armv5te works just fine on the files where march=armv5te in the first places. I'm not sure that the original patch is enough. The tools have always allowed us to drop down the arch to march=armv5te along with using -mfpu=neon. We are now changing command line behaviour, so an inform in terms of diagnostics to the user would be useful as it states that we don't really have mfpu=neon generating neon code any more because of this particular case. If we are to do this then the original patch is probably not enough as it then doesn't handle the case of TARGET_VFP3 / TARGET_VFP5 / TARGET_NEON_FP16 / TARGET_FP16 / TARGET_FPU_ARMV8 etc. etc. etc. regards Ramana > > Kyrill > > > >> >> Thanks, >> James >> >>> Andrew >>> >>> P.S. arm_override_options was renamed in 2010. >>> 2014-09-15 Andrew Stubbs <ams@codesourcery.com> >>> >>> * gcc/config/arm/arm.c (arm_option_override): Reject -mfpu=neon >>> when architecture is older than ARMv7. >>> >>> Index: gcc/config/arm/arm.c >>> =================================================================== >>> --- gcc/config/arm/arm.c (revision 215228) >>> +++ gcc/config/arm/arm.c (working copy) >>> @@ -2845,6 +2845,9 @@ >>> arm_fpu_desc = &all_fpus[arm_fpu_index]; >>> + if (TARGET_NEON && !arm_arch7) >>> + error ("target CPU does not support NEON"); >>> + >>> switch (arm_fpu_desc->model) >>> { >>> case ARM_FP_MODEL_VFP: > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-12-02 21:45 ` Ramana Radhakrishnan @ 2014-12-03 15:03 ` Andrew Stubbs 2014-12-23 17:32 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-12-03 15:03 UTC (permalink / raw) To: ramrad01, Kyrill Tkachov Cc: James Greenhalgh, Andrew Stubbs, Richard Earnshaw, gcc-patches On 02/12/14 21:45, Ramana Radhakrishnan wrote: > I've spent some time this evening pondering over your patch. Firstly > it appears that the current behaviour is going to cause more breakage > than originally expected. If this is to go in we'd have a number of > users having to add -mfloat-abi=soft to the command line option to > ensure that -march=armv5te works just fine on the files where > march=armv5te in the first places. Agreed. I've just reverted the patch. > I'm not sure that the original patch is enough. > > The tools have always allowed us to drop down the arch to > march=armv5te along with using -mfpu=neon. We are now changing command > line behaviour, so an inform in terms of diagnostics to the user would > be useful as it states that we don't really have mfpu=neon generating > neon code any more because of this particular case. If we are to do > this then the original patch is probably not enough as it then doesn't > handle the case of TARGET_VFP3 / TARGET_VFP5 / TARGET_NEON_FP16 / > TARGET_FP16 / TARGET_FPU_ARMV8 etc. etc. etc. I'll take a look at those shortly. Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-12-03 15:03 ` Andrew Stubbs @ 2014-12-23 17:32 ` Andrew Stubbs 2015-01-12 14:14 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2014-12-23 17:32 UTC (permalink / raw) To: ramrad01, Kyrill Tkachov; +Cc: James Greenhalgh, Richard Earnshaw, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1515 bytes --] On 03/12/14 15:03, Andrew Stubbs wrote: >> The tools have always allowed us to drop down the arch to >> march=armv5te along with using -mfpu=neon. We are now changing command >> line behaviour, so an inform in terms of diagnostics to the user would >> be useful as it states that we don't really have mfpu=neon generating >> neon code any more because of this particular case. If we are to do >> this then the original patch is probably not enough as it then doesn't >> handle the case of TARGET_VFP3 / TARGET_VFP5 / TARGET_NEON_FP16 / >> TARGET_FP16 / TARGET_FPU_ARMV8 etc. etc. etc. > > I'll take a look at those shortly. Or, not so shortly. It seems that, on ARM, the arch/CPU setting is basically orthogonal to the FPU setting, and the compiler doesn't even try to match the one to the other. The assembler does the same. In fact, the testcases that James refers to, that have hard-coded -march options, really do emit armv4 code with Neon, say, although most probably don't have vectorizable code. They only work because they're most likely executed on Neon hardware. This means that there's no obvious patch to fix the issue, in the compiler. It's easy to reject Neon for pre-v7 CPUs, but that has consequences, as we've seen. We'd have to have a table of fall-back FPUs or something, and that doesn't seem straight-forward (and anyway, I'm not sure what values to enter into that table). So, I've attacked the problem from the other end, and updated the compiler check. OK to commit? Andrew [-- Attachment #2: arm-neon-ok-2.patch --] [-- Type: text/x-patch, Size: 693 bytes --] 2014-12-23 Andrew Stubbs <ams@codesourcery.com> gcc/testsuite/ * lib/target-supports.exp (check_effective_target_arm_neon_ok_nocache): Don't try to test Neon on ARM architures before v7. Index: gcc/testsuite/lib/target-supports.exp =================================================================== --- gcc/testsuite/lib/target-supports.exp (revision 219043) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -2565,6 +2565,9 @@ if { [check_no_compiler_messages_nocache arm_neon_ok object { #include "arm_neon.h" int dummy; + #if __ARM_ARCH < 7 + #error Architecture too old for NEON. + #endif } "$flags"] } { set et_arm_neon_flags $flags return 1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2014-12-23 17:32 ` Andrew Stubbs @ 2015-01-12 14:14 ` Andrew Stubbs 2015-01-12 14:27 ` Ramana Radhakrishnan 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2015-01-12 14:14 UTC (permalink / raw) To: ramrad01, Kyrill Tkachov; +Cc: James Greenhalgh, Richard Earnshaw, gcc-patches Ping. On 23/12/14 16:46, Andrew Stubbs wrote: > On 03/12/14 15:03, Andrew Stubbs wrote: >>> The tools have always allowed us to drop down the arch to >>> march=armv5te along with using -mfpu=neon. We are now changing command >>> line behaviour, so an inform in terms of diagnostics to the user would >>> be useful as it states that we don't really have mfpu=neon generating >>> neon code any more because of this particular case. If we are to do >>> this then the original patch is probably not enough as it then doesn't >>> handle the case of TARGET_VFP3 / TARGET_VFP5 / TARGET_NEON_FP16 / >>> TARGET_FP16 / TARGET_FPU_ARMV8 etc. etc. etc. >> >> I'll take a look at those shortly. > > Or, not so shortly. > > It seems that, on ARM, the arch/CPU setting is basically orthogonal to > the FPU setting, and the compiler doesn't even try to match the one to > the other. The assembler does the same. In fact, the testcases that > James refers to, that have hard-coded -march options, really do emit > armv4 code with Neon, say, although most probably don't have > vectorizable code. They only work because they're most likely executed > on Neon hardware. > > This means that there's no obvious patch to fix the issue, in the > compiler. It's easy to reject Neon for pre-v7 CPUs, but that has > consequences, as we've seen. We'd have to have a table of fall-back FPUs > or something, and that doesn't seem straight-forward (and anyway, I'm > not sure what values to enter into that table). > > So, I've attacked the problem from the other end, and updated the > compiler check. > > OK to commit? > > Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2015-01-12 14:14 ` Andrew Stubbs @ 2015-01-12 14:27 ` Ramana Radhakrishnan 2015-01-13 21:08 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Ramana Radhakrishnan @ 2015-01-12 14:27 UTC (permalink / raw) To: Andrew Stubbs, Kyrylo Tkachov Cc: James Greenhalgh, Richard Earnshaw, gcc-patches Sorry about the slow response- have been on holiday and still catching up on email. On 12/01/15 13:16, Andrew Stubbs wrote: > Ping. > > On 23/12/14 16:46, Andrew Stubbs wrote: >> On 03/12/14 15:03, Andrew Stubbs wrote: >>>> The tools have always allowed us to drop down the arch to >>>> march=armv5te along with using -mfpu=neon. We are now changing command >>>> line behaviour, so an inform in terms of diagnostics to the user would >>>> be useful as it states that we don't really have mfpu=neon generating >>>> neon code any more because of this particular case. If we are to do >>>> this then the original patch is probably not enough as it then doesn't >>>> handle the case of TARGET_VFP3 / TARGET_VFP5 / TARGET_NEON_FP16 / >>>> TARGET_FP16 / TARGET_FPU_ARMV8 etc. etc. etc. >>> >>> I'll take a look at those shortly. >> >> Or, not so shortly. >> Sigh. >> It seems that, on ARM, the arch/CPU setting is basically orthogonal to >> the FPU setting, and the compiler doesn't even try to match the one to >> the other. The assembler does the same. In fact, the testcases that >> James refers to, that have hard-coded -march options, really do emit >> armv4 code with Neon, say, although most probably don't have >> vectorizable code. They only work because they're most likely executed >> on Neon hardware. Yes - though I'm surprised as I run an armv5te soft float only test run once a while on my Sheevaplug and don't see these issues. Maybe others do. >> >> This means that there's no obvious patch to fix the issue, in the >> compiler. It's easy to reject Neon for pre-v7 CPUs, but that has >> consequences, as we've seen. We'd have to have a table of fall-back FPUs >> or something, and that doesn't seem straight-forward (and anyway, I'm >> not sure what values to enter into that table). >> >> So, I've attacked the problem from the other end, and updated the >> compiler check. >> >> OK to commit? In principle ok, but I'd like a comment in there explaining why we've done this. Can you also post under what configurations these have been tested ? Ramana >> >> Andrew > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2015-01-12 14:27 ` Ramana Radhakrishnan @ 2015-01-13 21:08 ` Andrew Stubbs 2015-01-14 9:06 ` Ramana Radhakrishnan 0 siblings, 1 reply; 25+ messages in thread From: Andrew Stubbs @ 2015-01-13 21:08 UTC (permalink / raw) To: Ramana Radhakrishnan, Kyrylo Tkachov Cc: James Greenhalgh, Richard Earnshaw, gcc-patches [-- Attachment #1: Type: text/plain, Size: 303 bytes --] On 12/01/15 13:50, Ramana Radhakrishnan wrote: > In principle ok, but I'd like a comment in there explaining why we've > done this. Can you also post under what configurations these have been > tested ? Is this better? I tested it by running the vect.exp tests with a variety of -mcpu flags. Andrew [-- Attachment #2: arm-neon-ok-3.patch --] [-- Type: text/x-patch, Size: 822 bytes --] 2015-01-13 Andrew Stubbs <ams@codesourcery.com> gcc/testsuite/ * lib/target-supports.exp (check_effective_target_arm_neon_ok_nocache): Don't try to test Neon on ARM architures before v7. Index: gcc/testsuite/lib/target-supports.exp =================================================================== --- gcc/testsuite/lib/target-supports.exp (revision 219058) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -2565,6 +2565,11 @@ if { [check_no_compiler_messages_nocache arm_neon_ok object { #include "arm_neon.h" int dummy; + /* Avoid the case where a test adds -mfpu=neon, but the toolchain is + configured for -mcpu=arm926ej-s, for example. */ + #if __ARM_ARCH < 7 + #error Architecture too old for NEON. + #endif } "$flags"] } { set et_arm_neon_flags $flags return 1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2015-01-13 21:08 ` Andrew Stubbs @ 2015-01-14 9:06 ` Ramana Radhakrishnan 2015-01-14 15:25 ` Andrew Stubbs 0 siblings, 1 reply; 25+ messages in thread From: Ramana Radhakrishnan @ 2015-01-14 9:06 UTC (permalink / raw) To: Andrew Stubbs, Kyrylo Tkachov Cc: James Greenhalgh, Richard Earnshaw, gcc-patches On 13/01/15 21:01, Andrew Stubbs wrote: > On 12/01/15 13:50, Ramana Radhakrishnan wrote: >> In principle ok, but I'd like a comment in there explaining why we've >> done this. Can you also post under what configurations these have been >> tested ? > > Is this better? > > I tested it by running the vect.exp tests with a variety of -mcpu flags. > > Andrew > Ok, that should be enough. Please watch out for any testing fallout this week. Ramana ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [arm][patch] fix arm_neon_ok check on !arm_arch7 2015-01-14 9:06 ` Ramana Radhakrishnan @ 2015-01-14 15:25 ` Andrew Stubbs 0 siblings, 0 replies; 25+ messages in thread From: Andrew Stubbs @ 2015-01-14 15:25 UTC (permalink / raw) To: Ramana Radhakrishnan, Kyrylo Tkachov Cc: James Greenhalgh, Richard Earnshaw, gcc-patches On 14/01/15 08:21, Ramana Radhakrishnan wrote: > Ok, that should be enough. Please watch out for any testing fallout this > week. Committed, thanks. Andrew ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2015-01-14 15:09 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-13 21:39 [arm][patch] fix arm_neon_ok check on !arm_arch7 Andrew Stubbs 2014-09-15 9:46 ` Richard Earnshaw 2014-09-15 10:56 ` Andrew Stubbs 2014-09-15 13:30 ` Richard Earnshaw 2014-09-17 11:00 ` Andrew Stubbs 2014-09-23 8:27 ` James Greenhalgh 2014-09-23 15:22 ` Stubbs, Andrew 2014-10-15 16:37 ` Jiong Wang 2014-10-15 16:59 ` Andrew Stubbs 2014-10-16 13:53 ` Jiong Wang 2014-11-07 10:35 ` Andrew Stubbs 2014-11-14 11:17 ` Andrew Stubbs 2014-11-26 13:11 ` Andrew Stubbs 2014-11-27 17:28 ` Mike Stump 2014-11-27 19:29 ` Andrew Stubbs 2014-11-28 8:42 ` Mike Stump 2014-12-02 14:01 ` Kyrill Tkachov 2014-12-02 21:45 ` Ramana Radhakrishnan 2014-12-03 15:03 ` Andrew Stubbs 2014-12-23 17:32 ` Andrew Stubbs 2015-01-12 14:14 ` Andrew Stubbs 2015-01-12 14:27 ` Ramana Radhakrishnan 2015-01-13 21:08 ` Andrew Stubbs 2015-01-14 9:06 ` Ramana Radhakrishnan 2015-01-14 15:25 ` Andrew Stubbs
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).