On Tue, Mar 11, 2025 at 9:58 AM Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> 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?
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;
> > +}
