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

On Tue, 4 Aug 2026, Tamar Christina wrote:

> > -----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]
> > 
> > On Tue, 4 Aug 2026, Tamar Christina wrote:
> > 
> > > 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 = (vector(8) unsigned int) a_6;
> > > _16 = (vector(8) unsigned int) _13;
> > > _17 = (vector(8) unsigned int) _1;
> > > _18 = .FNMA (_15, _16, _17);
> > > _3 = (svfloat32_t __attribute__((arm_sve_vector_bits(256)))) _18;
> > > Generated FMA _3 = (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 apply 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 = 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)
> > 
> > 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.
> > 
> 
> 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

Ah, true.

> So I originally rewrote it to 
> 
>   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.

So with your above argument the ANY_INTEGRAL_TYPE_P (t) should
be superfluous.  It was probably present to guard
the TYPE_UNSIGNED (t) || !TYPE_OVERFLOW_SANITIZED (t) checks
where the latter ICEs when applied to non-integral types.

So I'd say re-spin with the test dropped or added as && as you
suggest above.

Richard.

> 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 = 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=armv9-a -msve-vector-bits=256" } */
> > > +
> > > +#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 = *pa, b = *pb;
> > > +  v8f m = a * b;
> > > +  *pc = *pc - (sv8f)m;
> > > +}
> > > +
> > > +/* Mirrored: multiply in the SVE type, addend a GNU vector.  */
> > > +void q (sv8f *pa, sv8f *pb, v8f *pc)
> > > +{
> > > +  sv8f m = *pa * *pb;
> > > +  *pc = *pc - (v8f)m;
> > > +}
> > > +
> > > +/* Explicit negate of a multiplicand.  */
> > > +void r (v8f *pa, v8f *pb, sv8f *pc)
> > > +{
> > > +  v8f m = (-*pa) * *pb;
> > > +  *pc = *pc + (sv8f)m;
> > > +}
> > >
> > >
> > >
> > 
> > --
> > 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)
> 

-- 
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)