RE: [PATCH v2] match.pd: Fold umin(a, 1) | umin(b, 1) into umin(a | b, 1) [PR126646]

AMD General

Hi

> -----Original Message-----
> From: Andrea Pinski <[email protected]>
> Sent: Friday, August 14, 2026 11:43 AM
> To: Roy, Reshma <[email protected]>
> Cc: [email protected]; Kumar, Venkataramanan
> <[email protected]>
> Subject: Re: [PATCH v2] match.pd: Fold umin(a, 1) | umin(b, 1) into umin(a | b, 1)
> [PR126646]
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On Thu, Aug 13, 2026 at 10:28 PM <[email protected]> wrote:
> >
> > From: Reshma Roy <[email protected]>
> >
> > gcc/ChangeLog:
> >
> >         PR tree-optimization/126646
> >         * match.pd: Fold umin(a, 1) | umin(b, 1) into umin(a | b, 1).
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/tree-ssa/pr126646-1.c: New test.
> >         * gcc.dg/tree-ssa/pr126646-2.c: New test.
>
> Ok, see below about being ok about the testcase.

Committed to gcc trunk at
 https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b190388bad47b1d3507abd52dc2a07f39a899984

regards,
Venkat.

>
> >
> > ---
> > Hi,
> >   The patch is updated as per the comments and added a new test case for
> >   the same. My comments are added inline to the email.
> >
> >   Bootstrapped and tested on x86_64-linux
> >
> > > -----Original Message-----
> > > From: Andrea Pinski <[email protected]>
> > > Sent: Wednesday, August 12, 2026 9:59 AM
> > > To: Roy, Reshma <[email protected]>
> > > Cc: [email protected]; Kumar, Venkataramanan
> > > <[email protected]>
> > > Subject: Re: [PATCH] match.pd: Fold umin(a, 1) | umin(b, 1) into
> > > umin(a | b, 1) [PR126646]
> > >
> > > [You don't often get email from [email protected].
> > > Learn why this is important at
> > > https://aka.ms/LearnAboutSenderIdentification ]
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > On Tue, Aug 11, 2026 at 8:50 PM <[email protected]> wrote:
> > > >
> > > > From: Reshma Roy <[email protected]>
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > >         PR tree-optimization/126646
> > > >         * match.pd: Fold umin(a, 1) | umin(b, 1) into umin(a | b, 1).
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > >         * gcc.dg/tree-ssa/pr126646-1.c: New test.
> > > > ---
> > > >
> > > > Hi,
> > > >   This patch fixes the missed optimization opportunity reported in
> > > >   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126646.
> > > >   For the following snippet of code, gcc generates 2 MIN_EXPR.
> > > >   unsigned f(unsigned a, unsigned b)
> > > >   {
> > > >     unsigned t = 1;
> > > >     a = a < t ? a : t;
> > > >     b = b < t ? b : t;
> > > >     return a | b;
> > > >   }
> > > >  But this can be optimized with just one MIN_EXPR since
> > > >  umin(a,1) | umin(b,1) can be simplified to umin(a|b, 1).
> > > >
> > > >  Bootstrapped and tested on x86_64-linux.
> > > >
> > > > Thanks,
> > > >   Reshma
> > > >
> > > >  gcc/match.pd                               |  5 +++++
> > > >  gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c | 15 +++++++++++++++
> > > >  2 files changed, 20 insertions(+)  create mode 100644
> > > > gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > > >
> > > > diff --git a/gcc/match.pd b/gcc/match.pd index
> > > > 50e73177022..7347b83ec4f 100644
> > > > --- a/gcc/match.pd
> > > > +++ b/gcc/match.pd
> > > > @@ -4860,6 +4860,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > > >    (bit_not (minmax:cs (bit_not @0) @1))
> > > >    (maxmin @0 (bit_not @1))))
> > > >
> > > > +/* umin (a, 1) | umin (b, 1) -> umin (a | b, 1).  */  (simplify
> > > > +  (bit_ior (min @0 integer_onep@2) (min @1 @2))
> > > > +    (if (TYPE_UNSIGNED (type))
> > >
> > > Instead of TYPE_UNSIGNED here; I think it might be a good idea to
> > > use tree_expr_nonnegative_p on both @0 and @1..
> > > tree_expr_nonnegative_p does return true for TYPE_UNSIGNED already
> > > but it also returns true when the argument is known to be zero or positive.
> > Agreed, updated to use tree_expr_nonnegative_p (@0) &&
> > tree_expr_nonnegative_p (@1). This is strictly more general as it
> > covers unsigned types (where tree_expr_nonnegative_p returns true by
> > definition) and also signed types when the operands are non-negative.
> > >
> > > This allows for say:
> > > ```
> > > int f(int a, int b)
> > > {
> > >   int c = 0;
> > >   if (a >= 0 && b >= 0)
> > >     c = (a > 1 ? 1 : a) | (b > 1 ? 1 : b);
> > >   return c;
> > > }
> > > ```
> > Understood the test case scenario. But the above test case doesn't
> > apply the new guard in practice because by the time match.pd sees the
> > MIN_EXPRs, the condition a >= 0 && b >= 0 has already been folded into
> > (a | b) >= 0. The dominating condition is now on the combined OR, so
> > tree_expr_nonnegative_p on a and b individually can't prove
> > non-negativity. Hence I have added a different test case
> > (pr126646-2.c) that uses __builtin_abs, where non-negativity is
> > implied in the expression itself. Hope that is fine. Let me know your thoughts.
>
> Yes, the different testcase using abs is fine. I forgot the ranger is NOT always
> enabled for forwprop so it is not always used for tree_expr_nonnegative_p;
> instead tree_expr_nonnegative_p will use the global range. That is something
> which I have been thinking about changing but I always get side tracked.
>
>
> >
> > > Which then should optimize to just:
> > >   _10 = a_6(D) | b_7(D);
> > >   c_8 = MIN_EXPR <_10, 1>;
> > >   _11 = MAX_EXPR <c_8, 0>;
> > >
> > >
> > > Thanks,
> > > Andrea
> > >
> > > > +     (min (bit_ior @0 @1) @2)))
> > > >  /* MIN (X, Y) == X -> X <= Y  */
> > > >  /* MIN (X, Y) < X -> X > Y  */
> > > >  /* MIN (X, Y) >= X -> X <= Y  */
> > > > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > > > b/gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > > > new file mode 100644
> > > > index 00000000000..447a1739b5f
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > > > @@ -0,0 +1,15 @@
> > > > +/* { dg-do compile } */
> > > > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > > > +
> > > > +/* The test case should produce only one min expr.  */
> > > > +/* umin(a,1) | umin(b,1) -> umin(a|b, 1).  */
> > > > +
> > > > +unsigned min_or (unsigned a, unsigned b) {
> > > > +  unsigned t = 1;
> > > > +  a = a < t ? a : t;
> > > > +  b = b < t ? b : t;
> > > > +  return a | b;
> > > > +}
> > > > +
> > > > +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "optimized" } }
> > > > +*/
> > > > --
> > > > 2.34.1
> > > >
> >
> > Thanks,
> >  Reshma
> >
> >
> >  gcc/match.pd                               |  5 +++++
> >  gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c | 15 +++++++++++++++
> > gcc/testsuite/gcc.dg/tree-ssa/pr126646-2.c | 13 +++++++++++++
> >  3 files changed, 33 insertions(+)
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr126646-2.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd index
> > 5fc89426880..be2bc0dd8df 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -4887,6 +4887,11 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> >    (bit_not (minmax:cs (bit_not @0) @1))
> >    (maxmin @0 (bit_not @1))))
> >
> > +/* umin (a, 1) | umin (b, 1) -> umin (a | b, 1).  */  (simplify
> > +  (bit_ior (min @0 integer_onep@2) (min @1 @2))
> > +    (if (tree_expr_nonnegative_p (@0) && tree_expr_nonnegative_p (@1))
> > +     (min (bit_ior @0 @1) @2)))
> >  /* MIN (X, Y) == X -> X <= Y  */
> >  /* MIN (X, Y) < X -> X > Y  */
> >  /* MIN (X, Y) >= X -> X <= Y  */
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > new file mode 100644
> > index 00000000000..447a1739b5f
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr126646-1.c
> > @@ -0,0 +1,15 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > +
> > +/* The test case should produce only one min expr.  */
> > +/* umin(a,1) | umin(b,1) -> umin(a|b, 1).  */
> > +
> > +unsigned min_or (unsigned a, unsigned b) {
> > +  unsigned t = 1;
> > +  a = a < t ? a : t;
> > +  b = b < t ? b : t;
> > +  return a | b;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "optimized" } } */
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr126646-2.c
> > b/gcc/testsuite/gcc.dg/tree-ssa/pr126646-2.c
> > new file mode 100644
> > index 00000000000..3b47eee9656
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr126646-2.c
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-optimized" } */
> > +
> > +int f (int a, int b)
> > +{
> > +  int x = __builtin_abs (a);
> > +  int y = __builtin_abs (b);
> > +  int p = x < 1 ? x : 1;
> > +  int q = y < 1 ? y : 1;
> > +  return p | q;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-times "MIN_EXPR" 1 "optimized" } } */
> > --
> > 2.34.1
> >