On Thu, Aug 10, 2023 at 12:32 AM Richard Biener via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Thu, Aug 10, 2023 at 2:21 AM Andrew Pinski via Gcc-patches
> <gcc-patches@gcc.gnu.org> 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 00000000000..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
> >

Reply via email to