aarch64: Fix wrong code for bfloat when f16 is enabled [PR 111867]

Message ID 20231210085546.1531375-1-quic_apinski@quicinc.com
State Accepted
Headers
Series aarch64: Fix wrong code for bfloat when f16 is enabled [PR 111867] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew Pinski (QUIC) Dec. 10, 2023, 8:55 a.m. UTC
  The problem here is when f16 is enabled, movbf_aarch64 accepts `Ufc`
as a constraint:
     [ w        , Ufc ; fconsts     , fp16  ] fmov\t%h0, %1
But that is for fmov values and in this case fmov represents f16 rather than bfloat16 values.
This means we would get the wrong value in the register.

Built and tested for aarch64-linux-gnu with no regressions.  Also tested with `-march=armv9-a+sve2,
gcc.dg/torture/bfloat16-basic.c and gcc.dg/torture/bfloat16-builtin.c no longer fail.

gcc/ChangeLog:

	PR target/111867
	* config/aarch64/aarch64.cc (aarch64_float_const_representable_p): For BFmode,
	only accept +0.0.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/config/aarch64/aarch64.cc | 5 +++++
 1 file changed, 5 insertions(+)
  

Comments

Richard Sandiford Dec. 11, 2023, 3:54 p.m. UTC | #1
Andrew Pinski <quic_apinski@quicinc.com> writes:
> The problem here is when f16 is enabled, movbf_aarch64 accepts `Ufc`
> as a constraint:
>      [ w        , Ufc ; fconsts     , fp16  ] fmov\t%h0, %1
> But that is for fmov values and in this case fmov represents f16 rather than bfloat16 values.
> This means we would get the wrong value in the register.
>
> Built and tested for aarch64-linux-gnu with no regressions.  Also tested with `-march=armv9-a+sve2,
> gcc.dg/torture/bfloat16-basic.c and gcc.dg/torture/bfloat16-builtin.c no longer fail.
>
> gcc/ChangeLog:
>
> 	PR target/111867
> 	* config/aarch64/aarch64.cc (aarch64_float_const_representable_p): For BFmode,
> 	only accept +0.0.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/config/aarch64/aarch64.cc | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 5cffdabc62e..d48f5a1ba4b 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -23904,6 +23904,7 @@ aarch64_float_const_representable_p (rtx x)
>  
>    r = *CONST_DOUBLE_REAL_VALUE (x);
>  
> +
>    /* We cannot represent infinities, NaNs or +/-zero.  We won't
>       know if we have +zero until we analyse the mantissa, but we
>       can reject the other invalid values.  */

Seems like a stray change.

OK without that, thanks.

Richard

> @@ -23911,6 +23912,10 @@ aarch64_float_const_representable_p (rtx x)
>        || REAL_VALUE_MINUS_ZERO (r))
>      return false;
>  
> +  /* For BFmode, only handle 0.0. */
> +  if (GET_MODE (x) == BFmode)
> +    return real_iszero (&r, false);
> +
>    /* Extract exponent.  */
>    r = real_value_abs (&r);
>    exponent = REAL_EXP (&r);
  

Patch

diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 5cffdabc62e..d48f5a1ba4b 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -23904,6 +23904,7 @@  aarch64_float_const_representable_p (rtx x)
 
   r = *CONST_DOUBLE_REAL_VALUE (x);
 
+
   /* We cannot represent infinities, NaNs or +/-zero.  We won't
      know if we have +zero until we analyse the mantissa, but we
      can reject the other invalid values.  */
@@ -23911,6 +23912,10 @@  aarch64_float_const_representable_p (rtx x)
       || REAL_VALUE_MINUS_ZERO (r))
     return false;
 
+  /* For BFmode, only handle 0.0. */
+  if (GET_MODE (x) == BFmode)
+    return real_iszero (&r, false);
+
   /* Extract exponent.  */
   r = real_value_abs (&r);
   exponent = REAL_EXP (&r);