Re: [PATCH, ARM] Do softfloat when -mfpu set, -mfloat-abi=softfp and targeting Thumb-1

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 14/12/2018 21:15, Thomas Preudhomme wrote:
> Hi Richard,
> 
> Thanks for catching the problem with this approach. Hopefully this
> version should solve the real problem:
> 
> 
> FP instructions are only enabled for TARGET_32BIT and TARGET_HARD_FLOAT
> but GCC only gives an error when TARGET_HARD_FLOAT is true and -mfpu is
> not set. Among other things, it makes some of the cmse tests (eg.
> gcc.target/arm/cmse/baseline/softfp.c) fail when targeting
> -march=armv8-m.base -mcmse -mfpu=<something> -mfloat-abi=softfp. This
> patch adds an extra check for TARGET_32BIT to TARGET_HARD_FLOAT such
> that it is false on TARGET_THUMB1 targets even when a FPU is specified.
> 
> ChangeLog entries are as follows:
> 
> *** gcc/ChangeLog ***
> 
> 2018-12-14  thomas Preud'homme  <thomas.preudhomme@linaro.org>
> 
>         * config/arm/arm.h (TARGET_HARD_FLOAT): Restrict to TARGET_32BIT
>         targets.

Yes, this is better.  And with this change, I think this line:

	      if (TARGET_HARD_FLOAT && !TARGET_THUMB1)

in output_return_instruction() can be collapsed into simply


	if (TARGET_HARD_FLOAT)

OK with that change.

R.

> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2018-12-14  thomas Preud'homme  <thomas.preudhomme@linaro.org>
> 
>         * gcc.target/arm/cmse/baseline/softfp.c: Force an FPU.
> 
> Testing: No testsuite regression when targeting arm-none-eabi Armv6S-M
> with -mfloat-abi=softfp
> 
> Is this ok for stage3?
> 
> Best regards,
> 
> Thomas
> 
> On Thu, 29 Nov 2018 at 14:52, Richard Earnshaw (lists)
> <Richard.Earnshaw@arm.com> wrote:
>>
>> On 29/11/2018 10:51, Thomas Preudhomme wrote:
>>> Hi,
>>>
>>> FP instructions are only enabled for TARGET_32BIT and TARGET_HARD_FLOAT
>>> but GCC only gives an error when TARGET_HARD_FLOAT is true and -mfpu is
>>> not set. Among other things, it makes some of the cmse tests (eg.
>>> gcc.target/arm/cmse/baseline/softfp.c) fail when targeting
>>> -march=armv8-m.base -mfpu=<something> -mfloat-abi=softfp. This patch
>>> errors out when a Thumb-1 -like target is selected and a FPU is
>>> specified, thus making such tests being skipped.
>>>
>>> ChangeLog entries are as follows:
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2018-11-28  thomas Preud'homme  <thomas.preudhomme@linaro.org>
>>>
>>>     * config/arm/arm.c (arm_options_perform_arch_sanity_checks): Error out
>>>     if targeting Thumb-1 with an FPU specified.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2018-11-28  thomas Preud'homme  <thomas.preudhomme@linaro.org>
>>>
>>>     * gcc.target/arm/thumb1_mfpu-1.c: New testcase.
>>>     * gcc.target/arm/thumb1_mfpu-2.c: Likewise.
>>>
>>> Testing: No testsuite regression when targeting arm-none-eabi Armv6S-M.
>>> Fails as expected when targeting Armv6-M with an -mfpu or a default FPU.
>>> Succeeds without.
>>>
>>> Is this ok for stage3?
>>>
>>
>> This doesn't sound right.  Specifically this bit...
>>
>> +      else if (TARGET_THUMB1
>> +              && bitmap_bit_p (arm_active_target.isa, isa_bit_vfpv2))
>> +       error ("Thumb-1 does not allow FP instructions");
>>
>> If I use
>>
>>     -mcpu=arm1176jzf-s -mfpu=auto -mfloat-abi=softfp -mthumb
>>
>> then that shouldn't error, since softfp and thumb is, in reality, just
>> float-abi=soft (as there are no fp instructions in thumb).  We also want
>> it to work this way so that I can add the thumb/arm attribute to
>> specific functions and have the compiler use HW float instructions when
>> they are suitable.
>>
>>
>> R.
>>
>>> Best regards,
>>>
>>> Thomas
>>>
>>>
>>> thumb1_mfpu_error.patch
>>>
>>> From 051e38552d7c596873e0303f6ec4272b26d50900 Mon Sep 17 00:00:00 2001
>>> From: Thomas Preud'homme <thomas.preudhomme@linaro.org>
>>> Date: Tue, 27 Nov 2018 15:52:38 +0000
>>> Subject: [PATCH] [PATCH, ARM] Error out when -mfpu set and targeting Thumb-1
>>>
>>> Hi,
>>>
>>> FP instructions are only enabled for TARGET_32BIT and TARGET_HARD_FLOAT
>>> but GCC only gives an error when TARGET_HARD_FLOAT is true and -mfpu is
>>> not set. Among other things, it makes some of the cmse tests (eg.
>>> gcc.target/arm/cmse/baseline/softfp.c) fail when targeting
>>> -march=armv8-m.base -mfpu=<something> -mfloat-abi=softfp. This patch
>>> errors out when a Thumb-1 -like target is selected and a FPU is
>>> specified, thus making such tests being skipped.
>>>
>>> ChangeLog entries are as follows:
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2018-11-28  thomas Preud'homme  <thomas.preudhomme@linaro.org>
>>>
>>>       * config/arm/arm.c (arm_options_perform_arch_sanity_checks): Error out
>>>       if targeting Thumb-1 with an FPU specified.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2018-11-28  thomas Preud'homme  <thomas.preudhomme@linaro.org>
>>>
>>>       * gcc.target/arm/thumb1_mfpu-1.c: New testcase.
>>>       * gcc.target/arm/thumb1_mfpu-2.c: Likewise.
>>>
>>> Testing: No testsuite regression when targeting arm-none-eabi Armv6S-M.
>>> Fails as expected when targeting Armv6-M with an -mfpu or a default FPU.
>>> Succeeds without.
>>>
>>> Is this ok for stage3?
>>>
>>> Best regards,
>>>
>>> Thomas
>>> ---
>>>  gcc/config/arm/arm.c                         | 3 +++
>>>  gcc/testsuite/gcc.target/arm/thumb1_mfpu-1.c | 7 +++++++
>>>  gcc/testsuite/gcc.target/arm/thumb1_mfpu-2.c | 8 ++++++++
>>>  3 files changed, 18 insertions(+)
>>>  create mode 100644 gcc/testsuite/gcc.target/arm/thumb1_mfpu-1.c
>>>  create mode 100644 gcc/testsuite/gcc.target/arm/thumb1_mfpu-2.c
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index 40f0574e32e..1a205123cf5 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -3747,6 +3747,9 @@ arm_options_perform_arch_sanity_checks (void)
>>>      {
>>>        if (arm_abi == ARM_ABI_IWMMXT)
>>>       arm_pcs_default = ARM_PCS_AAPCS_IWMMXT;
>>> +      else if (TARGET_THUMB1
>>> +            && bitmap_bit_p (arm_active_target.isa, isa_bit_vfpv2))
>>> +     error ("Thumb-1 does not allow FP instructions");
>>>        else if (TARGET_HARD_FLOAT_ABI)
>>>       {
>>>         arm_pcs_default = ARM_PCS_AAPCS_VFP;
>>> diff --git a/gcc/testsuite/gcc.target/arm/thumb1_mfpu-1.c b/gcc/testsuite/gcc.target/arm/thumb1_mfpu-1.c
>>> new file mode 100644
>>> index 00000000000..5347e63f9b6
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/thumb1_mfpu-1.c
>>> @@ -0,0 +1,7 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-require-effective-target arm_thumb1_ok } */
>>> +/* { dg-skip-if "incompatible float ABI" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=softfp" } } */
>>> +/* { dg-options "-mthumb -mfpu=vfp -mfloat-abi=softfp" } */
>>> +/* { dg-error "Thumb-1 does not allow FP instructions" "" { target *-*-* } 0 } */
>>> +
>>> +int foo;
>>> diff --git a/gcc/testsuite/gcc.target/arm/thumb1_mfpu-2.c b/gcc/testsuite/gcc.target/arm/thumb1_mfpu-2.c
>>> new file mode 100644
>>> index 00000000000..941ed26ed01
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.target/arm/thumb1_mfpu-2.c
>>> @@ -0,0 +1,8 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-require-effective-target arm_thumb1_ok } */
>>> +/* { dg-skip-if "incompatible float ABI" { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=softfp" } } */
>>> +/* No need to skip in presence of -mfpu since arm_thumb1_ok will already fail
>>> +   due to Thumb-1 with -mfpu which is tested by thumb1_mfpu-1 testcase.  */
>>> +/* { dg-options "-mthumb -mfloat-abi=softfp" } */
>>> +
>>> +int foo;
>>>
>>
>>
>> softfloat_mfpu_set_softfp_thumb1.patch
>>
>> From 7c056f386df458f08c0ae367edc7050d53cd6602 Mon Sep 17 00:00:00 2001
>> From: Thomas Preud'homme <thomas.preudhomme@linaro.org>
>> Date: Tue, 27 Nov 2018 15:52:38 +0000
>> Subject: [PATCH] [PATCH, ARM] Do softfloat when -mfpu set, -mfloat-abi=softfp
>>  and targeting Thumb-1
>>
>> Hi,
>>
>> FP instructions are only enabled for TARGET_32BIT and TARGET_HARD_FLOAT
>> but GCC only gives an error when TARGET_HARD_FLOAT is true and -mfpu is
>> not set. Among other things, it makes some of the cmse tests (eg.
>> gcc.target/arm/cmse/baseline/softfp.c) fail when targeting
>> -march=armv8-m.base -mcmse -mfpu=<something> -mfloat-abi=softfp. This
>> patch adds an extra check for TARGET_32BIT to TARGET_HARD_FLOAT such
>> that it is false on TARGET_THUMB1 targets even when a FPU is specified.
>>
>> ChangeLog entries are as follows:
>>
>> *** gcc/ChangeLog ***
>>
>> 2018-12-14  thomas Preud'homme  <thomas.preudhomme@linaro.org>
>>
>> 	* config/arm/arm.h (TARGET_HARD_FLOAT): Restrict to TARGET_32BIT
>> 	targets.
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2018-12-14  thomas Preud'homme  <thomas.preudhomme@linaro.org>
>>
>> 	* gcc.target/arm/cmse/baseline/softfp.c: Force an FPU.
>>
>> Testing: No testsuite regression when targeting arm-none-eabi Armv6S-M.
>>
>> Is this ok for stage3?
>>
>> Best regards,
>>
>> Thomas
>> ---
>>  gcc/config/arm/arm.h                                | 3 ++-
>>  gcc/testsuite/gcc.target/arm/cmse/baseline/softfp.c | 4 +++-
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>> index 8472312487b..149243d0978 100644
>> --- a/gcc/config/arm/arm.h
>> +++ b/gcc/config/arm/arm.h
>> @@ -125,7 +125,8 @@ extern tree arm_fp16_type_node;
>>  /* Use hardware floating point instructions. */
>>  #define TARGET_HARD_FLOAT	(arm_float_abi != ARM_FLOAT_ABI_SOFT	\
>>  				 && bitmap_bit_p (arm_active_target.isa, \
>> -						  isa_bit_vfpv2))
>> +						  isa_bit_vfpv2) \
>> +				 && TARGET_32BIT)
>>  #define TARGET_SOFT_FLOAT	(!TARGET_HARD_FLOAT)
>>  /* User has permitted use of FP instructions, if they exist for this
>>     target.  */
>> diff --git a/gcc/testsuite/gcc.target/arm/cmse/baseline/softfp.c b/gcc/testsuite/gcc.target/arm/cmse/baseline/softfp.c
>> index 3d383ff6ee1..30b3eec078c 100644
>> --- a/gcc/testsuite/gcc.target/arm/cmse/baseline/softfp.c
>> +++ b/gcc/testsuite/gcc.target/arm/cmse/baseline/softfp.c
>> @@ -1,5 +1,7 @@
>>  /* { dg-do compile } */
>> -/* { dg-options "-mcmse -mfloat-abi=softfp" } */
>> +/* Force an FPU to test that it is ignored for Thumb-1 -like targets and that
>> +   no clearing of VFP register occurs.  */
>> +/* { dg-options "-mcmse -mfloat-abi=softfp -mfpu=fpv5-d16" } */
>>  
>>  double __attribute__ ((cmse_nonsecure_call)) (*bar) (float, double);
>>