Andrew Pinski <quic_apin...@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_apin...@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;
> +}

Reply via email to