[PATCH] ARM: reset arm_fp16_format

[PATCH] ARM: reset arm_fp16_format

[Martin Liška]

Hello.

The patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98636#c20 where
target option restore can be called and arm_fp16_format should be reset
to ARM_FP16_FORMAT_NONE.

It fixes the ICE in the PR.

Can please ARM folks test me the patch on a Arm machine?
Thanks,
Martin

gcc/ChangeLog:

	PR target/98636
	* config/arm/arm.c (arm_option_reconfigure_globals): Reset
	the option if isa_bit_fp16 is not set.
---
  gcc/config/arm/arm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7b37e1b602c..4543f3c6b55 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -3765,6 +3765,8 @@ arm_option_reconfigure_globals (void)
  	error ("selected fp16 options are incompatible");
        arm_fp16_format = ARM_FP16_FORMAT_IEEE;
      }
+  else
+    arm_fp16_format = ARM_FP16_FORMAT_NONE;
  
    arm_arch_cde = 0;
    arm_arch_cde_coproc = 0;
-- 
2.31.1

Re: [PATCH] ARM: reset arm_fp16_format

[Richard Earnshaw]

On 01/06/2021 15:05, Martin Liška wrote:
> Hello.
> 
> The patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98636#c20 
> where
> target option restore can be called and arm_fp16_format should be reset
> to ARM_FP16_FORMAT_NONE.
> 
> It fixes the ICE in the PR.
> 
> Can please ARM folks test me the patch on a Arm machine?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
>      PR target/98636
>      * config/arm/arm.c (arm_option_reconfigure_globals): Reset
>      the option if isa_bit_fp16 is not set.
> ---
>   gcc/config/arm/arm.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 7b37e1b602c..4543f3c6b55 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -3765,6 +3765,8 @@ arm_option_reconfigure_globals (void)
>       error ("selected fp16 options are incompatible");
>         arm_fp16_format = ARM_FP16_FORMAT_IEEE;
>       }
> +  else
> +    arm_fp16_format = ARM_FP16_FORMAT_NONE;
> 
>     arm_arch_cde = 0;
>     arm_arch_cde_coproc = 0;

My initial reaction is 'that can't be right'.

How would -mfp16-format=alternative ever work in this case?

R.

RE: [PATCH] ARM: reset arm_fp16_format

[Tamar Christina]

Hi Martin,

Testsuite isn't very happy with it:

Before:

# of expected passes            149743
# of unexpected failures        294   
# of unexpected successes       2     
# of expected failures          947   
# of unresolved testcases       56    
# of unsupported tests          8248  

After:

# of expected passes            148907
# of unexpected failures        380
# of unexpected successes       2
# of expected failures          947
# of unresolved testcases       267
# of unsupported tests          8466

Regards,
Tamar

> -----Original Message-----
> From: Martin Liška <mliska@suse.cz>
> Sent: Tuesday, June 1, 2021 3:06 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Christophe Lyon <christophe.lyon@linaro.org>; Tamar Christina
> <Tamar.Christina@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Subject: [PATCH] ARM: reset arm_fp16_format
> 
> Hello.
> 
> The patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98636#c20
> where target option restore can be called and arm_fp16_format should be
> reset to ARM_FP16_FORMAT_NONE.
> 
> It fixes the ICE in the PR.
> 
> Can please ARM folks test me the patch on a Arm machine?
> Thanks,
> Martin
> 
> gcc/ChangeLog:
> 
> 	PR target/98636
> 	* config/arm/arm.c (arm_option_reconfigure_globals): Reset
> 	the option if isa_bit_fp16 is not set.
> ---
>   gcc/config/arm/arm.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index
> 7b37e1b602c..4543f3c6b55 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -3765,6 +3765,8 @@ arm_option_reconfigure_globals (void)
>   	error ("selected fp16 options are incompatible");
>         arm_fp16_format = ARM_FP16_FORMAT_IEEE;
>       }
> +  else
> +    arm_fp16_format = ARM_FP16_FORMAT_NONE;
> 
>     arm_arch_cde = 0;
>     arm_arch_cde_coproc = 0;
> --
> 2.31.1

Re: [PATCH] ARM: reset arm_fp16_format

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]

On 6/1/21 5:35 PM, Richard Earnshaw wrote:
> My initial reaction is 'that can't be right'.
> 
> How would -mfp16-format=alternative ever work in this case?

You are right. I'm going to revert the hunk done
in g:ebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e

Martin

[-- Attachment #2: 0001-arm-Revert-partially-ebd5e86c0f41dc1d692f9b2b68a510b.patch --]
[-- Type: text/x-patch, Size: 855 bytes --]

From 371c1992624c9269e2d5747561a8b27b30e485ee Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Wed, 23 Jun 2021 15:30:17 +0200
Subject: [PATCH] arm: Revert partially
 ebd5e86c0f41dc1d692f9b2b68a510b1f6835a3e

	PR target/98636

gcc/ChangeLog:

	* optc-save-gen.awk: Put back arm_fp16_format to
	checked_options.
---
 gcc/optc-save-gen.awk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gcc/optc-save-gen.awk b/gcc/optc-save-gen.awk
index e2a9a496bfd..e363ac731e1 100644
--- a/gcc/optc-save-gen.awk
+++ b/gcc/optc-save-gen.awk
@@ -1442,6 +1442,8 @@ checked_options["flag_omit_frame_pointer"]++
 checked_options["TARGET_ALIGN_CALL"]++
 checked_options["TARGET_CASE_VECTOR_PC_RELATIVE"]++
 checked_options["arc_size_opt_level"]++
+# arm exceptions
+checked_options["arm_fp16_format"]++
 
 
 for (i = 0; i < n_opts; i++) {
-- 
2.32.0