Re: [PATCH] Fix PR 110954: wrong code with cmp | !cmp

[Andrew Pinski via Gcc-patches]

On Thu, Aug 10, 2023 at 12:32 AM Richard Biener via Gcc-patches
 wrote:
>
> On Thu, Aug 10, 2023 at 2:21 AM Andrew Pinski via Gcc-patches
>  wrote:
> >
> > This was an oversight on my part not realizing that
> > comparisons in generic can have a non-boolean type.
> > This means if we have `(f < 0) | !(f < 0)` we would
> > optimize this to -1 rather than just 1.
> > This patch just adds the check for the type of the comparisons
> > to be boolean type to keep the optimization in that case.
> >
> > OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
> >
> > PR 110954
> >
> > gcc/ChangeLog:
> >
> > * generic-match-head.cc (bitwise_inverted_equal_p): Check
> > the type of the comparison to be boolean too.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.c-torture/execute/pr110954-1.c: New test.
> > ---
> >  gcc/generic-match-head.cc|  3 ++-
> >  gcc/testsuite/gcc.c-torture/execute/pr110954-1.c | 10 ++
> >  2 files changed, 12 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
> >
> > diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
> > index ddaf22f2179..ac2119bfdd0 100644
> > --- a/gcc/generic-match-head.cc
> > +++ b/gcc/generic-match-head.cc
> > @@ -146,7 +146,8 @@ bitwise_inverted_equal_p (tree expr1, tree expr2)
> >&& bitwise_equal_p (expr1, TREE_OPERAND (expr2, 0)))
> >  return true;
> >if (COMPARISON_CLASS_P (expr1)
> > -  && COMPARISON_CLASS_P (expr2))
> > +  && COMPARISON_CLASS_P (expr2)
> > +  && TREE_CODE (TREE_TYPE (expr1)) == BOOLEAN_TYPE)
>
> in other places we restrict this to single-bit integral types instead which
> covers a few more cases and also would handle BOOLEAN_TYPE
> with either padding or non-padding extra bits correctly (IIRC fortran
> has only padding bits but Ada has BOOLEAN_TYPEs with possibly
> > 1 bit precision and arbitrary signedness - maybe even with custom
> true/false values).

Yes and I have a better patch which still allows us to optimize the
case of `cmp & ~cmp` and `cmp | ~cmp`. I will post it after it
finishes bootstrapping.

Thanks,
Andrew

>
> Richard.
>
> >  {
> >tree op10 = TREE_OPERAND (expr1, 0);
> >tree op20 = TREE_OPERAND (expr2, 0);
> > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c 
> > b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
> > new file mode 100644
> > index 000..8aad758e10f
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
> > @@ -0,0 +1,10 @@
> > +
> > +#define comparison (f < 0)
> > +int main() {
> > +  int f = 0;
> > +  int d = comparison | !comparison;
> > +  if (d != 1)
> > +__builtin_abort();
> > +  return 0;
> > +}
> > +
> > --
> > 2.31.1
> >

Re: [PATCH] Fix PR 110954: wrong code with cmp | !cmp

[Richard Biener via Gcc-patches]

On Thu, Aug 10, 2023 at 2:21 AM Andrew Pinski via Gcc-patches
 wrote:
>
> This was an oversight on my part not realizing that
> comparisons in generic can have a non-boolean type.
> This means if we have `(f < 0) | !(f < 0)` we would
> optimize this to -1 rather than just 1.
> This patch just adds the check for the type of the comparisons
> to be boolean type to keep the optimization in that case.
>
> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> PR 110954
>
> gcc/ChangeLog:
>
> * generic-match-head.cc (bitwise_inverted_equal_p): Check
> the type of the comparison to be boolean too.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.c-torture/execute/pr110954-1.c: New test.
> ---
>  gcc/generic-match-head.cc|  3 ++-
>  gcc/testsuite/gcc.c-torture/execute/pr110954-1.c | 10 ++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
>
> diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
> index ddaf22f2179..ac2119bfdd0 100644
> --- a/gcc/generic-match-head.cc
> +++ b/gcc/generic-match-head.cc
> @@ -146,7 +146,8 @@ bitwise_inverted_equal_p (tree expr1, tree expr2)
>&& bitwise_equal_p (expr1, TREE_OPERAND (expr2, 0)))
>  return true;
>if (COMPARISON_CLASS_P (expr1)
> -  && COMPARISON_CLASS_P (expr2))
> +  && COMPARISON_CLASS_P (expr2)
> +  && TREE_CODE (TREE_TYPE (expr1)) == BOOLEAN_TYPE)

in other places we restrict this to single-bit integral types instead which
covers a few more cases and also would handle BOOLEAN_TYPE
with either padding or non-padding extra bits correctly (IIRC fortran
has only padding bits but Ada has BOOLEAN_TYPEs with possibly
> 1 bit precision and arbitrary signedness - maybe even with custom
true/false values).

Richard.

>  {
>tree op10 = TREE_OPERAND (expr1, 0);
>tree op20 = TREE_OPERAND (expr2, 0);
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c 
> b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
> new file mode 100644
> index 000..8aad758e10f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
> @@ -0,0 +1,10 @@
> +
> +#define comparison (f < 0)
> +int main() {
> +  int f = 0;
> +  int d = comparison | !comparison;
> +  if (d != 1)
> +__builtin_abort();
> +  return 0;
> +}
> +
> --
> 2.31.1
>

[PATCH] Fix PR 110954: wrong code with cmp | !cmp

[Andrew Pinski via Gcc-patches]

This was an oversight on my part not realizing that
comparisons in generic can have a non-boolean type.
This means if we have `(f < 0) | !(f < 0)` we would
optimize this to -1 rather than just 1.
This patch just adds the check for the type of the comparisons
to be boolean type to keep the optimization in that case.

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

PR 110954

gcc/ChangeLog:

* generic-match-head.cc (bitwise_inverted_equal_p): Check
the type of the comparison to be boolean too.

gcc/testsuite/ChangeLog:

* gcc.c-torture/execute/pr110954-1.c: New test.
---
 gcc/generic-match-head.cc|  3 ++-
 gcc/testsuite/gcc.c-torture/execute/pr110954-1.c | 10 ++
 2 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr110954-1.c

diff --git a/gcc/generic-match-head.cc b/gcc/generic-match-head.cc
index ddaf22f2179..ac2119bfdd0 100644
--- a/gcc/generic-match-head.cc
+++ b/gcc/generic-match-head.cc
@@ -146,7 +146,8 @@ bitwise_inverted_equal_p (tree expr1, tree expr2)
   && bitwise_equal_p (expr1, TREE_OPERAND (expr2, 0)))
 return true;
   if (COMPARISON_CLASS_P (expr1)
-  && COMPARISON_CLASS_P (expr2))
+  && COMPARISON_CLASS_P (expr2)
+  && TREE_CODE (TREE_TYPE (expr1)) == BOOLEAN_TYPE)
 {
   tree op10 = TREE_OPERAND (expr1, 0);
   tree op20 = TREE_OPERAND (expr2, 0);
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c 
b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
new file mode 100644
index 000..8aad758e10f
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr110954-1.c
@@ -0,0 +1,10 @@
+
+#define comparison (f < 0)
+int main() {
+  int f = 0;
+  int d = comparison | !comparison;
+  if (d != 1)
+__builtin_abort();
+  return 0;
+}
+
-- 
2.31.1