On Wed, Jun 17, 2026 at 11:38 PM Li, Pan2 <[email protected]> wrote: > > Thanks Pinski for comments. > > The outer convert here tries to align the in and out type of the SAT_MUL from > the implementation. > The type of Var[1|2|3] in below should be totally the same to align the > standard name usmul<m>, > like other sat_*, aka we don't have usmul<nm> now. > > Var3 = SAT_MUL(Var1, Var2); > > Therefore the inner convert of @0 and @1 is required, as the form 13 needs > the widen type for mul. > > Do we need to adjust the sematics of SAT_MUL? Or keep it as is for now.
Keep it as is for now. But we definitely need to it in the back of our minds that we should be able to handle this case without the outer convert. So this version of the patch is ok. (Also the testsuite patch is ok too). Thanks, Andrea > > Pan > > > -----Original Message----- > From: Andrea Pinski <[email protected]> > Sent: Thursday, June 18, 2026 11:39 AM > 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 v2 1/3] Match: Support unsigned scalar SAT_MUL form 13 > > On Wed, Jun 17, 2026 at 5:47 PM <[email protected]> wrote: > > > > From: Pan Li <[email protected]> > > > > This patch would like to try to match the the unsigned > > SAT_MUL form 13, aka below > > > > #define DEF_SAT_U_MUL_FMT_13(NT, WT) \ > > NT __attribute__((noinline)) \ > > sat_u_mul_##NT##_from_##WT##_fmt_13 (NT a, NT b) \ > > { \ > > WT x = (WT)a * (WT)b; \ > > if ((x >> sizeof(a) * 8) != 0) \ > > return (NT)-1; \ > > else \ > > return (NT)x; \ > > } > > > > while WT is uint128_t, uint64_t, uint32_t and uint16_t, and > > NT is uint64_t, uint32_t, uint16_t or uint8_t. > > > > gcc/ChangeLog: > > > > * match-sat-alu.pd: Add pattern for unsigned scalar > > SAT_MUL form 13. > > > > Signed-off-by: Pan Li <[email protected]> > > --- > > gcc/match-sat-alu.pd | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > > > > diff --git a/gcc/match-sat-alu.pd b/gcc/match-sat-alu.pd > > index 6145a0c9948..c47d73f958d 100644 > > --- a/gcc/match-sat-alu.pd > > +++ b/gcc/match-sat-alu.pd > > @@ -524,6 +524,31 @@ along with GCC; see the file COPYING3. If not see > > bool c2_is_type_precision_p = c2 == prec; > > } > > (if (widen_prec > prec && c2_is_type_precision_p && c4_is_max_p))))) > > + (match (unsigned_integer_sat_mul @0 @1) > > + /* SAT_U_MUL (X, Y) = { > > + WT x = (WT)a * (WT)b; > > + if ((x >> sizeof(a) * 8) != 0) > > + return (T)-1; > > + else > > + return (T)x; > > + } > > + while WT is uint128_t, T is uint8_t, uint16_t, uint32_t or uint64_t. > > */ > > + (convert (cond^ (ne (rshift (mult:c@3 (convert @0) (convert @1)) > > + INTEGER_CST@2) > > + integer_zerop) > > + INTEGER_CST@4 @3)) > > + (if (types_match (type, @0, @1) && tree_fits_uhwi_p (@2)) > > + (with > > + { > > + unsigned prec = TYPE_PRECISION (type); > > + unsigned widen_prec = TYPE_PRECISION (TREE_TYPE (@3)); > > + wide_int c4 = wi::to_wide (@4); > > + wide_int max = wi::mask (prec, false, widen_prec); > > + bool c4_is_max_p = wi::eq_p (c4, max); > > + unsigned c2 = tree_to_uhwi (@2); > > + bool c2_is_type_precision_p = c2 == prec; > > + } > > + (if (widen_prec > prec && c2_is_type_precision_p && c4_is_max_p))))) > > Hmm, is there a way to match without the outer convert here? Because > even without that convert it will be unsigned sat mult with a type > precision of @2. > Also I think the inner converts can be removed if we fit into unsigned > precision of that same type. > There is a ranger helper function for ranges to answer that question. > > I am not saying we need to remove those currently just that it might be > useful. > > An example there would be: > ``` > uint32_t f (uint32_t a, uint32_t b) { > uint32_t x = ((uint32_t)(uint16_t)a) * (uint32_t)(uint16_t)b; > if ((x >> sizeof(uint16_t) * 8) != 0) > return (uint16_t)-1; > else > return (uint16_t)x; > } > uint32_t f1 (uint16_t a, uint16_t b) { > uint32_t x = ((uint32_t)(uint16_t)a) * (uint32_t)(uint16_t)b; > if ((x >> sizeof(uint16_t) * 8) != 0) > return (uint16_t)-1; > else > return (uint16_t)x; > } > ``` > > I noticed that LLVM is not able to handle f but does handle f1. > > > > (match (unsigned_integer_sat_mul @0 @1) > > /* SAT_U_MUL (X, Y) = { > > WT x = (WT)a * (WT)b; > > -- > > 2.43.0 > >
