diff mbox series

match: Use tree_expr_nonnegative_p for (X / Y) (==, !=) 0 -> X (<,>=) Y [PR125738]

Message ID CACAb0qegy25jT8of8FLRqJjeGssbziTLW39gDkax04C37f9ChQ@mail.gmail.com
State New
Headers
Series match: Use tree_expr_nonnegative_p for (X / Y) (==, !=) 0 -> X (<,>=) Y [PR125738] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap fail Patch failed to apply

Commit Message

Kael Andrew Franco July 7, 2026, 5:13 p.m. UTC 
  From cf6e4302cddd979315a58008e77f0c869280318c Mon Sep 17 00:00:00 2001
From: Kael Andrew Alonzo Franco <kaelfandrew@gmail.com>
Date: Tue, 7 Jul 2026 13:09:01 -0400
Subject: [PATCH] match: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X
 (<,>=) Y [PR125738]

TYPE_UNSIGNED did not cover non-negative X and Y so use tree_expr_nonnegative_p
to relax condition on optimizations.
gcc/testsuite/gcc.dg/tree-ssa/pr64130.c fails because match.pd
optimize funsigned () before
evrp pass. Fix this by removing funsigned () and one scan-tree-dump.

Bootstrapped and tested on x86_64-pc-linux-gnu.

	PR tree-optimization/125738
	PR tree-optimization/64130

gcc/ChangeLog:

	* match.pd: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X (<,>=) Y.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr125738.c: New test.
	* gcc.dg/tree-ssa/pr64130.c: Remove scan-tree-dump [2, 8589934591].
	(funsigned): Remove.
	(funsigned2): Rename to funsigned.

Signed-off-by: Kael Franco <kaelfandrew@gmail.com>
---
 gcc/match.pd                            |  9 ++++-----
 gcc/testsuite/gcc.dg/pr125738.c         | 22 ++++++++++++++++++++++
 gcc/testsuite/gcc.dg/tree-ssa/pr64130.c |  6 ------
 3 files changed, 26 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr125738.c

Comments

Jeffrey Law July 9, 2026, 2:14 p.m. UTC | #1 
On 7/7/2026 11:13 AM, Kael Andrew Franco wrote:
>  From cf6e4302cddd979315a58008e77f0c869280318c Mon Sep 17 00:00:00 2001
> From: Kael Andrew Alonzo Franco <kaelfandrew@gmail.com>
> Date: Tue, 7 Jul 2026 13:09:01 -0400
> Subject: [PATCH] match: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X
>   (<,>=) Y [PR125738]
>
> TYPE_UNSIGNED did not cover non-negative X and Y so use tree_expr_nonnegative_p
> to relax condition on optimizations.
> gcc/testsuite/gcc.dg/tree-ssa/pr64130.c fails because match.pd optimize funsigned () before
> evrp pass. Fix this by removing funsigned () and one scan-tree-dump.
>
> Bootstrapped and tested on x86_64-pc-linux-gnu.
>
> 	PR tree-optimization/125738
> 	PR tree-optimization/64130
>
> gcc/ChangeLog:
>
> 	* match.pd: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X (<,>=) Y.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/pr125738.c: New test.
> 	* gcc.dg/tree-ssa/pr64130.c: Remove scan-tree-dump [2, 8589934591].
> 	(funsigned): Remove.
> 	(funsigned2): Rename to funsigned.
So conceptually good.  Throwing it into an LLM did uncover one concern 
worth relaying.

In particular the old code checked TYPE_UNSIGNED, so it'd work for 
vector types.  tree_expr_nonnegative_p always returns false for 
vectors.  So we could end up with a code quality regression here (and 
other places where we've converted to tree_expr_nonnegative_p).

So I think there's a question here.  Do we want tree_expr_nonnegative_p 
to return tree for vector types that  are unsigned.  Conceptually that's 
a good thing, but may have unintended consequences elsewhere.   If no, 
then we probably want to use something like like TYPE_UNSIGNED || 
tree_expr_nonnegative_p in a test rather than in the capture.  I'd tend 
to lean towards the former, but that' s without any real investigation.

A much smaller issue.  I would have at least considered leaving the 
original funsigned test in place and instead checked the dump for the 
optimized form.  It essentially turns into an additional test to this 
patch.   But again, this is small.

Richi, Andrea thoughts?

jeff
Richard Biener July 10, 2026, 12:55 p.m. UTC | #2 
On Thu, Jul 9, 2026 at 4:15 PM Jeffrey Law <jefflaw@qti.qualcomm.com> wrote:
>
> On 7/7/2026 11:13 AM, Kael Andrew Franco wrote:
> >  From cf6e4302cddd979315a58008e77f0c869280318c Mon Sep 17 00:00:00 2001
> > From: Kael Andrew Alonzo Franco <kaelfandrew@gmail.com>
> > Date: Tue, 7 Jul 2026 13:09:01 -0400
> > Subject: [PATCH] match: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X
> >   (<,>=) Y [PR125738]
> >
> > TYPE_UNSIGNED did not cover non-negative X and Y so use tree_expr_nonnegative_p
> > to relax condition on optimizations.
> > gcc/testsuite/gcc.dg/tree-ssa/pr64130.c fails because match.pd optimize funsigned () before
> > evrp pass. Fix this by removing funsigned () and one scan-tree-dump.
> >
> > Bootstrapped and tested on x86_64-pc-linux-gnu.
> >
> >       PR tree-optimization/125738
> >       PR tree-optimization/64130
> >
> > gcc/ChangeLog:
> >
> >       * match.pd: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X (<,>=) Y.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/pr125738.c: New test.
> >       * gcc.dg/tree-ssa/pr64130.c: Remove scan-tree-dump [2, 8589934591].
> >       (funsigned): Remove.
> >       (funsigned2): Rename to funsigned.
> So conceptually good.  Throwing it into an LLM did uncover one concern
> worth relaying.
>
> In particular the old code checked TYPE_UNSIGNED, so it'd work for
> vector types.  tree_expr_nonnegative_p always returns false for
> vectors.  So we could end up with a code quality regression here (and
> other places where we've converted to tree_expr_nonnegative_p).
>
> So I think there's a question here.  Do we want tree_expr_nonnegative_p
> to return tree for vector types that  are unsigned.  Conceptually that's
> a good thing, but may have unintended consequences elsewhere.   If no,
> then we probably want to use something like like TYPE_UNSIGNED ||
> tree_expr_nonnegative_p in a test rather than in the capture.  I'd tend
> to lean towards the former, but that' s without any real investigation.
>
> A much smaller issue.  I would have at least considered leaving the
> original funsigned test in place and instead checked the dump for the
> optimized form.  It essentially turns into an additional test to this
> patch.   But again, this is small.
>
> Richi, Andrea thoughts?

I think it returns true even for unsigned vectors via the
tree_single_nonnegative_p fallthru for SSA_NAME.

>
> jeff
Jeffrey Law July 10, 2026, 2:02 p.m. UTC | #3 
On 7/10/2026 6:55 AM, Richard Biener wrote:
> On Thu, Jul 9, 2026 at 4:15 PM Jeffrey Law <jefflaw@qti.qualcomm.com> wrote:
>> On 7/7/2026 11:13 AM, Kael Andrew Franco wrote:
>>>   From cf6e4302cddd979315a58008e77f0c869280318c Mon Sep 17 00:00:00 2001
>>> From: Kael Andrew Alonzo Franco <kaelfandrew@gmail.com>
>>> Date: Tue, 7 Jul 2026 13:09:01 -0400
>>> Subject: [PATCH] match: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X
>>>    (<,>=) Y [PR125738]
>>>
>>> TYPE_UNSIGNED did not cover non-negative X and Y so use tree_expr_nonnegative_p
>>> to relax condition on optimizations.
>>> gcc/testsuite/gcc.dg/tree-ssa/pr64130.c fails because match.pd optimize funsigned () before
>>> evrp pass. Fix this by removing funsigned () and one scan-tree-dump.
>>>
>>> Bootstrapped and tested on x86_64-pc-linux-gnu.
>>>
>>>        PR tree-optimization/125738
>>>        PR tree-optimization/64130
>>>
>>> gcc/ChangeLog:
>>>
>>>        * match.pd: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X (<,>=) Y.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>        * gcc.dg/pr125738.c: New test.
>>>        * gcc.dg/tree-ssa/pr64130.c: Remove scan-tree-dump [2, 8589934591].
>>>        (funsigned): Remove.
>>>        (funsigned2): Rename to funsigned.
>> So conceptually good.  Throwing it into an LLM did uncover one concern
>> worth relaying.
>>
>> In particular the old code checked TYPE_UNSIGNED, so it'd work for
>> vector types.  tree_expr_nonnegative_p always returns false for
>> vectors.  So we could end up with a code quality regression here (and
>> other places where we've converted to tree_expr_nonnegative_p).
>>
>> So I think there's a question here.  Do we want tree_expr_nonnegative_p
>> to return tree for vector types that  are unsigned.  Conceptually that's
>> a good thing, but may have unintended consequences elsewhere.   If no,
>> then we probably want to use something like like TYPE_UNSIGNED ||
>> tree_expr_nonnegative_p in a test rather than in the capture.  I'd tend
>> to lean towards the former, but that' s without any real investigation.
>>
>> A much smaller issue.  I would have at least considered leaving the
>> original funsigned test in place and instead checked the dump for the
>> optimized form.  It essentially turns into an additional test to this
>> patch.   But again, this is small.
>>
>> Richi, Andrea thoughts?
> I think it returns true even for unsigned vectors via the
> tree_single_nonnegative_p fallthru for SSA_NAME.
So I'm not entirely sure what I was looking at yesterday; I know after 
seeing that from the LLM I dove into what I thought was 
tree_expr_nonnegative_p thinking that the LLM must have been wrong. What 
I saw had a early out based on types.  But looking today after your 
message, I don't see that structure.   The only conclusion I can come to 
is I was in the wrong routine or looking at some old bits (I was 
wandering as far back as gcc-13).  Too many machines, too many source 
trees...

Anyway, I'll pick this back up for Kael.

jeff
Jeffrey Law July 10, 2026, 2:31 p.m. UTC | #4 
On 7/7/2026 11:13 AM, Kael Andrew Franco wrote:
>  From cf6e4302cddd979315a58008e77f0c869280318c Mon Sep 17 00:00:00 2001
> From: Kael Andrew Alonzo Franco <kaelfandrew@gmail.com>
> Date: Tue, 7 Jul 2026 13:09:01 -0400
> Subject: [PATCH] match: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X
>   (<,>=) Y [PR125738]
>
> TYPE_UNSIGNED did not cover non-negative X and Y so use tree_expr_nonnegative_p
> to relax condition on optimizations.
> gcc/testsuite/gcc.dg/tree-ssa/pr64130.c fails because match.pd optimize funsigned () before
> evrp pass. Fix this by removing funsigned () and one scan-tree-dump.
>
> Bootstrapped and tested on x86_64-pc-linux-gnu.
>
> 	PR tree-optimization/125738
> 	PR tree-optimization/64130
>
> gcc/ChangeLog:
>
> 	* match.pd: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X (<,>=) Y.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/pr125738.c: New test.
> 	* gcc.dg/tree-ssa/pr64130.c: Remove scan-tree-dump [2, 8589934591].
> 	(funsigned): Remove.
> 	(funsigned2): Rename to funsigned.
>
> Signed-off-by: Kael Franco <kaelfandrew@gmail.com>
Thanks.  I've pushed this to the trunk.

Given the frequency of contributions you've started making it would make 
sense to go ahead and get you write permissions for the project so that 
you can commit your own approved patches.    This page should have a 
link to the form.  Use jeffreyalaw@gmail.com as the person sponsoring 
your request.

https://gcc.gnu.org/gitwrite.html#authenticated

Thanks again!

jeff
Kael Andrew Franco July 11, 2026, 6:26 p.m. UTC | #5 
Done. Please let me know if I need to do anything else.


On Fri, Jul 10, 2026 at 10:31 AM Jeffrey Law <jeffrey.law@oss.qualcomm.com>
wrote:

>
>
> On 7/7/2026 11:13 AM, Kael Andrew Franco wrote:
> >  From cf6e4302cddd979315a58008e77f0c869280318c Mon Sep 17 00:00:00 2001
> > From: Kael Andrew Alonzo Franco <kaelfandrew@gmail.com>
> > Date: Tue, 7 Jul 2026 13:09:01 -0400
> > Subject: [PATCH] match: Use tree_expr_nonnegative_p for (X / Y) (==,!=)
> 0 -> X
> >   (<,>=) Y [PR125738]
> >
> > TYPE_UNSIGNED did not cover non-negative X and Y so use
> tree_expr_nonnegative_p
> > to relax condition on optimizations.
> > gcc/testsuite/gcc.dg/tree-ssa/pr64130.c fails because match.pd optimize
> funsigned () before
> > evrp pass. Fix this by removing funsigned () and one scan-tree-dump.
> >
> > Bootstrapped and tested on x86_64-pc-linux-gnu.
> >
> >       PR tree-optimization/125738
> >       PR tree-optimization/64130
> >
> > gcc/ChangeLog:
> >
> >       * match.pd: Use tree_expr_nonnegative_p for (X / Y) (==,!=) 0 -> X
> (<,>=) Y.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/pr125738.c: New test.
> >       * gcc.dg/tree-ssa/pr64130.c: Remove scan-tree-dump [2, 8589934591].
> >       (funsigned): Remove.
> >       (funsigned2): Rename to funsigned.
> >
> > Signed-off-by: Kael Franco <kaelfandrew@gmail.com>
> Thanks.  I've pushed this to the trunk.
>
> Given the frequency of contributions you've started making it would make
> sense to go ahead and get you write permissions for the project so that
> you can commit your own approved patches.    This page should have a
> link to the form.  Use jeffreyalaw@gmail.com as the person sponsoring
> your request.
>
> https://gcc.gnu.org/gitwrite.html#authenticated
>
> Thanks again!
>
> jeff
>
diff mbox series

Patch

diff --git a/gcc/match.pd b/gcc/match.pd
index a7cec25dbad..81c32b4a5a7 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3095,15 +3095,14 @@  DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
    (op @1 { build_zero_cst (TREE_TYPE (@1)); }))))

 /* Transform:
-   (X / Y) == 0 -> X < Y if X, Y are unsigned.
-   (X / Y) != 0 -> X >= Y, if X, Y are unsigned.  */
+   (X / Y) == 0 -> X < Y if X, Y are non-negative.
+   (X / Y) != 0 -> X >= Y, if X, Y are non-negative.  */
 (for cmp (eq ne)
      ocmp (lt ge)
  (simplify
-  (cmp (trunc_div @0 @1) integer_zerop)
-  (if (TYPE_UNSIGNED (TREE_TYPE (@0))
+  (cmp (trunc_div tree_expr_nonnegative_p@0
tree_expr_nonnegative_p@1) integer_zerop)
        /* Complex ==/!= is allowed, but not </>=.  */
-       && TREE_CODE (TREE_TYPE (@0)) != COMPLEX_TYPE
+  (if (TREE_CODE (TREE_TYPE (@0)) != COMPLEX_TYPE
        && (VECTOR_TYPE_P (type) || !VECTOR_TYPE_P (TREE_TYPE (@0))))
    (ocmp @0 @1))))

diff --git a/gcc/testsuite/gcc.dg/pr125738.c b/gcc/testsuite/gcc.dg/pr125738.c
new file mode 100644
index 00000000000..4f675c0fc2a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr125738.c
@@ -0,0 +1,22 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+int
+aa_div_bb_eq_0 (unsigned short a, unsigned short b)
+{
+  int aa = a;
+  int bb = b;
+  int c = aa / bb;
+  return (c == 0) == (a < b);
+}
+
+int
+aa_div_bb_ne_0 (unsigned short a, unsigned short b)
+{
+  int aa = a;
+  int bb = b;
+  int c = aa / bb;
+  return (c != 0) == (a >= b);
+}
+
+/* { dg-final { scan-tree-dump-times "return 1;" 2 "optimized" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr64130.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr64130.c
index b694ec171c1..b9cd65ba8b8 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr64130.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr64130.c
@@ -4,16 +4,10 @@ 
 __extension__ typedef __UINT32_TYPE__ uint32_t;

 int funsigned (uint32_t a)
-{
-  return 0x1ffffffffL / a == 0;
-}
-
-int funsigned2 (uint32_t a)
 {
   if (a < 1) return 1;
   return (-1 * 0x1ffffffffL) / a == 0;
 }

-/* { dg-final { scan-tree-dump "int \\\[2, 8589934591\\\]" "evrp" } } */
 /* { dg-final { scan-tree-dump "int \\\[-8589934591, -2\\\]" "evrp" } } */