public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] MATCH: Avoid recusive zero_one_valued_p for conversions
@ 2023-09-17  1:44 Andrew Pinski
  2023-09-18  9:08 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Pinski @ 2023-09-17  1:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski

So when VN finds a name which has a nop conversion, it says
both names are equivalent to each other and the valuaization
function for one will return the other. This normally does not
cause any issues as there is no recusive matches. But after
r14-4038-gb975c0dc3be285, there was one added. So we would
do an infinite recusion on the match and never finish.
This fixes the issue (and adds a comment in match.pd) by
for converts just handle one level instead of being recusive
always.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

Note the testcase was reduced from tree-ssa-loop-niter.cc and then
changed slightly into C rather than C++ but it still needs exceptions
turned on get the IR that VN would produce this equivalence relationship
going on. Also had to turn off early inline to force put to be inlined later.

	PR tree-optimization/111435

gcc/ChangeLog:

	* match.pd (zero_one_valued_p): Don't do recusion
	on converts.

gcc/testsuite/ChangeLog:

	* gcc.c-torture/compile/pr111435-1.c: New test.
---
 gcc/match.pd                                   |  8 +++++++-
 .../gcc.c-torture/compile/pr111435-1.c         | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr111435-1.c

diff --git a/gcc/match.pd b/gcc/match.pd
index 97405e6a5c3..887665633d4 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -2188,8 +2188,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
 
 /* A conversion from an zero_one_valued_p is still a [0,1].
    This is useful when the range of a variable is not known */
+/* Note this matches can't be recusive because of the way VN handles
+   nop conversions being equivalent and then recusive between them. */
 (match zero_one_valued_p
- (convert@0 zero_one_valued_p))
+ (convert@0 @1)
+ (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
+      && (TYPE_UNSIGNED (TREE_TYPE (@1))
+	  || TYPE_PRECISION (TREE_TYPE (@1)) > 1)
+      && wi::leu_p (tree_nonzero_bits (@1), 1))))
 
 /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
 (simplify
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr111435-1.c b/gcc/testsuite/gcc.c-torture/compile/pr111435-1.c
new file mode 100644
index 00000000000..afa84dd59dd
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr111435-1.c
@@ -0,0 +1,18 @@
+/* { dg-options "-fexceptions -fno-early-inlining" } */
+/* { dg-require-effective-target exceptions } */
+
+void find_slot_with_hash(const int *);
+
+void put(const int *k, const int *) {
+    find_slot_with_hash(k);
+}
+unsigned len();
+int *address();
+void h(int header, int **bounds) {
+  if (!*bounds)
+    return;
+  unsigned t = *bounds ? len() : 0;
+  int queue_index = t;
+  address()[(unsigned)queue_index] = 0;
+  put(&header, &queue_index);
+}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] MATCH: Avoid recusive zero_one_valued_p for conversions
  2023-09-17  1:44 [PATCH] MATCH: Avoid recusive zero_one_valued_p for conversions Andrew Pinski
@ 2023-09-18  9:08 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-09-18  9:08 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: gcc-patches

On Sun, Sep 17, 2023 at 3:45 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> So when VN finds a name which has a nop conversion, it says
> both names are equivalent to each other and the valuaization
> function for one will return the other. This normally does not
> cause any issues as there is no recusive matches. But after
> r14-4038-gb975c0dc3be285, there was one added. So we would
> do an infinite recusion on the match and never finish.
> This fixes the issue (and adds a comment in match.pd) by
> for converts just handle one level instead of being recusive
> always.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

OK.

> Note the testcase was reduced from tree-ssa-loop-niter.cc and then
> changed slightly into C rather than C++ but it still needs exceptions
> turned on get the IR that VN would produce this equivalence relationship
> going on. Also had to turn off early inline to force put to be inlined later.
>
>         PR tree-optimization/111435
>
> gcc/ChangeLog:
>
>         * match.pd (zero_one_valued_p): Don't do recusion
>         on converts.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.c-torture/compile/pr111435-1.c: New test.
> ---
>  gcc/match.pd                                   |  8 +++++++-
>  .../gcc.c-torture/compile/pr111435-1.c         | 18 ++++++++++++++++++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr111435-1.c
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 97405e6a5c3..887665633d4 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -2188,8 +2188,14 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
>
>  /* A conversion from an zero_one_valued_p is still a [0,1].
>     This is useful when the range of a variable is not known */
> +/* Note this matches can't be recusive because of the way VN handles
> +   nop conversions being equivalent and then recusive between them. */
>  (match zero_one_valued_p
> - (convert@0 zero_one_valued_p))
> + (convert@0 @1)
> + (if (INTEGRAL_TYPE_P (TREE_TYPE (@1))
> +      && (TYPE_UNSIGNED (TREE_TYPE (@1))
> +         || TYPE_PRECISION (TREE_TYPE (@1)) > 1)
> +      && wi::leu_p (tree_nonzero_bits (@1), 1))))
>
>  /* Transform { 0 or 1 } * { 0 or 1 } into { 0 or 1 } & { 0 or 1 }.  */
>  (simplify
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr111435-1.c b/gcc/testsuite/gcc.c-torture/compile/pr111435-1.c
> new file mode 100644
> index 00000000000..afa84dd59dd
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr111435-1.c
> @@ -0,0 +1,18 @@
> +/* { dg-options "-fexceptions -fno-early-inlining" } */
> +/* { dg-require-effective-target exceptions } */
> +
> +void find_slot_with_hash(const int *);
> +
> +void put(const int *k, const int *) {
> +    find_slot_with_hash(k);
> +}
> +unsigned len();
> +int *address();
> +void h(int header, int **bounds) {
> +  if (!*bounds)
> +    return;
> +  unsigned t = *bounds ? len() : 0;
> +  int queue_index = t;
> +  address()[(unsigned)queue_index] = 0;
> +  put(&header, &queue_index);
> +}
> --
> 2.31.1
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-09-18  9:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-17  1:44 [PATCH] MATCH: Avoid recusive zero_one_valued_p for conversions Andrew Pinski
2023-09-18  9:08 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).