Thanks Richard for comments.

> So you say nop_convert here but the pattern allows (convert ...)

The outer cannot be nop_convert(predicate) in simplify, so I use convert with 
additional check.
Looks I should take tree_nop_conversion_p.

> I'm not sure, we don't want general motion of conversions, so I'd
> keep both separate but possibly have similar exceptions
> (!POINTER_TYPE, not OFFSET_TYPE).

Sure, will update in v2.

Pan

-----Original Message-----
From: Richard Biener <[email protected]> 
Sent: Friday, November 14, 2025 4:17 PM
To: Andrew Pinski <[email protected]>
Cc: Li, Pan2 <[email protected]>; [email protected]; 
[email protected]; [email protected]; [email protected]; 
[email protected]; Chen, Ken <[email protected]>; Liu, Hongtao 
<[email protected]>
Subject: Re: [PATCH v1 1/2] Match: Simplify (T1)(a bit_op (T2)b) to (T1)a 
bit_op (T2)b

On Fri, Nov 14, 2025 at 5:12 AM Andrew Pinski
<[email protected]> wrote:
>
> On Thu, Nov 13, 2025 at 7:45 PM <[email protected]> wrote:
> >
> > From: Pan Li <[email protected]>
> >
> > During the match pattern of SAT_U_MUL form 7, we found there is
> > a pattern like below:
> >
> > (nop_convert)(a bit_op (convert b))
> >

So you say nop_convert here but the pattern allows (convert ...)

> > which result in the pattern match of SAT_U_MUL complicated and
> > unintuitive.  According to the suggestion of Richard, we would
> > like to simply it to blew:
> >
> > (convert a) bit_op (convert b)
> >
> > which is more friendly for reading and bit_op.  There are three
> > bit_op here, aka bit_ior, bit_and and bit_xor.
> >
> > gcc/ChangeLog:
> >
> >         * match.pd: Add simplfy to fold outer convert of bit_op
> >         to inner captures.
>
> So we already have:
>  /* In GIMPLE, getting rid of 2 conversions for one new results
>     in smaller IL.  */
>  (simplify
>   (convert (bitop:cs@2 (nop_convert:s @0) @1))
>   (if (GIMPLE
>        && TREE_CODE (@1) != INTEGER_CST
>        && tree_nop_conversion_p (type, TREE_TYPE (@2))
>        && types_match (type, @0)
>        && !POINTER_TYPE_P (TREE_TYPE (@0))
>        && TREE_CODE (TREE_TYPE (@0)) != OFFSET_TYPE)
>    (bitop @0 (convert @1)))))
>
> It looks very similar. Except the inner is only a nop_convert.
> So maybe we should just extend it instead.

I'm not sure, we don't want general motion of conversions, so I'd
keep both separate but possibly have similar exceptions
(!POINTER_TYPE, not OFFSET_TYPE).

> At least the new pattern should be right below this one.

Yes, definitely.  I didn't manage to find the above but I also remembered
we have some ...

> Note you do need a few extra checks like the above pattern dealing
> with pointer types and offset types too (which was added with
> r13-2658-g645ef01a463f15 ).

I think the outermost INTEGRAL_TYPE_P check covers at least
the vector issue, a conversion between vector and non-vector doesn't
exist.

>
> Thanks,
> Andrew Pinski
>
> >
> > Signed-off-by: Pan Li <[email protected]>
> > ---
> >  gcc/match.pd | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index 3cd9ab1e9b0..8d0d7b907ea 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -3653,6 +3653,25 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >          || (wi::eq_p (int_cst_1, itype_max) && wi::eq_p (int_cst_2, 
> > limit_1)))
> >          && wi::eq_p (int_cst_3, otype_max)))))))
> >
> > +/* (nop_convert)(a bit_op (convert b)) => bit_op ((convert a), (convert 
> > b))  */
> > +(if (INTEGRAL_TYPE_P (type))
> > + (for bit_op (bit_ior bit_and bit_xor)
> > +  (simplify
> > +   (convert (bit_op:c @0 (convert @1)))
> > +   (with
> > +    {
> > +      unsigned prec = TYPE_PRECISION (type);
> > +      tree type_c0 = TREE_TYPE (@0);
> > +      tree type_c1 = TREE_TYPE (@1);
> > +      unsigned prec_c0 = TYPE_PRECISION (type_c0);
> > +      unsigned prec_c1 = TYPE_PRECISION (type_c1);
> > +    }
> > +    (if (INTEGRAL_TYPE_P (type_c0)
> > +        && INTEGRAL_TYPE_P (type_c1)
> > +        && prec_c1 > prec
> > +        && prec == prec_c0)
> > +    (bit_op:type (convert @0) (convert @1)))))))
> > +
> >  /* Saturation mult for unsigned integer.  */
> >  (if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type))
> >   (for mult_op (mult widen_mult)
> > --
> > 2.43.0
> >

Reply via email to