* [PATCH] Fix PR 110874: infinite loop in gimple_bitwise_inverted_equal_p with fre
@ 2023-08-03 2:34 Andrew Pinski
2023-08-03 7:21 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Pinski @ 2023-08-03 2:34 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
So I didn't expect valueization to cause calling gimple_nop_convert
to iterate between 2 different SSA names causing an infinite loop
in gimple_bitwise_inverted_equal_p.
So we should cause a bound on gimple_bitwise_inverted_equal_p calling
gimple_nop_convert and only look through one rather than always.
OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
gcc/ChangeLog:
PR tree-optimization/110874
* gimple-match-head.cc (gimple_bitwise_inverted_equal_p):
Add new argument, again with default value of true.
Don't try gimple_nop_convert if again is false.
Update call to gimple_bitwise_inverted_equal_p for
new argument.
gcc/testsuite/ChangeLog:
PR tree-optimization/110874
* gcc.c-torture/compile/pr110874-a.c: New test.
---
gcc/gimple-match-head.cc | 14 +++++++++-----
.../gcc.c-torture/compile/pr110874-a.c | 17 +++++++++++++++++
2 files changed, 26 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
index b1e96304d7c..e91aaab86dd 100644
--- a/gcc/gimple-match-head.cc
+++ b/gcc/gimple-match-head.cc
@@ -273,7 +273,7 @@ gimple_bitwise_equal_p (tree expr1, tree expr2, tree (*valueize) (tree))
/* Helper function for bitwise_equal_p macro. */
static inline bool
-gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree))
+gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree), bool again = true)
{
if (expr1 == expr2)
return false;
@@ -285,12 +285,16 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree)
return false;
tree other;
- if (gimple_nop_convert (expr1, &other, valueize)
- && gimple_bitwise_inverted_equal_p (other, expr2, valueize))
+ if (again
+ && gimple_nop_convert (expr1, &other, valueize)
+ && other != expr1
+ && gimple_bitwise_inverted_equal_p (other, expr2, valueize, false))
return true;
- if (gimple_nop_convert (expr2, &other, valueize)
- && gimple_bitwise_inverted_equal_p (expr1, other, valueize))
+ if (again
+ && gimple_nop_convert (expr2, &other, valueize)
+ && other != expr2
+ && gimple_bitwise_inverted_equal_p (expr1, other, valueize, false))
return true;
if (TREE_CODE (expr1) != SSA_NAME
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c b/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
new file mode 100644
index 00000000000..b314410a892
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
@@ -0,0 +1,17 @@
+struct S1 {
+ unsigned f0;
+};
+static int g_161;
+void func_109(unsigned g_227, unsigned t) {
+ struct S1 l_178;
+ int l_160 = 0x1FAE99D5L;
+ int *l_230[] = {&l_160};
+ if (l_160) {
+ for (l_178.f0 = -7; l_178.f0;) {
+ ++g_227;
+ break;
+ }
+ (g_161) = g_227;
+ }
+ (g_161) &= t;
+}
--
2.31.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PR 110874: infinite loop in gimple_bitwise_inverted_equal_p with fre
2023-08-03 2:34 [PATCH] Fix PR 110874: infinite loop in gimple_bitwise_inverted_equal_p with fre Andrew Pinski
@ 2023-08-03 7:21 ` Richard Biener
2023-08-03 9:23 ` Andrew Pinski
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2023-08-03 7:21 UTC (permalink / raw)
To: Andrew Pinski; +Cc: gcc-patches
On Thu, Aug 3, 2023 at 4:34 AM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> So I didn't expect valueization to cause calling gimple_nop_convert
> to iterate between 2 different SSA names causing an infinite loop
> in gimple_bitwise_inverted_equal_p.
> So we should cause a bound on gimple_bitwise_inverted_equal_p calling
> gimple_nop_convert and only look through one rather than always.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
> PR tree-optimization/110874
> * gimple-match-head.cc (gimple_bitwise_inverted_equal_p):
> Add new argument, again with default value of true.
> Don't try gimple_nop_convert if again is false.
> Update call to gimple_bitwise_inverted_equal_p for
> new argument.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/110874
> * gcc.c-torture/compile/pr110874-a.c: New test.
> ---
> gcc/gimple-match-head.cc | 14 +++++++++-----
> .../gcc.c-torture/compile/pr110874-a.c | 17 +++++++++++++++++
> 2 files changed, 26 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
>
> diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> index b1e96304d7c..e91aaab86dd 100644
> --- a/gcc/gimple-match-head.cc
> +++ b/gcc/gimple-match-head.cc
> @@ -273,7 +273,7 @@ gimple_bitwise_equal_p (tree expr1, tree expr2, tree (*valueize) (tree))
> /* Helper function for bitwise_equal_p macro. */
>
> static inline bool
> -gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree))
> +gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree), bool again = true)
> {
> if (expr1 == expr2)
> return false;
> @@ -285,12 +285,16 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree)
> return false;
>
> tree other;
> - if (gimple_nop_convert (expr1, &other, valueize)
> - && gimple_bitwise_inverted_equal_p (other, expr2, valueize))
> + if (again
> + && gimple_nop_convert (expr1, &other, valueize)
> + && other != expr1
> + && gimple_bitwise_inverted_equal_p (other, expr2, valueize, false))
> return true;
>
> - if (gimple_nop_convert (expr2, &other, valueize)
> - && gimple_bitwise_inverted_equal_p (expr1, other, valueize))
> + if (again
> + && gimple_nop_convert (expr2, &other, valueize)
> + && other != expr2
> + && gimple_bitwise_inverted_equal_p (expr1, other, valueize, false))
> return true;
Hmm, I don't think this tests all three relevant combinations? I think the way
gimple_bitwise_equal_p handles this is better (not recursing). I'd split out
the "tail" matching the BIT_NOT to another helper, I suppose that could
even be a (match ...) pattern here.
> if (TREE_CODE (expr1) != SSA_NAME
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c b/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
> new file mode 100644
> index 00000000000..b314410a892
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
> @@ -0,0 +1,17 @@
> +struct S1 {
> + unsigned f0;
> +};
> +static int g_161;
> +void func_109(unsigned g_227, unsigned t) {
> + struct S1 l_178;
> + int l_160 = 0x1FAE99D5L;
> + int *l_230[] = {&l_160};
> + if (l_160) {
> + for (l_178.f0 = -7; l_178.f0;) {
> + ++g_227;
> + break;
> + }
> + (g_161) = g_227;
> + }
> + (g_161) &= t;
> +}
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PR 110874: infinite loop in gimple_bitwise_inverted_equal_p with fre
2023-08-03 7:21 ` Richard Biener
@ 2023-08-03 9:23 ` Andrew Pinski
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Pinski @ 2023-08-03 9:23 UTC (permalink / raw)
To: Richard Biener; +Cc: Andrew Pinski, gcc-patches
On Thu, Aug 3, 2023 at 12:23 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Thu, Aug 3, 2023 at 4:34 AM Andrew Pinski via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > So I didn't expect valueization to cause calling gimple_nop_convert
> > to iterate between 2 different SSA names causing an infinite loop
> > in gimple_bitwise_inverted_equal_p.
> > So we should cause a bound on gimple_bitwise_inverted_equal_p calling
> > gimple_nop_convert and only look through one rather than always.
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > gcc/ChangeLog:
> >
> > PR tree-optimization/110874
> > * gimple-match-head.cc (gimple_bitwise_inverted_equal_p):
> > Add new argument, again with default value of true.
> > Don't try gimple_nop_convert if again is false.
> > Update call to gimple_bitwise_inverted_equal_p for
> > new argument.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR tree-optimization/110874
> > * gcc.c-torture/compile/pr110874-a.c: New test.
> > ---
> > gcc/gimple-match-head.cc | 14 +++++++++-----
> > .../gcc.c-torture/compile/pr110874-a.c | 17 +++++++++++++++++
> > 2 files changed, 26 insertions(+), 5 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
> >
> > diff --git a/gcc/gimple-match-head.cc b/gcc/gimple-match-head.cc
> > index b1e96304d7c..e91aaab86dd 100644
> > --- a/gcc/gimple-match-head.cc
> > +++ b/gcc/gimple-match-head.cc
> > @@ -273,7 +273,7 @@ gimple_bitwise_equal_p (tree expr1, tree expr2, tree (*valueize) (tree))
> > /* Helper function for bitwise_equal_p macro. */
> >
> > static inline bool
> > -gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree))
> > +gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree), bool again = true)
> > {
> > if (expr1 == expr2)
> > return false;
> > @@ -285,12 +285,16 @@ gimple_bitwise_inverted_equal_p (tree expr1, tree expr2, tree (*valueize) (tree)
> > return false;
> >
> > tree other;
> > - if (gimple_nop_convert (expr1, &other, valueize)
> > - && gimple_bitwise_inverted_equal_p (other, expr2, valueize))
> > + if (again
> > + && gimple_nop_convert (expr1, &other, valueize)
> > + && other != expr1
> > + && gimple_bitwise_inverted_equal_p (other, expr2, valueize, false))
> > return true;
> >
> > - if (gimple_nop_convert (expr2, &other, valueize)
> > - && gimple_bitwise_inverted_equal_p (expr1, other, valueize))
> > + if (again
> > + && gimple_nop_convert (expr2, &other, valueize)
> > + && other != expr2
> > + && gimple_bitwise_inverted_equal_p (expr1, other, valueize, false))
> > return true;
>
> Hmm, I don't think this tests all three relevant combinations? I think the way
> gimple_bitwise_equal_p handles this is better (not recursing). I'd split out
> the "tail" matching the BIT_NOT to another helper, I suppose that could
> even be a (match ...) pattern here.
That sounds like a better idea. I am testing the patch right now that
uses a (match ) pattern for
the BIT_NOT and CMP cases. That will remove the recursiveness too.
Thanks,
Andrew
>
> > if (TREE_CODE (expr1) != SSA_NAME
> > diff --git a/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c b/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
> > new file mode 100644
> > index 00000000000..b314410a892
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.c-torture/compile/pr110874-a.c
> > @@ -0,0 +1,17 @@
> > +struct S1 {
> > + unsigned f0;
> > +};
> > +static int g_161;
> > +void func_109(unsigned g_227, unsigned t) {
> > + struct S1 l_178;
> > + int l_160 = 0x1FAE99D5L;
> > + int *l_230[] = {&l_160};
> > + if (l_160) {
> > + for (l_178.f0 = -7; l_178.f0;) {
> > + ++g_227;
> > + break;
> > + }
> > + (g_161) = g_227;
> > + }
> > + (g_161) &= t;
> > +}
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-03 9:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03 2:34 [PATCH] Fix PR 110874: infinite loop in gimple_bitwise_inverted_equal_p with fre Andrew Pinski
2023-08-03 7:21 ` Richard Biener
2023-08-03 9:23 ` Andrew Pinski
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).