ARM: reset arm_fp16_format

Message ID 66e2cc2d-e409-1a11-5b0a-82e0e90201aa@suse.cz
State New
Headers show
Series
  • ARM: reset arm_fp16_format
Related show

Commit Message

Martin Liška June 1, 2021, 2:05 p.m.
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(+)

-- 
2.31.1

Comments

H.J. Lu via Gcc-patches June 1, 2021, 3:35 p.m. | #1
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.
H.J. Lu via Gcc-patches June 2, 2021, 1:37 p.m. | #2
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
Martin Liška June 23, 2021, 1:31 p.m. | #3
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
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

Patch

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;