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

Jeffrey Law <[email protected]> Mon, 3 Aug 2026 08:27:29 -0600
Newsgroups gmane.comp.gcc.patches
Message-ID <[email protected]>

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.

Jeff