aarch64: Fix DFP constants [PR119131]

  After r15-6660-g45d306a835cb3f865, in some cases
DFP constants would cause an ICE. This is due to
do a mismatch of a few things. The predicate of the move
uses aarch64_valid_fp_move to say if the constant is valid or not.
But after reload/LRA when can_create_pseudo_p returns false; aarch64_valid_fp_move
would return false for constants that were valid for the constraints
of the instruction. A strictor predicate compared to the constraint is wrong.
In this case `Uvi` is the constraint while aarch64_valid_fp_move allows it
via aarch64_can_const_movi_rtx_p for !DECIMAL_FLOAT_MODE_P, there is no such check
for DECIMAL_FLOAT_MODE_P.

The fix is to remove the check !DECIMAL_FLOAT_MODE_P in aarch64_valid_fp_move
and in the define_expand. As now the predicate allows a superset of what is allowed
by the constraints.

Built and tested on aarch64-linux-gnu with no regressions.

	PR target/119131

gcc/ChangeLog:

	* config/aarch64/aarch64.cc (aarch64_valid_fp_move): Remove check
	for !DECIMAL_FLOAT_MODE_P.
	* config/aarch64/aarch64.md (mov<mode>): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.dg/torture/pr119131-1.c: New test.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/config/aarch64/aarch64.cc             | 17 +++++--------
 gcc/config/aarch64/aarch64.md             |  3 +--
 gcc/testsuite/gcc.dg/torture/pr119131-1.c | 31 +++++++++++++++++++++++
 3 files changed, 39 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr119131-1.c
  

Andrew Pinski <quic_apinski@quicinc.com> writes:
> After r15-6660-g45d306a835cb3f865, in some cases
> DFP constants would cause an ICE. This is due to
> do a mismatch of a few things. The predicate of the move
> uses aarch64_valid_fp_move to say if the constant is valid or not.
> But after reload/LRA when can_create_pseudo_p returns false; aarch64_valid_fp_move
> would return false for constants that were valid for the constraints
> of the instruction. A strictor predicate compared to the constraint is wrong.
> In this case `Uvi` is the constraint while aarch64_valid_fp_move allows it
> via aarch64_can_const_movi_rtx_p for !DECIMAL_FLOAT_MODE_P, there is no such check
> for DECIMAL_FLOAT_MODE_P.
>
> The fix is to remove the check !DECIMAL_FLOAT_MODE_P in aarch64_valid_fp_move
> and in the define_expand. As now the predicate allows a superset of what is allowed
> by the constraints.
>
> Built and tested on aarch64-linux-gnu with no regressions.
>
> 	PR target/119131
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64.cc (aarch64_valid_fp_move): Remove check
> 	for !DECIMAL_FLOAT_MODE_P.
> 	* config/aarch64/aarch64.md (mov<mode>): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/torture/pr119131-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/config/aarch64/aarch64.cc             | 17 +++++--------
>  gcc/config/aarch64/aarch64.md             |  3 +--
>  gcc/testsuite/gcc.dg/torture/pr119131-1.c | 31 +++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/torture/pr119131-1.c
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index 36b65df50c5..ecd005d32d8 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -11328,17 +11328,14 @@ aarch64_valid_fp_move (rtx dst, rtx src, machine_mode mode)
>    if (MEM_P (src))
>      return true;
>  
> -  if (!DECIMAL_FLOAT_MODE_P (mode))
> -    {
> -      if (aarch64_can_const_movi_rtx_p (src, mode)
> -	  || aarch64_float_const_representable_p (src)
> -	  || aarch64_float_const_zero_rtx_p (src))
> -	return true;
> +  if (aarch64_can_const_movi_rtx_p (src, mode)
> +      || aarch64_float_const_representable_p (src)
> +      || aarch64_float_const_zero_rtx_p (src))
> +    return true;

Don't we then also need to add a !DECIMAL_FLOAT_MODE_P (mode)
check to aarch64_float_const_representable_p?

Otherwise it looks good.  Thanks for fixing this.

Richard

>  
> -      /* Block FP immediates which are split during expand.  */
> -      if (aarch64_float_const_rtx_p (src))
> -	return false;
> -    }
> +  /* Block FP immediates which are split during expand.  */
> +  if (aarch64_float_const_rtx_p (src))
> +    return false;
>  
>    return can_create_pseudo_p ();
>  }
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 03188a64ab1..031e621c98a 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -1762,8 +1762,7 @@ (define_expand "mov<mode>"
>  	      && aarch64_float_const_zero_rtx_p (operands[1])))
>        operands[1] = force_reg (<MODE>mode, operands[1]);
>  
> -    if (!DECIMAL_FLOAT_MODE_P (<MODE>mode)
> -	&& GET_CODE (operands[1]) == CONST_DOUBLE
> +    if (GET_CODE (operands[1]) == CONST_DOUBLE
>  	&& can_create_pseudo_p ()
>  	&& !aarch64_can_const_movi_rtx_p (operands[1], <MODE>mode)
>  	&& !aarch64_float_const_representable_p (operands[1])
> diff --git a/gcc/testsuite/gcc.dg/torture/pr119131-1.c b/gcc/testsuite/gcc.dg/torture/pr119131-1.c
> new file mode 100644
> index 00000000000..c62f702f98c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr119131-1.c
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target dfp } */
> +/* PR target/119131 */
> +
> +typedef __attribute__((__vector_size__ (64))) char C;
> +typedef __attribute__((__vector_size__ (64))) _Decimal32 D;
> +int a, b;
> +_Decimal32 f;
> +C e;
> +C c;
> +
> +void
> +foo (D d)
> +{
> +  d -= *(_Decimal32 *) __builtin_memset (&f, 0, 4);
> +  b += a;
> +  if (a)
> +    b /= 0; /* { dg-warning "division by zero" } */
> +  c = (C) d + e;
> +}
> +
> +void
> +foo1 (D d)
> +{
> +  __builtin_memset (&f, 0, 4);
> +  d -= *(_Decimal32 *)&f;
> +  b += a;
> +  if (a)
> +    b /= 0;/* { dg-warning "division by zero" } */
> +  c = (C) d + e;
> +}
  

On Tue, Mar 11, 2025 at 9:58 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> Andrew Pinski <quic_apinski@quicinc.com> writes:
> > After r15-6660-g45d306a835cb3f865, in some cases
> > DFP constants would cause an ICE. This is due to
> > do a mismatch of a few things. The predicate of the move
> > uses aarch64_valid_fp_move to say if the constant is valid or not.
> > But after reload/LRA when can_create_pseudo_p returns false; aarch64_valid_fp_move
> > would return false for constants that were valid for the constraints
> > of the instruction. A strictor predicate compared to the constraint is wrong.
> > In this case `Uvi` is the constraint while aarch64_valid_fp_move allows it
> > via aarch64_can_const_movi_rtx_p for !DECIMAL_FLOAT_MODE_P, there is no such check
> > for DECIMAL_FLOAT_MODE_P.
> >
> > The fix is to remove the check !DECIMAL_FLOAT_MODE_P in aarch64_valid_fp_move
> > and in the define_expand. As now the predicate allows a superset of what is allowed
> > by the constraints.
> >
> > Built and tested on aarch64-linux-gnu with no regressions.
> >
> >       PR target/119131
> >
> > gcc/ChangeLog:
> >
> >       * config/aarch64/aarch64.cc (aarch64_valid_fp_move): Remove check
> >       for !DECIMAL_FLOAT_MODE_P.
> >       * config/aarch64/aarch64.md (mov<mode>): Likewise.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/torture/pr119131-1.c: New test.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> >  gcc/config/aarch64/aarch64.cc             | 17 +++++--------
> >  gcc/config/aarch64/aarch64.md             |  3 +--
> >  gcc/testsuite/gcc.dg/torture/pr119131-1.c | 31 +++++++++++++++++++++++
> >  3 files changed, 39 insertions(+), 12 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/torture/pr119131-1.c
> >
> > diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> > index 36b65df50c5..ecd005d32d8 100644
> > --- a/gcc/config/aarch64/aarch64.cc
> > +++ b/gcc/config/aarch64/aarch64.cc
> > @@ -11328,17 +11328,14 @@ aarch64_valid_fp_move (rtx dst, rtx src, machine_mode mode)
> >    if (MEM_P (src))
> >      return true;
> >
> > -  if (!DECIMAL_FLOAT_MODE_P (mode))
> > -    {
> > -      if (aarch64_can_const_movi_rtx_p (src, mode)
> > -       || aarch64_float_const_representable_p (src)
> > -       || aarch64_float_const_zero_rtx_p (src))
> > -     return true;
> > +  if (aarch64_can_const_movi_rtx_p (src, mode)
> > +      || aarch64_float_const_representable_p (src)
> > +      || aarch64_float_const_zero_rtx_p (src))
> > +    return true;
>
> Don't we then also need to add a !DECIMAL_FLOAT_MODE_P (mode)
> check to aarch64_float_const_representable_p?

Yes it should. Let me add that and retest and push with that change.

Thanks,
Andrew

>
> Otherwise it looks good.  Thanks for fixing this.
>
> Richard
>
> >
> > -      /* Block FP immediates which are split during expand.  */
> > -      if (aarch64_float_const_rtx_p (src))
> > -     return false;
> > -    }
> > +  /* Block FP immediates which are split during expand.  */
> > +  if (aarch64_float_const_rtx_p (src))
> > +    return false;
> >
> >    return can_create_pseudo_p ();
> >  }
> > diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> > index 03188a64ab1..031e621c98a 100644
> > --- a/gcc/config/aarch64/aarch64.md
> > +++ b/gcc/config/aarch64/aarch64.md
> > @@ -1762,8 +1762,7 @@ (define_expand "mov<mode>"
> >             && aarch64_float_const_zero_rtx_p (operands[1])))
> >        operands[1] = force_reg (<MODE>mode, operands[1]);
> >
> > -    if (!DECIMAL_FLOAT_MODE_P (<MODE>mode)
> > -     && GET_CODE (operands[1]) == CONST_DOUBLE
> > +    if (GET_CODE (operands[1]) == CONST_DOUBLE
> >       && can_create_pseudo_p ()
> >       && !aarch64_can_const_movi_rtx_p (operands[1], <MODE>mode)
> >       && !aarch64_float_const_representable_p (operands[1])
> > diff --git a/gcc/testsuite/gcc.dg/torture/pr119131-1.c b/gcc/testsuite/gcc.dg/torture/pr119131-1.c
> > new file mode 100644
> > index 00000000000..c62f702f98c
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/torture/pr119131-1.c
> > @@ -0,0 +1,31 @@
> > +/* { dg-do compile } */
> > +/* { dg-require-effective-target dfp } */
> > +/* PR target/119131 */
> > +
> > +typedef __attribute__((__vector_size__ (64))) char C;
> > +typedef __attribute__((__vector_size__ (64))) _Decimal32 D;
> > +int a, b;
> > +_Decimal32 f;
> > +C e;
> > +C c;
> > +
> > +void
> > +foo (D d)
> > +{
> > +  d -= *(_Decimal32 *) __builtin_memset (&f, 0, 4);
> > +  b += a;
> > +  if (a)
> > +    b /= 0; /* { dg-warning "division by zero" } */
> > +  c = (C) d + e;
> > +}
> > +
> > +void
> > +foo1 (D d)
> > +{
> > +  __builtin_memset (&f, 0, 4);
> > +  d -= *(_Decimal32 *)&f;
> > +  b += a;
> > +  if (a)
> > +    b /= 0;/* { dg-warning "division by zero" } */
> > +  c = (C) d + e;
> > +}