| Message ID | CAKdteOafYBWCDEVDDgLwaZ3ebyResUcf02c+2kT6bbn-butk2A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Christophe, On 12/17/2015 11:17 PM, Christophe Lyon wrote: > Hi, > > Here is an updated version of this patch. > I did test it with > -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in > addition to my usual set of options. > > Compared to the previous version: > - I added some doc in sourcebuild.texi > - I no longer modify arm_vfp_ok... > - I replaced all uses of arm_vfp with the new arm_fp because I found > that the existing tests do not actually need to pass -mfpu=vfp: this > is implicitly set as the default when using -mfloat-abi={softfp|hard} > - I chose not to remove arm_vfp_ok because we may need it in the > future, if a test really needs vfp (as opposed to neon for instance) > - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp > via pragma instead, so that the next pragma fpu > fpu=crypto-neon-fp-armv8 is always compatible, regardless of the > command-line options/default fpu > - same for attr-neon2.c and attr-neon3.c > - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c, > vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c, > vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp > effective target instead of arm_vfp. This is so that they don't need > to use -mfpu=vfp and can use the new dg-add-options arm_fp > > The validation results show (in addition to what I originally reported): > - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895. > - depending on the GCC configuration (e.g. --with-fpu=neon) > attr-neon3.c may fail. This is PR68896. Those last tests are invalid, as they use simd builtin type declaration in nosimd context (that was my fault originally). I'm rewriting those and a few others to be compliant. I'm also working on improving the reporting for invalid uses so you don't get this ICE anymore. > > OK? > > Christophe > > 2015-12-17 Christophe Lyon <christophe.lyon@linaro.org> > > * doc/sourcebuild.texi (arm_fp_ok): Document new entry. > (arm_fp): Likewise. > * lib/target-supports.exp > (check_effective_target_arm_fp_ok_nocache): New. > (check_effective_target_arm_fp_ok): New. > (add_options_for_arm_fp): New. > (check_effective_target_arm_crypto_ok_nocache): Require > target_arm_v8_neon_ok instead of arm32. > (check_effective_target_arm_crypto_pragma_ok_nocache): New. > (check_effective_target_arm_crypto_pragma_ok): New. > (add_options_for_arm_vfp): New. > * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective > target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective > target instead. Force initial fpu to vfp. > * gcc.target/arm/attr-neon-builtin-fail.c: Do not force > -mfloat-abi=softfp, use arm_fp_ok effective target instead. > * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok > dependency. > * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, > use arm_vfp effective target instead. Force initial fpu to vfp. > * gcc.target/arm/attr-neon3.c: Likewise. > * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of > arm_vfp_ok. > * gcc.target/arm/unsigned-float.c: Likewise. > * gcc.target/arm/vfp-1.c: Likewise. > * gcc.target/arm/vfp-ldmdbd.c: Likewise. > * gcc.target/arm/vfp-ldmdbs.c: Likewise. > * gcc.target/arm/vfp-ldmiad.c: Likewise. > * gcc.target/arm/vfp-ldmias.c: Likewise. > * gcc.target/arm/vfp-stmdbd.c: Likewise. > * gcc.target/arm/vfp-stmdbs.c: Likewise. > * gcc.target/arm/vfp-stmiad.c: Likewise. > * gcc.target/arm/vfp-stmias.c: Likewise. > * gcc.target/arm/vnmul-1.c: Likewise. > * gcc.target/arm/vnmul-2.c: Likewise. > * gcc.target/arm/vnmul-3.c: Likewise. > * gcc.target/arm/vnmul-4.c: Likewise. > > > > On 10 December 2015 at 20:52, Christophe Lyon > <christophe.lyon@linaro.org> wrote: >> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: >>> >>> On 10/12/15 13:04, Christophe Lyon wrote: >>>> >>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>> wrote: >>>>> >>>>> Hi Christophe, >>>>> >>>>> >>>>> On 08/12/15 11:18, Christophe Lyon wrote: >>>>>> >>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>>>> wrote: >>>>>>> >>>>>>> Hi Christophe, >>>>>>> >>>>>>> >>>>>>> On 27/11/15 13:00, Christophe Lyon wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> After the recent commits from Christian adding target attributes >>>>>>>> support for ARM FPU settings, I've noticed that some of the tests >>>>>>>> were failing because of incorrect assumptions wrt to the default >>>>>>>> cpu/fpu/float-abi of the compiler. >>>>>>>> >>>>>>>> This patch fixes the problems I've noticed in the following way: >>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts >>>>>>>> when gcc is configured --with-float=hard >>>>>>>> >>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi >>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not >>>>>>>> defined >>>>>>>> >>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu >>>>>>>> setting >>>>>>>> >>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that >>>>>>>> setting this fpu via a pragma is actually supported by the current >>>>>>>> "multilib". This is different from checking the command-line option >>>>>>>> because the pragma might conflict with the command-line options in >>>>>>>> use. >>>>>>>> >>>>>>>> The updates in the testcases are as follows: >>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not >>>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp >>>>>>>> options/effective_target. This is needed if gcc has been configured >>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would >>>>>>>> conflict. >>>>>>>> >>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate >>>>>>>> float-abi setting. Enforcing fpu is not needed here. >>>>>>>> >>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was >>>>>>>> not necessary to make the test pass in my testing. On second thought, >>>>>>>> I'm wondering whether I should leave it and make the test unsupported >>>>>>>> in more cases (such as when forcing -march=armv5t, although it does >>>>>>>> pass with this patch) >>>>>>>> >>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi >>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the >>>>>>>> pragma target fpu=neon (for instance if the toolchain default is >>>>>>>> neon-fp16) >>>>>>>> >>>>>>>> - attr-neon3.c: similar >>>>>>>> >>>>>>>> Tested on a variety of configurations, see: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html >>>>>>>> >>>>>>>> Note that the regressions reported fall into 3 categories: >>>>>>>> - when forcing march=armv5t: tests are now unsupported because I >>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32. >>>>>>>> >>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12 >>>>>>>> to 14 and is thus seen as a regression + one improvement >>>>>>>> >>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for >>>>>>>> which I need to post a bugzilla. >>>>>>>> >>>>>>>> >>>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like >>>>>>>> conditions. I'm confident that I'm still missing some combinations :-) >>>>>>>> >>>>>>>> And with new target attributes coming, new architectures etc... all >>>>>>>> this logic is likely to become even more complex. >>>>>>>> >>>>>>>> That being said, OK for trunk? >>>>>>> >>>>>>> >>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite >>>>>>> at some point to make it more robust with respect to the tons of >>>>>>> multilib >>>>>>> options and configurations we can have for arm. It's becoming very >>>>>>> frustrating >>>>>>> to test feature-specific stuff :( >>>>>>> >>>>>>> This is ok in the meantime. >>>>>>> Sorry for the delay. >>>>>>> >>>>>> Thanks for the approval, glad to see I'm not the only one willing to see >>>>>> improvements in this area :) >>>>>> >>>>>> Committed as r231403. >>>>> >>>>> >>>>> With this patch we're seeing legitimate PASSes go to NA. >>>>> For example: >>>>> >>>>> gcc.target/arm/vfp-1.c >>>>> gcc.target/arm/vfp-ldmdbs.c >>>>> and other vfp tests in arm.exp. >>>>> This is, for example, for the >>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard >>>>> >>>> Hmm I'm attempting to cover such a configuration in my matrix of >>>> validations: >>>> >>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html >>>> >>>> The difference is that I use similar flags at GCC configure time, while >>>> you >>>> override them when running the testsuite: >>>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57 >>>> --with-fpu=crypto-neon-fp-armv8 >>>> >>>> I this case, I do not see the regressions. >>> >>> >>> My gcc is arm-none-eabi configured with >>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4 >>> --with-float=hard >>> >>>>> I suspect under your new predicates they'd need to be changed to check >>>>> for >>>>> arm_fp_ok rather than arm_vfp_ok. >>>> >>>> Probably, yes. >>>> >>> >>> In the test log where it checks the effective target it fails due to: >>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined >>> it's compiling the check with >>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o >>> arm_vfp_ok27268.o arm_vfp_ok27268.c >>> >>>>> We want to avoid removing any PASSes. >>>>> Can you please test some more to make sure we don't remove any legitimate >>>>> passes with your patch? >>>> >>>> Is that the only combination in which you saw less PASSes? >>> >>> >>> That's the one that was brought to my attention, but I think the issue here >>> is just the >>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set >>> -mfpu=vfp explicitly. >>> They appear unsupported if testing with an explicit neon option in mfpu, I >>> think. >>> >>>>> Also, Ramana reminded me offline that any new predicates in >>>>> target-supports.exp >>>>> need documenting in sourcebuild.txt. >>>>> >>>>> In light of that, can you please revert your patch and address the issues >>>>> above >>>>> so that we can be confident we don't lose existing legitimate test >>>>> coverage? >>>> >>>> OK. >>>> >>>>> Sorry for not catching these sooner. >>>> >>>> No problem, there are so many combinations. >>>> I'm not sure how to define a reasonable set. >>> >>> >>> So, I think the action plan here is to audit the tests that need to be >>> changed >>> to arm_fp_ok and add the documentation for the new predicate checks. >>> >> >> I came up with a new version where I now use only the new arm_fp_ok, so >> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok. >> >> However, I'm still not happy with this version because it has problems with >> a few tests that use target attributes such as attr-crypto.c, which starts with: >> #pragma GCC target ("fpu=crypto-neon-fp-armv8") >> >> This pragma fails if the compiler+options have set an incompatible >> fpu before processing the pragma. >> >> So for instance, if gcc has been configured --with-fpu=neon, compiling the test >> with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8 >> because the latter has fp16) >> >> Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp, >> we end up compiling with -mfpu=vfp and the pragma passes. >> >> But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8 >> as you do, the effective_target arm_vfp_ok test now fails because it is >> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines >> __ARM_NEON_FP >> >> In short, the problem is how to make sure that the fpu setting is always >> compatible with the pragma, whatever the gcc configuration and multilib >> options used. >> >> Thanks, >> >> Christophe. >> >>> Thanks, >>> Kyrill >>> >>> >>>> Christophe. >>>> >>>>> Kyrill >>>>> >>>>> >>>>>> Christophe. >>>>>> >>>>>>> Thanks for handling this! >>>>>>> Kyrill >>>>>>> >>>>>>> >>>>>>>> Christophe >>>>>>>> >>>>>>>> >>>>>>>> 2015-11-27 Christophe Lyon <christophe.lyon@linaro.org> >>>>>>>> >>>>>>>> * lib/target-supports.exp >>>>>>>> (check_effective_target_arm_vfp_ok_nocache): New. >>>>>>>> (check_effective_target_arm_vfp_ok): Call the new >>>>>>>> check_effective_target_arm_vfp_ok_nocache function. >>>>>>>> (check_effective_target_arm_fp_ok_nocache): New. >>>>>>>> (check_effective_target_arm_fp_ok): New. >>>>>>>> (add_options_for_arm_fp): New. >>>>>>>> (check_effective_target_arm_crypto_ok_nocache): Require >>>>>>>> target_arm_v8_neon_ok instead of arm32. >>>>>>>> (check_effective_target_arm_crypto_pragma_ok_nocache): New. >>>>>>>> (check_effective_target_arm_crypto_pragma_ok): New. >>>>>>>> (add_options_for_arm_vfp): New. >>>>>>>> * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok >>>>>>>> effective >>>>>>>> target. Do not force -mfloat-abi=softfp, use arm_vfp effective >>>>>>>> target instead. >>>>>>>> * gcc.target/arm/attr-neon-builtin-fail.c: Do not force >>>>>>>> -mfloat-abi=softfp, use arm_fp effective target instead. >>>>>>>> * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok >>>>>>>> dependency. >>>>>>>> * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, >>>>>>>> use arm_vfp effective target instead. >>>>>>>> * gcc.target/arm/attr-neon3.c: Likewise. >>>>>>> >>>>>>> >>>
Hi Christophe, On 17/12/15 22:17, Christophe Lyon wrote: > Hi, > > Here is an updated version of this patch. > I did test it with > -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in > addition to my usual set of options. > > Compared to the previous version: > - I added some doc in sourcebuild.texi > - I no longer modify arm_vfp_ok... > - I replaced all uses of arm_vfp with the new arm_fp because I found > that the existing tests do not actually need to pass -mfpu=vfp: this > is implicitly set as the default when using -mfloat-abi={softfp|hard} > - I chose not to remove arm_vfp_ok because we may need it in the > future, if a test really needs vfp (as opposed to neon for instance) > - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp > via pragma instead, so that the next pragma fpu > fpu=crypto-neon-fp-armv8 is always compatible, regardless of the > command-line options/default fpu > - same for attr-neon2.c and attr-neon3.c > - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c, > vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c, > vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp > effective target instead of arm_vfp. This is so that they don't need > to use -mfpu=vfp and can use the new dg-add-options arm_fp > > The validation results show (in addition to what I originally reported): > - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895. > - depending on the GCC configuration (e.g. --with-fpu=neon) > attr-neon3.c may fail. This is PR68896. > > OK? Thanks for following up on this. I think you also need to document the new arm_crypto_pragma_ok. Kyrill > Christophe > > 2015-12-17 Christophe Lyon <christophe.lyon@linaro.org> > > * doc/sourcebuild.texi (arm_fp_ok): Document new entry. > (arm_fp): Likewise. > * lib/target-supports.exp > (check_effective_target_arm_fp_ok_nocache): New. > (check_effective_target_arm_fp_ok): New. > (add_options_for_arm_fp): New. > (check_effective_target_arm_crypto_ok_nocache): Require > target_arm_v8_neon_ok instead of arm32. > (check_effective_target_arm_crypto_pragma_ok_nocache): New. > (check_effective_target_arm_crypto_pragma_ok): New. > (add_options_for_arm_vfp): New. > * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective > target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective > target instead. Force initial fpu to vfp. > * gcc.target/arm/attr-neon-builtin-fail.c: Do not force > -mfloat-abi=softfp, use arm_fp_ok effective target instead. > * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok > dependency. > * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, > use arm_vfp effective target instead. Force initial fpu to vfp. > * gcc.target/arm/attr-neon3.c: Likewise. > * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of > arm_vfp_ok. > * gcc.target/arm/unsigned-float.c: Likewise. > * gcc.target/arm/vfp-1.c: Likewise. > * gcc.target/arm/vfp-ldmdbd.c: Likewise. > * gcc.target/arm/vfp-ldmdbs.c: Likewise. > * gcc.target/arm/vfp-ldmiad.c: Likewise. > * gcc.target/arm/vfp-ldmias.c: Likewise. > * gcc.target/arm/vfp-stmdbd.c: Likewise. > * gcc.target/arm/vfp-stmdbs.c: Likewise. > * gcc.target/arm/vfp-stmiad.c: Likewise. > * gcc.target/arm/vfp-stmias.c: Likewise. > * gcc.target/arm/vnmul-1.c: Likewise. > * gcc.target/arm/vnmul-2.c: Likewise. > * gcc.target/arm/vnmul-3.c: Likewise. > * gcc.target/arm/vnmul-4.c: Likewise. > > > > On 10 December 2015 at 20:52, Christophe Lyon > <christophe.lyon@linaro.org> wrote: >> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: >>> On 10/12/15 13:04, Christophe Lyon wrote: >>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>> wrote: >>>>> Hi Christophe, >>>>> >>>>> >>>>> On 08/12/15 11:18, Christophe Lyon wrote: >>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>>>> wrote: >>>>>>> Hi Christophe, >>>>>>> >>>>>>> >>>>>>> On 27/11/15 13:00, Christophe Lyon wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> After the recent commits from Christian adding target attributes >>>>>>>> support for ARM FPU settings, I've noticed that some of the tests >>>>>>>> were failing because of incorrect assumptions wrt to the default >>>>>>>> cpu/fpu/float-abi of the compiler. >>>>>>>> >>>>>>>> This patch fixes the problems I've noticed in the following way: >>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts >>>>>>>> when gcc is configured --with-float=hard >>>>>>>> >>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi >>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not >>>>>>>> defined >>>>>>>> >>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu >>>>>>>> setting >>>>>>>> >>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that >>>>>>>> setting this fpu via a pragma is actually supported by the current >>>>>>>> "multilib". This is different from checking the command-line option >>>>>>>> because the pragma might conflict with the command-line options in >>>>>>>> use. >>>>>>>> >>>>>>>> The updates in the testcases are as follows: >>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not >>>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp >>>>>>>> options/effective_target. This is needed if gcc has been configured >>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would >>>>>>>> conflict. >>>>>>>> >>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate >>>>>>>> float-abi setting. Enforcing fpu is not needed here. >>>>>>>> >>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was >>>>>>>> not necessary to make the test pass in my testing. On second thought, >>>>>>>> I'm wondering whether I should leave it and make the test unsupported >>>>>>>> in more cases (such as when forcing -march=armv5t, although it does >>>>>>>> pass with this patch) >>>>>>>> >>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi >>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the >>>>>>>> pragma target fpu=neon (for instance if the toolchain default is >>>>>>>> neon-fp16) >>>>>>>> >>>>>>>> - attr-neon3.c: similar >>>>>>>> >>>>>>>> Tested on a variety of configurations, see: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html >>>>>>>> >>>>>>>> Note that the regressions reported fall into 3 categories: >>>>>>>> - when forcing march=armv5t: tests are now unsupported because I >>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32. >>>>>>>> >>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12 >>>>>>>> to 14 and is thus seen as a regression + one improvement >>>>>>>> >>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for >>>>>>>> which I need to post a bugzilla. >>>>>>>> >>>>>>>> >>>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like >>>>>>>> conditions. I'm confident that I'm still missing some combinations :-) >>>>>>>> >>>>>>>> And with new target attributes coming, new architectures etc... all >>>>>>>> this logic is likely to become even more complex. >>>>>>>> >>>>>>>> That being said, OK for trunk? >>>>>>> >>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite >>>>>>> at some point to make it more robust with respect to the tons of >>>>>>> multilib >>>>>>> options and configurations we can have for arm. It's becoming very >>>>>>> frustrating >>>>>>> to test feature-specific stuff :( >>>>>>> >>>>>>> This is ok in the meantime. >>>>>>> Sorry for the delay. >>>>>>> >>>>>> Thanks for the approval, glad to see I'm not the only one willing to see >>>>>> improvements in this area :) >>>>>> >>>>>> Committed as r231403. >>>>> >>>>> With this patch we're seeing legitimate PASSes go to NA. >>>>> For example: >>>>> >>>>> gcc.target/arm/vfp-1.c >>>>> gcc.target/arm/vfp-ldmdbs.c >>>>> and other vfp tests in arm.exp. >>>>> This is, for example, for the >>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard >>>>> >>>> Hmm I'm attempting to cover such a configuration in my matrix of >>>> validations: >>>> >>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html >>>> >>>> The difference is that I use similar flags at GCC configure time, while >>>> you >>>> override them when running the testsuite: >>>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57 >>>> --with-fpu=crypto-neon-fp-armv8 >>>> >>>> I this case, I do not see the regressions. >>> >>> My gcc is arm-none-eabi configured with >>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4 >>> --with-float=hard >>> >>>>> I suspect under your new predicates they'd need to be changed to check >>>>> for >>>>> arm_fp_ok rather than arm_vfp_ok. >>>> Probably, yes. >>>> >>> In the test log where it checks the effective target it fails due to: >>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined >>> it's compiling the check with >>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o >>> arm_vfp_ok27268.o arm_vfp_ok27268.c >>> >>>>> We want to avoid removing any PASSes. >>>>> Can you please test some more to make sure we don't remove any legitimate >>>>> passes with your patch? >>>> Is that the only combination in which you saw less PASSes? >>> >>> That's the one that was brought to my attention, but I think the issue here >>> is just the >>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set >>> -mfpu=vfp explicitly. >>> They appear unsupported if testing with an explicit neon option in mfpu, I >>> think. >>> >>>>> Also, Ramana reminded me offline that any new predicates in >>>>> target-supports.exp >>>>> need documenting in sourcebuild.txt. >>>>> >>>>> In light of that, can you please revert your patch and address the issues >>>>> above >>>>> so that we can be confident we don't lose existing legitimate test >>>>> coverage? >>>> OK. >>>> >>>>> Sorry for not catching these sooner. >>>> No problem, there are so many combinations. >>>> I'm not sure how to define a reasonable set. >>> >>> So, I think the action plan here is to audit the tests that need to be >>> changed >>> to arm_fp_ok and add the documentation for the new predicate checks. >>> >> I came up with a new version where I now use only the new arm_fp_ok, so >> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok. >> >> However, I'm still not happy with this version because it has problems with >> a few tests that use target attributes such as attr-crypto.c, which starts with: >> #pragma GCC target ("fpu=crypto-neon-fp-armv8") >> >> This pragma fails if the compiler+options have set an incompatible >> fpu before processing the pragma. >> >> So for instance, if gcc has been configured --with-fpu=neon, compiling the test >> with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8 >> because the latter has fp16) >> >> Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp, >> we end up compiling with -mfpu=vfp and the pragma passes. >> >> But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8 >> as you do, the effective_target arm_vfp_ok test now fails because it is >> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines >> __ARM_NEON_FP >> >> In short, the problem is how to make sure that the fpu setting is always >> compatible with the pragma, whatever the gcc configuration and multilib >> options used. >> >> Thanks, >> >> Christophe. >> >>> Thanks, >>> Kyrill >>> >>> >>>> Christophe. >>>> >>>>> Kyrill >>>>> >>>>> >>>>>> Christophe. >>>>>> >>>>>>> Thanks for handling this! >>>>>>> Kyrill >>>>>>> >>>>>>> >>>>>>>> Christophe >>>>>>>> >>>>>>>> >>>>>>>> 2015-11-27 Christophe Lyon <christophe.lyon@linaro.org> >>>>>>>> >>>>>>>> * lib/target-supports.exp >>>>>>>> (check_effective_target_arm_vfp_ok_nocache): New. >>>>>>>> (check_effective_target_arm_vfp_ok): Call the new >>>>>>>> check_effective_target_arm_vfp_ok_nocache function. >>>>>>>> (check_effective_target_arm_fp_ok_nocache): New. >>>>>>>> (check_effective_target_arm_fp_ok): New. >>>>>>>> (add_options_for_arm_fp): New. >>>>>>>> (check_effective_target_arm_crypto_ok_nocache): Require >>>>>>>> target_arm_v8_neon_ok instead of arm32. >>>>>>>> (check_effective_target_arm_crypto_pragma_ok_nocache): New. >>>>>>>> (check_effective_target_arm_crypto_pragma_ok): New. >>>>>>>> (add_options_for_arm_vfp): New. >>>>>>>> * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok >>>>>>>> effective >>>>>>>> target. Do not force -mfloat-abi=softfp, use arm_vfp effective >>>>>>>> target instead. >>>>>>>> * gcc.target/arm/attr-neon-builtin-fail.c: Do not force >>>>>>>> -mfloat-abi=softfp, use arm_fp effective target instead. >>>>>>>> * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok >>>>>>>> dependency. >>>>>>>> * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, >>>>>>>> use arm_vfp effective target instead. >>>>>>>> * gcc.target/arm/attr-neon3.c: Likewise. >>>>>>>
On 18 December 2015 at 15:16, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > Hi Christophe, > > > On 17/12/15 22:17, Christophe Lyon wrote: >> >> Hi, >> >> Here is an updated version of this patch. >> I did test it with >> -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in >> addition to my usual set of options. >> >> Compared to the previous version: >> - I added some doc in sourcebuild.texi >> - I no longer modify arm_vfp_ok... >> - I replaced all uses of arm_vfp with the new arm_fp because I found >> that the existing tests do not actually need to pass -mfpu=vfp: this >> is implicitly set as the default when using -mfloat-abi={softfp|hard} >> - I chose not to remove arm_vfp_ok because we may need it in the >> future, if a test really needs vfp (as opposed to neon for instance) >> - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp >> via pragma instead, so that the next pragma fpu >> fpu=crypto-neon-fp-armv8 is always compatible, regardless of the >> command-line options/default fpu >> - same for attr-neon2.c and attr-neon3.c >> - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c, >> vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c, >> vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp >> effective target instead of arm_vfp. This is so that they don't need >> to use -mfpu=vfp and can use the new dg-add-options arm_fp >> >> The validation results show (in addition to what I originally reported): >> - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895. >> - depending on the GCC configuration (e.g. --with-fpu=neon) >> attr-neon3.c may fail. This is PR68896. >> >> OK? > > > Thanks for following up on this. > I think you also need to document the new arm_crypto_pragma_ok. > Indeed, I forgot it. Here is a new version of the patch with a few words added to document this function. I did not modify the testcase after Christian's comments and PR68934: my understanding is that the testscase are valid after all and Christian is working on fixing the ICE. 2016-01-04 Christophe Lyon <christophe.lyon@linaro.org> * doc/sourcebuild.texi (arm_crypto_pragma_ok): Document new entry. (arm_fp_ok): Likewise. (arm_fp): Likewise. (arm_crypto): Likewise. * lib/target-supports.exp (check_effective_target_arm_fp_ok_nocache): New. (check_effective_target_arm_fp_ok): New. (add_options_for_arm_fp): New. (check_effective_target_arm_crypto_ok_nocache): Require target_arm_v8_neon_ok instead of arm32. (check_effective_target_arm_crypto_pragma_ok_nocache): New. (check_effective_target_arm_crypto_pragma_ok): New. (add_options_for_arm_vfp): New. * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective target instead. Force initial fpu to vfp. * gcc.target/arm/attr-neon-builtin-fail.c: Do not force -mfloat-abi=softfp, use arm_fp_ok effective target instead. * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok dependency. * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, use arm_vfp effective target instead. Force initial fpu to vfp. * gcc.target/arm/attr-neon3.c: Likewise. * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of arm_vfp_ok. * gcc.target/arm/unsigned-float.c: Likewise. * gcc.target/arm/vfp-1.c: Likewise. * gcc.target/arm/vfp-ldmdbd.c: Likewise. * gcc.target/arm/vfp-ldmdbs.c: Likewise. * gcc.target/arm/vfp-ldmiad.c: Likewise. * gcc.target/arm/vfp-ldmias.c: Likewise. * gcc.target/arm/vfp-stmdbd.c: Likewise. * gcc.target/arm/vfp-stmdbs.c: Likewise. * gcc.target/arm/vfp-stmiad.c: Likewise. * gcc.target/arm/vfp-stmias.c: Likewise. * gcc.target/arm/vnmul-1.c: Likewise. * gcc.target/arm/vnmul-2.c: Likewise. * gcc.target/arm/vnmul-3.c: Likewise. * gcc.target/arm/vnmul-4.c: Likewise. OK? Christophe. > Kyrill > > >> Christophe >> >> 2015-12-17 Christophe Lyon <christophe.lyon@linaro.org> >> >> * doc/sourcebuild.texi (arm_fp_ok): Document new entry. >> (arm_fp): Likewise. >> * lib/target-supports.exp >> (check_effective_target_arm_fp_ok_nocache): New. >> (check_effective_target_arm_fp_ok): New. >> (add_options_for_arm_fp): New. >> (check_effective_target_arm_crypto_ok_nocache): Require >> target_arm_v8_neon_ok instead of arm32. >> (check_effective_target_arm_crypto_pragma_ok_nocache): New. >> (check_effective_target_arm_crypto_pragma_ok): New. >> (add_options_for_arm_vfp): New. >> * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective >> target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective >> target instead. Force initial fpu to vfp. >> * gcc.target/arm/attr-neon-builtin-fail.c: Do not force >> -mfloat-abi=softfp, use arm_fp_ok effective target instead. >> * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok >> dependency. >> * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, >> use arm_vfp effective target instead. Force initial fpu to vfp. >> * gcc.target/arm/attr-neon3.c: Likewise. >> * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of >> arm_vfp_ok. >> * gcc.target/arm/unsigned-float.c: Likewise. >> * gcc.target/arm/vfp-1.c: Likewise. >> * gcc.target/arm/vfp-ldmdbd.c: Likewise. >> * gcc.target/arm/vfp-ldmdbs.c: Likewise. >> * gcc.target/arm/vfp-ldmiad.c: Likewise. >> * gcc.target/arm/vfp-ldmias.c: Likewise. >> * gcc.target/arm/vfp-stmdbd.c: Likewise. >> * gcc.target/arm/vfp-stmdbs.c: Likewise. >> * gcc.target/arm/vfp-stmiad.c: Likewise. >> * gcc.target/arm/vfp-stmias.c: Likewise. >> * gcc.target/arm/vnmul-1.c: Likewise. >> * gcc.target/arm/vnmul-2.c: Likewise. >> * gcc.target/arm/vnmul-3.c: Likewise. >> * gcc.target/arm/vnmul-4.c: Likewise. >> >> >> >> On 10 December 2015 at 20:52, Christophe Lyon >> <christophe.lyon@linaro.org> wrote: >>> >>> On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>> wrote: >>>> >>>> On 10/12/15 13:04, Christophe Lyon wrote: >>>>> >>>>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>>> wrote: >>>>>> >>>>>> Hi Christophe, >>>>>> >>>>>> >>>>>> On 08/12/15 11:18, Christophe Lyon wrote: >>>>>>> >>>>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> Hi Christophe, >>>>>>>> >>>>>>>> >>>>>>>> On 27/11/15 13:00, Christophe Lyon wrote: >>>>>>>>> >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> After the recent commits from Christian adding target attributes >>>>>>>>> support for ARM FPU settings, I've noticed that some of the tests >>>>>>>>> were failing because of incorrect assumptions wrt to the default >>>>>>>>> cpu/fpu/float-abi of the compiler. >>>>>>>>> >>>>>>>>> This patch fixes the problems I've noticed in the following way: >>>>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts >>>>>>>>> when gcc is configured --with-float=hard >>>>>>>>> >>>>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi >>>>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not >>>>>>>>> defined >>>>>>>>> >>>>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu >>>>>>>>> setting >>>>>>>>> >>>>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that >>>>>>>>> setting this fpu via a pragma is actually supported by the current >>>>>>>>> "multilib". This is different from checking the command-line option >>>>>>>>> because the pragma might conflict with the command-line options in >>>>>>>>> use. >>>>>>>>> >>>>>>>>> The updates in the testcases are as follows: >>>>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not >>>>>>>>> conflict with the one forced by pragma. That's why I use the >>>>>>>>> arm_vfp >>>>>>>>> options/effective_target. This is needed if gcc has been configured >>>>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would >>>>>>>>> conflict. >>>>>>>>> >>>>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate >>>>>>>>> float-abi setting. Enforcing fpu is not needed here. >>>>>>>>> >>>>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it >>>>>>>>> was >>>>>>>>> not necessary to make the test pass in my testing. On second >>>>>>>>> thought, >>>>>>>>> I'm wondering whether I should leave it and make the test >>>>>>>>> unsupported >>>>>>>>> in more cases (such as when forcing -march=armv5t, although it does >>>>>>>>> pass with this patch) >>>>>>>>> >>>>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi >>>>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the >>>>>>>>> pragma target fpu=neon (for instance if the toolchain default is >>>>>>>>> neon-fp16) >>>>>>>>> >>>>>>>>> - attr-neon3.c: similar >>>>>>>>> >>>>>>>>> Tested on a variety of configurations, see: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html >>>>>>>>> >>>>>>>>> Note that the regressions reported fall into 3 categories: >>>>>>>>> - when forcing march=armv5t: tests are now unsupported because I >>>>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32. >>>>>>>>> >>>>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line >>>>>>>>> 12 >>>>>>>>> to 14 and is thus seen as a regression + one improvement >>>>>>>>> >>>>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for >>>>>>>>> which I need to post a bugzilla. >>>>>>>>> >>>>>>>>> >>>>>>>>> TBH, I'm a bit concerned by the complexity of all these >>>>>>>>> multilib-like >>>>>>>>> conditions. I'm confident that I'm still missing some combinations >>>>>>>>> :-) >>>>>>>>> >>>>>>>>> And with new target attributes coming, new architectures etc... all >>>>>>>>> this logic is likely to become even more complex. >>>>>>>>> >>>>>>>>> That being said, OK for trunk? >>>>>>>> >>>>>>>> >>>>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite >>>>>>>> at some point to make it more robust with respect to the tons of >>>>>>>> multilib >>>>>>>> options and configurations we can have for arm. It's becoming very >>>>>>>> frustrating >>>>>>>> to test feature-specific stuff :( >>>>>>>> >>>>>>>> This is ok in the meantime. >>>>>>>> Sorry for the delay. >>>>>>>> >>>>>>> Thanks for the approval, glad to see I'm not the only one willing to >>>>>>> see >>>>>>> improvements in this area :) >>>>>>> >>>>>>> Committed as r231403. >>>>>> >>>>>> >>>>>> With this patch we're seeing legitimate PASSes go to NA. >>>>>> For example: >>>>>> >>>>>> gcc.target/arm/vfp-1.c >>>>>> gcc.target/arm/vfp-ldmdbs.c >>>>>> and other vfp tests in arm.exp. >>>>>> This is, for example, for the >>>>>> >>>>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard >>>>>> >>>>> Hmm I'm attempting to cover such a configuration in my matrix of >>>>> validations: >>>>> >>>>> >>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html >>>>> >>>>> The difference is that I use similar flags at GCC configure time, while >>>>> you >>>>> override them when running the testsuite: >>>>> --target arm-none-linux-gnueabihf --with-mode=thum >>>>> --with-cpu=cortex-a57 >>>>> --with-fpu=crypto-neon-fp-armv8 >>>>> >>>>> I this case, I do not see the regressions. >>>> >>>> >>>> My gcc is arm-none-eabi configured with >>>> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4 >>>> --with-float=hard >>>> >>>>>> I suspect under your new predicates they'd need to be changed to check >>>>>> for >>>>>> arm_fp_ok rather than arm_vfp_ok. >>>>> >>>>> Probably, yes. >>>>> >>>> In the test log where it checks the effective target it fails due to: >>>> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined >>>> it's compiling the check with >>>> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o >>>> arm_vfp_ok27268.o arm_vfp_ok27268.c >>>> >>>>>> We want to avoid removing any PASSes. >>>>>> Can you please test some more to make sure we don't remove any >>>>>> legitimate >>>>>> passes with your patch? >>>>> >>>>> Is that the only combination in which you saw less PASSes? >>>> >>>> >>>> That's the one that was brought to my attention, but I think the issue >>>> here >>>> is just the >>>> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set >>>> -mfpu=vfp explicitly. >>>> They appear unsupported if testing with an explicit neon option in mfpu, >>>> I >>>> think. >>>> >>>>>> Also, Ramana reminded me offline that any new predicates in >>>>>> target-supports.exp >>>>>> need documenting in sourcebuild.txt. >>>>>> >>>>>> In light of that, can you please revert your patch and address the >>>>>> issues >>>>>> above >>>>>> so that we can be confident we don't lose existing legitimate test >>>>>> coverage? >>>>> >>>>> OK. >>>>> >>>>>> Sorry for not catching these sooner. >>>>> >>>>> No problem, there are so many combinations. >>>>> I'm not sure how to define a reasonable set. >>>> >>>> >>>> So, I think the action plan here is to audit the tests that need to be >>>> changed >>>> to arm_fp_ok and add the documentation for the new predicate checks. >>>> >>> I came up with a new version where I now use only the new arm_fp_ok, so >>> it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok. >>> >>> However, I'm still not happy with this version because it has problems >>> with >>> a few tests that use target attributes such as attr-crypto.c, which >>> starts with: >>> #pragma GCC target ("fpu=crypto-neon-fp-armv8") >>> >>> This pragma fails if the compiler+options have set an incompatible >>> fpu before processing the pragma. >>> >>> So for instance, if gcc has been configured --with-fpu=neon, compiling >>> the test >>> with no fpu option fails (neon is not compatible with >>> crypto-neon-fp-armv8 >>> because the latter has fp16) >>> >>> Now, if we use effective_target arm_vfp_ok + dg_option to force >>> -mfpu=vfp, >>> we end up compiling with -mfpu=vfp and the pragma passes. >>> >>> But if, in addition, we force the testflags to set >>> -mfpu=crypto-neon-fp-armv8 >>> as you do, the effective_target arm_vfp_ok test now fails because it is >>> compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines >>> __ARM_NEON_FP >>> >>> In short, the problem is how to make sure that the fpu setting is always >>> compatible with the pragma, whatever the gcc configuration and multilib >>> options used. >>> >>> Thanks, >>> >>> Christophe. >>> >>>> Thanks, >>>> Kyrill >>>> >>>> >>>>> Christophe. >>>>> >>>>>> Kyrill >>>>>> >>>>>> >>>>>>> Christophe. >>>>>>> >>>>>>>> Thanks for handling this! >>>>>>>> Kyrill >>>>>>>> >>>>>>>> >>>>>>>>> Christophe >>>>>>>>> >>>>>>>>> >>>>>>>>> 2015-11-27 Christophe Lyon <christophe.lyon@linaro.org> >>>>>>>>> >>>>>>>>> * lib/target-supports.exp >>>>>>>>> (check_effective_target_arm_vfp_ok_nocache): New. >>>>>>>>> (check_effective_target_arm_vfp_ok): Call the new >>>>>>>>> check_effective_target_arm_vfp_ok_nocache function. >>>>>>>>> (check_effective_target_arm_fp_ok_nocache): New. >>>>>>>>> (check_effective_target_arm_fp_ok): New. >>>>>>>>> (add_options_for_arm_fp): New. >>>>>>>>> (check_effective_target_arm_crypto_ok_nocache): Require >>>>>>>>> target_arm_v8_neon_ok instead of arm32. >>>>>>>>> (check_effective_target_arm_crypto_pragma_ok_nocache): New. >>>>>>>>> (check_effective_target_arm_crypto_pragma_ok): New. >>>>>>>>> (add_options_for_arm_vfp): New. >>>>>>>>> * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok >>>>>>>>> effective >>>>>>>>> target. Do not force -mfloat-abi=softfp, use arm_vfp >>>>>>>>> effective >>>>>>>>> target instead. >>>>>>>>> * gcc.target/arm/attr-neon-builtin-fail.c: Do not force >>>>>>>>> -mfloat-abi=softfp, use arm_fp effective target instead. >>>>>>>>> * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove >>>>>>>>> arm_neon_ok >>>>>>>>> dependency. >>>>>>>>> * gcc.target/arm/attr-neon2.c: Do not force >>>>>>>>> -mfloat-abi=softfp, >>>>>>>>> use arm_vfp effective target instead. >>>>>>>>> * gcc.target/arm/attr-neon3.c: Likewise. >>>>>>>> >>>>>>>> >
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index 61de4a5..facde56 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -1514,6 +1514,12 @@ ARM target generates 32-bit code. @item arm_eabi ARM target adheres to the ABI for the ARM Architecture. +@item arm_fp_ok +@anchor{arm_fp_ok} +ARM target defines @code{__ARM_FP} using @code{-mfloat-abi=softfp} or +equivalent options.. Some multilibs may be incompatible with these +options. + @item arm_hf_eabi ARM target adheres to the VFP and Advanced SIMD Register Arguments variant of the ABI for the ARM Architecture (as selected with @@ -2035,6 +2040,11 @@ The supported values of @var{feature} for directive @code{dg-add-options} are: @table @code +@item arm_fp +@code{__ARM_FP} definition. Only ARM targets support this feature, and only then +in certain modes; see the @ref{arm_fp_ok,,arm_fp_ok effective target +keyword}. + @item arm_neon NEON support. Only ARM targets support this feature, and only then in certain modes; see the @ref{arm_neon_ok,,arm_neon_ok effective target diff --git a/gcc/testsuite/gcc.target/arm/attr-crypto.c b/gcc/testsuite/gcc.target/arm/attr-crypto.c index 1db5984..3b4b7c5 100644 diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 4e349e9..6ab9270 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2721,6 +2721,47 @@ proc check_effective_target_arm_hard_vfp_ok { } { } } +# Return 1 if this is an ARM target defining __ARM_FP. We may need +# -mfloat-abi=softfp or equivalent options. Some multilibs may be +# incompatible with these options. Also set et_arm_fp_flags to the +# best options to add. + +proc check_effective_target_arm_fp_ok_nocache { } { + global et_arm_fp_flags + set et_arm_fp_flags "" + if { [check_effective_target_arm32] } { + foreach flags {"" "-mfloat-abi=softfp" "-mfloat-abi=hard"} { + if { [check_no_compiler_messages_nocache arm_fp_ok object { + #ifndef __ARM_FP + #error __ARM_FP not defined + #endif + } "$flags"] } { + set et_arm_fp_flags $flags + return 1 + } + } + } + + return 0 +} + +proc check_effective_target_arm_fp_ok { } { + return [check_cached_effective_target arm_fp_ok \ + check_effective_target_arm_fp_ok_nocache] +} + +# Add the options needed to define __ARM_FP. We need either +# -mfloat-abi=softfp or -mfloat-abi=hard, but if one is already +# specified by the multilib, use it. + +proc add_options_for_arm_fp { flags } { + if { ! [check_effective_target_arm_fp_ok] } { + return "$flags" + } + global et_arm_fp_flags + return "$flags $et_arm_fp_flags" +} + # Return 1 if this is an ARM target that supports DSP multiply with # current multilib flags. @@ -2753,7 +2794,7 @@ proc check_effective_target_arm_unaligned { } { proc check_effective_target_arm_crypto_ok_nocache { } { global et_arm_crypto_flags set et_arm_crypto_flags "" - if { [check_effective_target_arm32] } { + if { [check_effective_target_arm_v8_neon_ok] } { foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} { if { [check_no_compiler_messages_nocache arm_crypto_ok object { #include "arm_neon.h" @@ -2779,6 +2820,42 @@ proc check_effective_target_arm_crypto_ok { } { check_effective_target_arm_crypto_ok_nocache] } +# Return 1 if this is an ARM target supporting pragma target +# -mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp or equivalent options. +# Some multilibs may be incompatible with these options. Also set +# et_arm_crypto_pragma_flags to the best options to add. + +proc check_effective_target_arm_crypto_pragma_ok_nocache { } { + global et_arm_crypto_pragma_flags + set et_arm_crypto_pragma_flags "" + if { [check_effective_target_arm_v8_neon_ok] } { + foreach flags {"" "-mfloat-abi=softfp" "-mfpu=crypto-neon-fp-armv8" "-mfpu=crypto-neon-fp-armv8 -mfloat-abi=softfp"} { + if { [check_no_compiler_messages_nocache arm_crypto_pragmaok object { + #pragma GCC target ("fpu=crypto-neon-fp-armv8") + #include "arm_neon.h" + uint8x16_t + foo (uint8x16_t a, uint8x16_t b) + { + return vaeseq_u8 (a, b); + } + } "[add_options_for_arm_v8_neon ""] $flags"] } { + set et_arm_crypto_pragma_flags "[add_options_for_arm_v8_neon ""] $flags" + return 1 + } + } + } + + return 0 +} + +# Return 1 if this is an ARM target supporting pragma target +# -mfpu=crypto-neon-fp-armv8. + +proc check_effective_target_arm_crypto_pragma_ok { } { + return [check_cached_effective_target arm_crypto_pragma_ok \ + check_effective_target_arm_crypto_pragma_ok_nocache] +} + # Add options for crypto extensions. proc add_options_for_arm_crypto { flags } { if { ! [check_effective_target_arm_crypto_ok] } { @@ -2907,8 +2984,8 @@ proc check_effective_target_arm_crc_ok { } { # Return 1 if this is an ARM target supporting -mfpu=neon-fp16 # -mfloat-abi=softfp or equivalent options. Some multilibs may be -# incompatible with these options. Also set et_arm_neon_flags to the -# best options to add. +# incompatible with these options. Also set et_arm_neon_fp16_flags to +# the best options to add. proc check_effective_target_arm_neon_fp16_ok_nocache { } { global et_arm_neon_fp16_flags --- a/gcc/testsuite/gcc.target/arm/attr-crypto.c +++ b/gcc/testsuite/gcc.target/arm/attr-crypto.c @@ -1,6 +1,14 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_crypto_ok } */ -/* { dg-options "-O2 -mfloat-abi=softfp" } */ +/* Make sure we can force fpu=vfp before switching using the + pragma. */ +/* { dg-require-effective-target arm_fp_ok } */ +/* { dg-require-effective-target arm_crypto_pragma_ok } */ +/* { dg-options "-O2 -march=armv8-a" } */ +/* { dg-add-options arm_fp } */ + +/* Reset fpu to a value compatible with the next pragmas. */ +#pragma GCC target ("fpu=vfp") +#pragma GCC push_options #pragma GCC target ("fpu=crypto-neon-fp-armv8") @@ -28,7 +36,7 @@ foo (void) return res[0]; } -#pragma GCC reset_options +#pragma GCC pop_options /* Check that the FP version is correctly reset. */ diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c index 6ac32fc..05dc579 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon-builtin-fail.c @@ -1,7 +1,9 @@ /* Check that calling a neon builtin from a function compiled with vfp fails. */ /* { dg-do compile } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-require-effective-target arm_neon_ok } */ -/* { dg-options "-O2 -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ #include <arm_neon.h> @@ -12,6 +14,5 @@ foo (uint8x16_t *p) *p = vmovq_n_u8 (3); /* { dg-message "called from here" } */ } -/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 } - */ +/* { dg-error "inlining failed in call to always_inline" "" { target *-*-* } 0 } */ diff --git a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c index 3cf8918..984992f 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon-fp16.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_neon_ok } */ -/* { dg-options "-mfp16-format=ieee -mfloat-abi=softfp" } */ +/* { dg-require-effective-target arm_fp_ok } */ +/* { dg-options "-mfp16-format=ieee" } */ +/* { dg-add-options arm_fp } */ #include "arm_neon.h" diff --git a/gcc/testsuite/gcc.target/arm/attr-neon2.c b/gcc/testsuite/gcc.target/arm/attr-neon2.c index 819fad4..2966825 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon2.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon2.c @@ -1,6 +1,12 @@ /* { dg-do compile } */ /* { dg-require-effective-target arm_neon_ok } */ -/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */ +/* { dg-require-effective-target arm_fp_ok } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ + +/* Reset fpu to a value compatible with the next pragmas. */ +#pragma GCC target ("fpu=vfp") +#pragma GCC push_options #pragma GCC target ("fpu=neon") #include <arm_neon.h> @@ -12,7 +18,7 @@ my (int8x8_t __a, int8x8_t __b) return __a + __b; } -#pragma GCC reset_options +#pragma GCC pop_options /* Check that command line option is restored. */ int8x8_t diff --git a/gcc/testsuite/gcc.target/arm/attr-neon3.c b/gcc/testsuite/gcc.target/arm/attr-neon3.c index 30a1479..17e429a 100644 --- a/gcc/testsuite/gcc.target/arm/attr-neon3.c +++ b/gcc/testsuite/gcc.target/arm/attr-neon3.c @@ -1,6 +1,12 @@ /* { dg-do compile } */ /* { dg-require-effective-target arm_crypto_ok } */ -/* { dg-options "-O2 -mfloat-abi=softfp -mfpu=vfp" } */ +/* { dg-require-effective-target arm_fp_ok } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ + +/* Reset fpu to a value compatible with the next pragmas. */ +#pragma GCC target ("fpu=vfp") +#pragma GCC push_options #include <arm_neon.h> diff --git a/gcc/testsuite/gcc.target/arm/cmp-2.c b/gcc/testsuite/gcc.target/arm/cmp-2.c index ed6b609..70e4509 100644 --- a/gcc/testsuite/gcc.target/arm/cmp-2.c +++ b/gcc/testsuite/gcc.target/arm/cmp-2.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O" } */ +/* { dg-add-options arm_fp } */ /* { dg-final { scan-assembler-not "\tbl\t" } } */ /* { dg-final { scan-assembler-not "__aeabi" } } */ int x, y; diff --git a/gcc/testsuite/gcc.target/arm/unsigned-float.c b/gcc/testsuite/gcc.target/arm/unsigned-float.c index b9ed681..e1cda0c 100644 --- a/gcc/testsuite/gcc.target/arm/unsigned-float.c +++ b/gcc/testsuite/gcc.target/arm/unsigned-float.c @@ -1,8 +1,9 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ /* { dg-options "-march=armv7-a -O1" } */ -/* { dg-additional-options "-mfloat-abi=softfp" { target { ! { arm_hf_eabi } } } } */ +/* { dg-add-options arm_fp } */ + #include <stdint.h> diff --git a/gcc/testsuite/gcc.target/arm/vfp-1.c b/gcc/testsuite/gcc.target/arm/vfp-1.c index 9aa5302..7add1b8 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-1.c +++ b/gcc/testsuite/gcc.target/arm/vfp-1.c @@ -1,6 +1,7 @@ /* { dg-do compile } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp -ffp-contract=off" } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ +/* { dg-options "-O2 -ffp-contract=off" } */ +/* { dg-add-options arm_fp } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ extern float fabsf (float); diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c b/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c index 7041579..3489a2a 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c +++ b/gcc/testsuite/gcc.target/arm/vfp-ldmdbd.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ extern void bar (double); diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c b/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c index 0187c01..8fda405 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c +++ b/gcc/testsuite/gcc.target/arm/vfp-ldmdbs.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ extern void bar (float); diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c b/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c index 9c22f1f..422e3ed 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c +++ b/gcc/testsuite/gcc.target/arm/vfp-ldmiad.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ extern void bar (double); diff --git a/gcc/testsuite/gcc.target/arm/vfp-ldmias.c b/gcc/testsuite/gcc.target/arm/vfp-ldmias.c index 92051fd..31d2ee1 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-ldmias.c +++ b/gcc/testsuite/gcc.target/arm/vfp-ldmias.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ extern void bar (float); diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c b/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c index 53383b5..686fe86 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c +++ b/gcc/testsuite/gcc.target/arm/vfp-stmdbd.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ void foo (double *p, double a, double b, int n) diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c b/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c index 6570def..dbb30ec 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c +++ b/gcc/testsuite/gcc.target/arm/vfp-stmdbs.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ void foo (float *p, float a, float b, int n) diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmiad.c b/gcc/testsuite/gcc.target/arm/vfp-stmiad.c index 28e9d73..665fa7a 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-stmiad.c +++ b/gcc/testsuite/gcc.target/arm/vfp-stmiad.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ void foo (double *p, double a, double b, int n) diff --git a/gcc/testsuite/gcc.target/arm/vfp-stmias.c b/gcc/testsuite/gcc.target/arm/vfp-stmias.c index efa5fbe..90940e5 100644 --- a/gcc/testsuite/gcc.target/arm/vfp-stmias.c +++ b/gcc/testsuite/gcc.target/arm/vfp-stmias.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -mfpu=vfp -mfloat-abi=softfp" } */ +/* { dg-options "-O2" } */ +/* { dg-add-options arm_fp } */ void foo (float *p, float a, float b, int n) diff --git a/gcc/testsuite/gcc.target/arm/vnmul-1.c b/gcc/testsuite/gcc.target/arm/vnmul-1.c index af0bebe..fd00388 100644 --- a/gcc/testsuite/gcc.target/arm/vnmul-1.c +++ b/gcc/testsuite/gcc.target/arm/vnmul-1.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -fno-rounding-math -mfpu=vfp -mfloat-abi=hard" } */ +/* { dg-options "-O2 -fno-rounding-math" } */ +/* { dg-add-options arm_fp } */ double foo_d (double a, double b) diff --git a/gcc/testsuite/gcc.target/arm/vnmul-2.c b/gcc/testsuite/gcc.target/arm/vnmul-2.c index 909b2a4..c299ec1 100644 --- a/gcc/testsuite/gcc.target/arm/vnmul-2.c +++ b/gcc/testsuite/gcc.target/arm/vnmul-2.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -frounding-math -mfpu=vfp -mfloat-abi=hard" } */ +/* { dg-options "-O2 -frounding-math" } */ +/* { dg-add-options arm_fp } */ double foo_d (double a, double b) diff --git a/gcc/testsuite/gcc.target/arm/vnmul-3.c b/gcc/testsuite/gcc.target/arm/vnmul-3.c index df02882..44c1967 100644 --- a/gcc/testsuite/gcc.target/arm/vnmul-3.c +++ b/gcc/testsuite/gcc.target/arm/vnmul-3.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -fno-rounding-math -mfpu=vfp -mfloat-abi=hard" } */ +/* { dg-options "-O2 -fno-rounding-math" } */ +/* { dg-add-options arm_fp } */ double foo_d (double a, double b) diff --git a/gcc/testsuite/gcc.target/arm/vnmul-4.c b/gcc/testsuite/gcc.target/arm/vnmul-4.c index 670ee40..dd9cab3 100644 --- a/gcc/testsuite/gcc.target/arm/vnmul-4.c +++ b/gcc/testsuite/gcc.target/arm/vnmul-4.c @@ -1,7 +1,8 @@ /* { dg-do compile } */ -/* { dg-require-effective-target arm_vfp_ok } */ +/* { dg-require-effective-target arm_fp_ok } */ /* { dg-skip-if "need fp instructions" { *-*-* } { "-mfloat-abi=soft" } { "" } } */ -/* { dg-options "-O2 -frounding-math -mfpu=vfp -mfloat-abi=hard" } */ +/* { dg-options "-O2 -frounding-math" } */ +/* { dg-add-options arm_fp } */ double foo_d (double a, double b)
Hi, Here is an updated version of this patch. I did test it with -mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard in addition to my usual set of options. Compared to the previous version: - I added some doc in sourcebuild.texi - I no longer modify arm_vfp_ok... - I replaced all uses of arm_vfp with the new arm_fp because I found that the existing tests do not actually need to pass -mfpu=vfp: this is implicitly set as the default when using -mfloat-abi={softfp|hard} - I chose not to remove arm_vfp_ok because we may need it in the future, if a test really needs vfp (as opposed to neon for instance) - in gcc.target/arm/attr-crypto.c I force the initial fpu to be vfp via pragma instead, so that the next pragma fpu fpu=crypto-neon-fp-armv8 is always compatible, regardless of the command-line options/default fpu - same for attr-neon2.c and attr-neon3.c - I updated cmp-2.c, unsigned-float.c, vfp-1.c, vfp-ldmdbd.c, vfp-ldmdbs.c, vfp-ldmiad.c, vfp-ldmias.c, vfp-stmdbd.c, vfp-stmdbs.c, vfp-stmiad.c, vfp-stmias.c, vnmul-[1234].c to use the new arm_fp effective target instead of arm_vfp. This is so that they don't need to use -mfpu=vfp and can use the new dg-add-options arm_fp The validation results show (in addition to what I originally reported): - attr-crypto.c and attr-neon3.c now ICE in some cases. This is PR68895. - depending on the GCC configuration (e.g. --with-fpu=neon) attr-neon3.c may fail. This is PR68896. OK? Christophe 2015-12-17 Christophe Lyon <christophe.lyon@linaro.org> * doc/sourcebuild.texi (arm_fp_ok): Document new entry. (arm_fp): Likewise. * lib/target-supports.exp (check_effective_target_arm_fp_ok_nocache): New. (check_effective_target_arm_fp_ok): New. (add_options_for_arm_fp): New. (check_effective_target_arm_crypto_ok_nocache): Require target_arm_v8_neon_ok instead of arm32. (check_effective_target_arm_crypto_pragma_ok_nocache): New. (check_effective_target_arm_crypto_pragma_ok): New. (add_options_for_arm_vfp): New. * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok effective target. Do not force -mfloat-abi=softfp, use arm_fp_ok effective target instead. Force initial fpu to vfp. * gcc.target/arm/attr-neon-builtin-fail.c: Do not force -mfloat-abi=softfp, use arm_fp_ok effective target instead. * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok dependency. * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, use arm_vfp effective target instead. Force initial fpu to vfp. * gcc.target/arm/attr-neon3.c: Likewise. * gcc.target/arm/cmp-2.c: Use arm_fp_ok effective target instead of arm_vfp_ok. * gcc.target/arm/unsigned-float.c: Likewise. * gcc.target/arm/vfp-1.c: Likewise. * gcc.target/arm/vfp-ldmdbd.c: Likewise. * gcc.target/arm/vfp-ldmdbs.c: Likewise. * gcc.target/arm/vfp-ldmiad.c: Likewise. * gcc.target/arm/vfp-ldmias.c: Likewise. * gcc.target/arm/vfp-stmdbd.c: Likewise. * gcc.target/arm/vfp-stmdbs.c: Likewise. * gcc.target/arm/vfp-stmiad.c: Likewise. * gcc.target/arm/vfp-stmias.c: Likewise. * gcc.target/arm/vnmul-1.c: Likewise. * gcc.target/arm/vnmul-2.c: Likewise. * gcc.target/arm/vnmul-3.c: Likewise. * gcc.target/arm/vnmul-4.c: Likewise. On 10 December 2015 at 20:52, Christophe Lyon <christophe.lyon@linaro.org> wrote: > On 10 December 2015 at 14:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote: >> >> On 10/12/15 13:04, Christophe Lyon wrote: >>> >>> On 10 December 2015 at 13:30, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>> wrote: >>>> >>>> Hi Christophe, >>>> >>>> >>>> On 08/12/15 11:18, Christophe Lyon wrote: >>>>> >>>>> On 8 December 2015 at 11:50, Kyrill Tkachov <kyrylo.tkachov@arm.com> >>>>> wrote: >>>>>> >>>>>> Hi Christophe, >>>>>> >>>>>> >>>>>> On 27/11/15 13:00, Christophe Lyon wrote: >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> After the recent commits from Christian adding target attributes >>>>>>> support for ARM FPU settings, I've noticed that some of the tests >>>>>>> were failing because of incorrect assumptions wrt to the default >>>>>>> cpu/fpu/float-abi of the compiler. >>>>>>> >>>>>>> This patch fixes the problems I've noticed in the following way: >>>>>>> - do not force -mfloat-abi=softfp in dg-options, to avoid conflicts >>>>>>> when gcc is configured --with-float=hard >>>>>>> >>>>>>> - change arm_vfp_ok such that it tries several -mfpu/-mfloat-abi >>>>>>> flags, checks that __ARM_FP is defined and __ARM_NEON_FP is not >>>>>>> defined >>>>>>> >>>>>>> - introduce arm_fp_ok, which is similar but does not enforce fpu >>>>>>> setting >>>>>>> >>>>>>> - add a new effective_target: arm_crypto_pragma_ok to check that >>>>>>> setting this fpu via a pragma is actually supported by the current >>>>>>> "multilib". This is different from checking the command-line option >>>>>>> because the pragma might conflict with the command-line options in >>>>>>> use. >>>>>>> >>>>>>> The updates in the testcases are as follows: >>>>>>> - attr-crypto.c, we have to make sure that the defaut fpu does not >>>>>>> conflict with the one forced by pragma. That's why I use the arm_vfp >>>>>>> options/effective_target. This is needed if gcc has been configured >>>>>>> --with-fpu=neon-fp16, as the pragma fpu=crypto-neon-fp-armv8 would >>>>>>> conflict. >>>>>>> >>>>>>> - attr-neon-builtin-fail.c: use arm_fp to force the appropriate >>>>>>> float-abi setting. Enforcing fpu is not needed here. >>>>>>> >>>>>>> - attr-neon-fp16.c: similar, I also removed arm_neon_ok since it was >>>>>>> not necessary to make the test pass in my testing. On second thought, >>>>>>> I'm wondering whether I should leave it and make the test unsupported >>>>>>> in more cases (such as when forcing -march=armv5t, although it does >>>>>>> pass with this patch) >>>>>>> >>>>>>> - attr-neon2.c: use arm_vfp to force the appropriate float-abi >>>>>>> setting. Enforcing mfpu=vfp is needed to avoid conflict with the >>>>>>> pragma target fpu=neon (for instance if the toolchain default is >>>>>>> neon-fp16) >>>>>>> >>>>>>> - attr-neon3.c: similar >>>>>>> >>>>>>> Tested on a variety of configurations, see: >>>>>>> >>>>>>> >>>>>>> >>>>>>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-patches/230929-target-attr/report-build-info.html >>>>>>> >>>>>>> Note that the regressions reported fall into 3 categories: >>>>>>> - when forcing march=armv5t: tests are now unsupported because I >>>>>>> modified arm_crypto_ok to require arm_v8_neon_ok instead of arm32. >>>>>>> >>>>>>> - the warning reported by attr-neon-builtin-fail.c moved from line 12 >>>>>>> to 14 and is thus seen as a regression + one improvement >>>>>>> >>>>>>> - finally, attr-neon-fp16.c causes an ICE on armeb compilers, for >>>>>>> which I need to post a bugzilla. >>>>>>> >>>>>>> >>>>>>> TBH, I'm a bit concerned by the complexity of all these multilib-like >>>>>>> conditions. I'm confident that I'm still missing some combinations :-) >>>>>>> >>>>>>> And with new target attributes coming, new architectures etc... all >>>>>>> this logic is likely to become even more complex. >>>>>>> >>>>>>> That being said, OK for trunk? >>>>>> >>>>>> >>>>>> I'd like us to clean up and reorganise the gcc.target/arm testsuite >>>>>> at some point to make it more robust with respect to the tons of >>>>>> multilib >>>>>> options and configurations we can have for arm. It's becoming very >>>>>> frustrating >>>>>> to test feature-specific stuff :( >>>>>> >>>>>> This is ok in the meantime. >>>>>> Sorry for the delay. >>>>>> >>>>> Thanks for the approval, glad to see I'm not the only one willing to see >>>>> improvements in this area :) >>>>> >>>>> Committed as r231403. >>>> >>>> >>>> With this patch we're seeing legitimate PASSes go to NA. >>>> For example: >>>> >>>> gcc.target/arm/vfp-1.c >>>> gcc.target/arm/vfp-ldmdbs.c >>>> and other vfp tests in arm.exp. >>>> This is, for example, for the >>>> variant-mthumb/-march=armv8-a/-mfpu=crypto-neon-fp-armv8/-mfloat-abi=hard >>>> >>> Hmm I'm attempting to cover such a configuration in my matrix of >>> validations: >>> >>> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/231403/report-build-info.html >>> >>> The difference is that I use similar flags at GCC configure time, while >>> you >>> override them when running the testsuite: >>> --target arm-none-linux-gnueabihf --with-mode=thum --with-cpu=cortex-a57 >>> --with-fpu=crypto-neon-fp-armv8 >>> >>> I this case, I do not see the regressions. >> >> >> My gcc is arm-none-eabi configured with >> --with-float=hard --with-cpu=cortex-a15 --with-fpu=neon-vfpv4 >> --with-float=hard >> >>>> I suspect under your new predicates they'd need to be changed to check >>>> for >>>> arm_fp_ok rather than arm_vfp_ok. >>> >>> Probably, yes. >>> >> >> In the test log where it checks the effective target it fails due to: >> arm_vfp_ok27268.c:6:4: error: #error __ARM_NEON_FP defined >> it's compiling the check with >> -mthumb -march=armv8-a -mfpu=crypto-neon-fp-armv8 -mfloat-abi=hard -o >> arm_vfp_ok27268.o arm_vfp_ok27268.c >> >>>> We want to avoid removing any PASSes. >>>> Can you please test some more to make sure we don't remove any legitimate >>>> passes with your patch? >>> >>> Is that the only combination in which you saw less PASSes? >> >> >> That's the one that was brought to my attention, but I think the issue here >> is just the >> tests that check for arm_vfp_ok rather than the new arm_fp_ok and set >> -mfpu=vfp explicitly. >> They appear unsupported if testing with an explicit neon option in mfpu, I >> think. >> >>>> Also, Ramana reminded me offline that any new predicates in >>>> target-supports.exp >>>> need documenting in sourcebuild.txt. >>>> >>>> In light of that, can you please revert your patch and address the issues >>>> above >>>> so that we can be confident we don't lose existing legitimate test >>>> coverage? >>> >>> OK. >>> >>>> Sorry for not catching these sooner. >>> >>> No problem, there are so many combinations. >>> I'm not sure how to define a reasonable set. >> >> >> So, I think the action plan here is to audit the tests that need to be >> changed >> to arm_fp_ok and add the documentation for the new predicate checks. >> > > I came up with a new version where I now use only the new arm_fp_ok, so > it might be easier just to update arm_vfp_ok and not introduce arm_fp_ok. > > However, I'm still not happy with this version because it has problems with > a few tests that use target attributes such as attr-crypto.c, which starts with: > #pragma GCC target ("fpu=crypto-neon-fp-armv8") > > This pragma fails if the compiler+options have set an incompatible > fpu before processing the pragma. > > So for instance, if gcc has been configured --with-fpu=neon, compiling the test > with no fpu option fails (neon is not compatible with crypto-neon-fp-armv8 > because the latter has fp16) > > Now, if we use effective_target arm_vfp_ok + dg_option to force -mfpu=vfp, > we end up compiling with -mfpu=vfp and the pragma passes. > > But if, in addition, we force the testflags to set -mfpu=crypto-neon-fp-armv8 > as you do, the effective_target arm_vfp_ok test now fails because it is > compiled with -mfpu=vfp -mfpu=crypto-neon-fp-armv8, which defines > __ARM_NEON_FP > > In short, the problem is how to make sure that the fpu setting is always > compatible with the pragma, whatever the gcc configuration and multilib > options used. > > Thanks, > > Christophe. > >> Thanks, >> Kyrill >> >> >>> Christophe. >>> >>>> Kyrill >>>> >>>> >>>>> Christophe. >>>>> >>>>>> Thanks for handling this! >>>>>> Kyrill >>>>>> >>>>>> >>>>>>> Christophe >>>>>>> >>>>>>> >>>>>>> 2015-11-27 Christophe Lyon <christophe.lyon@linaro.org> >>>>>>> >>>>>>> * lib/target-supports.exp >>>>>>> (check_effective_target_arm_vfp_ok_nocache): New. >>>>>>> (check_effective_target_arm_vfp_ok): Call the new >>>>>>> check_effective_target_arm_vfp_ok_nocache function. >>>>>>> (check_effective_target_arm_fp_ok_nocache): New. >>>>>>> (check_effective_target_arm_fp_ok): New. >>>>>>> (add_options_for_arm_fp): New. >>>>>>> (check_effective_target_arm_crypto_ok_nocache): Require >>>>>>> target_arm_v8_neon_ok instead of arm32. >>>>>>> (check_effective_target_arm_crypto_pragma_ok_nocache): New. >>>>>>> (check_effective_target_arm_crypto_pragma_ok): New. >>>>>>> (add_options_for_arm_vfp): New. >>>>>>> * gcc.target/arm/attr-crypto.c: Use arm_crypto_pragma_ok >>>>>>> effective >>>>>>> target. Do not force -mfloat-abi=softfp, use arm_vfp effective >>>>>>> target instead. >>>>>>> * gcc.target/arm/attr-neon-builtin-fail.c: Do not force >>>>>>> -mfloat-abi=softfp, use arm_fp effective target instead. >>>>>>> * gcc.target/arm/attr-neon-fp16.c: Likewise. Remove arm_neon_ok >>>>>>> dependency. >>>>>>> * gcc.target/arm/attr-neon2.c: Do not force -mfloat-abi=softfp, >>>>>>> use arm_vfp effective target instead. >>>>>>> * gcc.target/arm/attr-neon3.c: Likewise. >>>>>> >>>>>> >>