RE: [patch]middle-end: fix condition on multiple negate pattern [PR126602]

> -----Original Message-----
> From: Richard Biener <[email protected]>
> Sent: 04 August 2026 11:44
> To: Tamar Christina <[email protected]>
> Cc: [email protected]; nd <[email protected]>
> Subject: Re: [patch]middle-end: fix condition on multiple negate pattern
> [PR126602]
>=20
> On Tue, 4 Aug 2026, Tamar Christina wrote:
>=20
> > The optimization added in r17-527-gca2920882be has a bogus constraint
> which
> > allows floating point FMAs through and folds them into integer ones. i.=
e. we
> > produce
> >
> > Matching expression match.pd:159, gimple-match-10.cc:33
> > Matching expression match.pd:159, gimple-match-10.cc:33
> > Applying pattern match.pd:10328, gimple-match-5.cc:8685
> > gimple_simplified to _15 =3D (vector(8) unsigned int) a_6;
> > _16 =3D (vector(8) unsigned int) _13;
> > _17 =3D (vector(8) unsigned int) _1;
> > _18 =3D .FNMA (_15, _16, _17);
> > _3 =3D (svfloat32_t __attribute__((arm_sve_vector_bits(256)))) _18;
> > Generated FMA _3 =3D (svfloat32_t
> __attribute__((arm_sve_vector_bits(256)))) _18;
> >
> > Which ICEs because the SVE attributes don't match.
> >
> > This fixes the guard where I think the intention was for this to only a=
pply to
> > Integral types.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > -m32, -m64 and no issues.
> >
> > Ok for master?
> >
> > Thanks,
> > Tamar
> >
> > gcc/ChangeLog:
> >
> > 	PR tree-optimization/126602
> > 	* match.pd: Fix constraints.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	PR tree-optimization/126602
> > 	* gcc.target/aarch64/sve/pr126602.c: New test.
> >
> > ---
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index
> f6ecee41509e360e430da5e0339f998c069495ba..9c821575793fa51095c15
> d0cde68e636138383ad 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -10305,12 +10305,12 @@ DEFINE_INT_AND_FLOAT_ROUND_FN
> (RINT)
> >    (simplify
> >     (fmas:c (nop_convert (negate @0)) @1 @2)
> >     (with { tree t =3D TREE_TYPE (@0); }
> > -    (if ((!ANY_INTEGRAL_TYPE_P (type)
> > -	  || TYPE_UNSIGNED (type)
> > -	  || !TYPE_OVERFLOW_SANITIZED (type))
> > +    (if ((ANY_INTEGRAL_TYPE_P (type)
> > +	  && (TYPE_UNSIGNED (type)
> > +	      || !TYPE_OVERFLOW_SANITIZED (type))
> >  	  && (!ANY_INTEGRAL_TYPE_P (t)
>=20
> Doesn't this check on t have the same issue?  In fact for vectors
> nop_convert will also match VIEW_CONVERT_EXPR, and there
> the use of (convert:utype ..) in the transform looks wrong to me,
> esp. since the V_C_E could be from a float @0.  For integer @0
> a (convert: ...) should be OK.
>=20

I'm expecting that check to never be used. i.e. I'm assuming you're
talking about the !ANY_INTEGRAL_TYPE_P (t) bit,  I left it there
because I figured there must have been a reason in the original
patch.  I can't however think of any useful testcase because i.e.
a Float -> Int conversion would be blocked by tree_nop_conversion_p

So I originally rewrote it to=20

  ANY_INTEGRAL_TYPE_P (type)
  && (TYPE_UNSIGNED (type) || !TYPE_OVERFLOW_SANITIZED (type))
  && ANY_INTEGRAL_TYPE_P (t)
  && (TYPE_UNSIGNED (t) || !TYPE_OVERFLOW_SANITIZED (t))

Which looked more correct to me but I figured I must have missed
something..

This would also fix the convert:

If you agree that's a better guard I'll respin with that.

Thanks,
Tamar

> >  	      || TYPE_UNSIGNED (t)
> > -	      || !TYPE_OVERFLOW_SANITIZED (type)))
> > +	      || !TYPE_OVERFLOW_SANITIZED (t))))
> >     /* Move the negation into FNMA only when signed overflow is
> >        unobservable for both the outer operation and the inner negate. =
 */
> >       (with { tree utype =3D unsigned_type_for (type); }
> > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr126602.c
> b/gcc/testsuite/gcc.target/aarch64/sve/pr126602.c
> > new file mode 100644
> > index
> 0000000000000000000000000000000000000000..3b9378cebb14530443
> b2503a04634aeee2d2e901
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/aarch64/sve/pr126602.c
> > @@ -0,0 +1,28 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -march=3Darmv9-a -msve-vector-bits=3D256" } */
> > +
> > +#include <arm_sve.h>
> > +typedef svfloat32_t sv8f __attribute__((arm_sve_vector_bits(256)));
> > +typedef float v8f __attribute__((vector_size(32)));
> > +
> > +/* c - (sv8f)(a * b), multiply in the GNU vector type.  */
> > +void p (v8f *pa, v8f *pb, sv8f *pc)
> > +{
> > +  v8f a =3D *pa, b =3D *pb;
> > +  v8f m =3D a * b;
> > +  *pc =3D *pc - (sv8f)m;
> > +}
> > +
> > +/* Mirrored: multiply in the SVE type, addend a GNU vector.  */
> > +void q (sv8f *pa, sv8f *pb, v8f *pc)
> > +{
> > +  sv8f m =3D *pa * *pb;
> > +  *pc =3D *pc - (v8f)m;
> > +}
> > +
> > +/* Explicit negate of a multiplicand.  */
> > +void r (v8f *pa, v8f *pb, sv8f *pc)
> > +{
> > +  v8f m =3D (-*pa) * *pb;
> > +  *pc =3D *pc + (sv8f)m;
> > +}
> >
> >
> >
>=20
> --
> Richard Biener <[email protected]>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG
> Nuernberg)