Re: [PING][PATCH] match: 1 / X -> X == 1 for positive X [PR125735]

Andrew Pinski <[email protected]> Mon, 3 Aug 2026 07:56:32 -0700
Newsgroups gmane.comp.gcc.patches
Message-ID <CA+=Sn1kxVKsXDj0tHwUhckrD3YSU1L-=ognNq7pQ8HfDVkQFGA@mail.gmail.com>
On Mon, Aug 3, 2026, 7:32 AM Jeffrey Law <[email protected]>
wrote:

>
>
> On 8/3/2026 8:14 AM, Richard Biener wrote:
> > On Mon, Aug 3, 2026 at 3:57 PM Jeffrey Law <[email protected]>
> wrote:
> >>
> >>
> >> On 7/31/2026 7:51 AM, Kael Andrew Franco wrote:
> >>> Ping of
> https://gcc.gnu.org/pipermail/gcc-patches/2026-July/724387.html.
> >>> Updated patch still works:
> >>>   From a8dad89050597f075c197bba9b0da2703277671e Mon Sep 17 00:00:00
> 2001
> >>> From: Kael Andrew Alonzo Franco <[email protected]>
> >>> Date: Thu, 30 Jul 2026 21:59:35 -0400
> >>> Subject: [PATCH] match: 1 / X -> X == 1 for positive X [PR125735]
> >>>
> >>> TYPE_UNSIGNED (type) doesn't cover positive signed types and
> >>> tree_expr_nonnegative_p () doesn't work so use vr0.nonnegative_p ().
> >>>
> >>> Bootstrapped and tested on x86_64-pc-linux-gnu.
> >>>
> >>>        PR tree-optimization/125735
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>>        * match.pd: 1 / X -> X == 1 for positive X. [PR125735]
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>>        * gcc.dg/pr125735.c: New test.
> >>>
> >>> Signed-off-by: Kael Andrew Franco <[email protected]>
> >> Looks good.  Just one nit:
> >>
> >>
> >>> @@ -640,13 +640,24 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >>>          && TYPE_PRECISION (type) > 1
> >>>          && !integer_zerop (@1)
> >>>          && (!flag_non_call_exceptions || tree_expr_nonzero_p (@1)))
> >>> -  (if (TYPE_UNSIGNED (type))
> >>> +  (with {
> >>> +    bool positive_p = TYPE_UNSIGNED (type);
> >>> +#if GIMPLE
> >>> +    int_range_max vr0;
> >>> +    wide_int lower_bnd;
> >>> +    if (!positive_p
> >>> +        && gimple_match_range_of_expr (vr0, @1)
> >>> +        && vr0.nonnegative_p ())
> >>> +      positive_p = true;
> >>> +#endif
> >> lower_bnd isn't used.  You should just remove it.
> >>
> >> OK with that change.  No need to go through another review round.
> > I'm not disagreeing, but given match is supposed to be IL agnostic
> > and we're accumulating more and more uses like the above I'd like
> > to see some abstraction around this.  The above _should_ be
> > equal to tree_expr_nonnegative_p (@1), no?
> We certainly do seem to be accumulating a lot of this stuff.
>
> Kael, if you haven't committed already, let's hold and see if we can
> push the support up a level.  Interested in doing some experimentation
> there Kael?
>
> tree_expr_nonnegative_p is just a dispatcher for binary, unary, etc
> variants.  We could query range before the dispatcher and fall into the
> dispatcher if Ranger doesn't give us something useful.  Or we could push
> it into tree_binary_nonnegative_p and friends.  No strong opinions on
> those options at this time, though I probably lean ever so slightly
> towards putting it into the tree_*_nonnegative_p routines since at least
> some are public and could be called from outside fold-const and we get
> the benefit of catching the new cases.
>

I had a patch for this.
See
https://inbox.sourceware.org/gcc-patches/[email protected]/

I should be able to work on finishing this week.




> Jeff
>
>
>