Thanks Pinski for comments. > At least the new pattern should be right below this one.
Sure thing, good to known and will update in v2. > Oh one more thing your check on INTEGRAL_TYPE_P is after using > TYPE_PRECISION which might cause issues with vector types. Yes, I promote the INTEGRAL_TYPE_P to outer because without it we may have tree check failure as below. tree check: expected none of vector_type, have vector_type in gimple_simplify_508, I will update the pattern to align the existing one in v2. Pan -----Original Message----- From: Andrew Pinski <[email protected]> Sent: Friday, November 14, 2025 12:15 PM To: Li, Pan2 <[email protected]> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Chen, Ken <[email protected]>; Liu, Hongtao <[email protected]>; [email protected] Subject: Re: [PATCH v1 1/2] Match: Simplify (T1)(a bit_op (T2)b) to (T1)a bit_op (T2)b On Thu, Nov 13, 2025 at 8:12 PM 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)) > > > > 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. > At least the new pattern should be right below this one. > 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 ). Oh one more thing your check on INTEGRAL_TYPE_P is after using TYPE_PRECISION which might cause issues with vector types. And yes, convert for vector types is valid. Maybe this should be element_precision instead. > > 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 > >
