[Bug tree-optimization/110487] New: invalid wide Boolean value

[Bug tree-optimization/110487] New: invalid wide Boolean value

[kristerw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

            Bug ID: 110487
           Summary: invalid wide Boolean value
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kristerw at gcc dot gnu.org
  Target Milestone: ---

The vrp2 pass generates IR where a <signed-boolean:32> may get the value 1 (in
addition to the valid 0 and -1).

This can be seen in gcc.c-torture/compile/pr53410-1.c

  int *a, b, c, d;

  void
  foo (void)
  {
    for (; d <= 0; d++)
      b &= ((a || d) ^ c) == 1;
  }

when compiled as "gcc -O3". The vectorizer has created (correct) code

  _Bool _16;
  <signed-boolean:32> _66;
  ...
  _16 = a.1_1 != 0B;
  _66 = _16 ? -1 : 0;

which then is transformed by vrp2 to

  _Bool _16;
  <signed-boolean:32> _38;
  <signed-boolean:32> _66;
  ...
  _16 = a.1_1 != 0B;
  _38 = (<signed-boolean:32>) _16;
  _66 = -_38;

_16 can be both true/false depending on the values of some global variables, so
_38 has the value 0 or -1, and _66 has the value 0 or 1.

[Bug tree-optimization/110487] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2023-06-29

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Applying pattern match.pd:4705, gimple-match-10.cc:15878
gimple_simplified to _38 = (<signed-boolean:32>) _16;
_66 = -_38;
Global Exported: _66 = [irange] <signed-boolean:32> [-1, 0]
Folded into: _66 = -_38;


    /* a ? -1 : 0 -> -a.  No need to check the TYPE_PRECISION not being 1
       here as the powerof2cst case above will handle that case correctly.  */
    (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
     (negate (convert (convert:boolean_type_node @0))))))


I guess is mine ....

[Bug tree-optimization/110487] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Part of the issue is what does TYPE_PRECISION on BOOLEAN_TYPE really mean. 
there are many more of these issues all over GCC about boolean types assuming
being TYPE_PRECISION == 1 even and such.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|invalid wide Boolean value  |[12/13/14 Regression]
                   |                            |invalid wide Boolean value
             Target|                            |aarch64-linux-gnu
   Target Milestone|---                         |12.4

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
/* Optimize
   # x_5 in range [cst1, cst2] where cst2 = cst1 + 1
   x_5 ? cstN ? cst4 : cst3
   # op is == or != and N is 1 or 2
   to r_6 = x_5 + (min (cst3, cst4) - cst1) or
   r_6 = (min (cst3, cst4) + cst1) - x_5 depending on op, N and which
   of cst3 and cst4 is smaller.
   This was originally done by two_value_replacement in phiopt (PR 88676).  */


is going to have the same issue I think.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
BOOLEAN_TYPE means it has two values (else undefined), when the values are 0
and 1 the precision can always be 1 and just TYPE_SIZE can be larger when
required
(fortran bools).  With values -1 and 0 and vector ISAs who tend to look at
the MSB we need sign-extension to the size of the type which means we need
TYPE_PRECISION != 1.

We have multiple places that check for two-valuedness with TYPE_PRECISION == 1
|| BOOLEAN_TYPE and that's OK.  Some also check for TYPE_UNSIGNED if they
require [0, 1].

The spirit of transforming this COND_EXPR to a negation is "OK", but I think
you need to go via an integer type here:

  _16 = a.1_1 != 0B;
  _38 = (int32_t) _16;
  _39 = -_38;
  _66 = (<signed_boolean:32) _39;

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think there is zero_one_value_p is also accepting signed-boolean:32
incorrectly too ...

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55440
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55440&action=edit
Fixes the first part

Here is the patch which fixes the first part, I will fix zero_one_p too.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #55440|0                           |1
        is obsolete|                            |

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55441
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55441&action=edit
Fixes the first part

Attach the correct patch this time around.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #5)
> I think there is zero_one_value_p is also accepting signed-boolean:32
> incorrectly too ...

No, zero_one_value_p is fine as truth_valued_p only accepts single bit
integeral types.

Though the pattern mentioned in comment #3 is still an issue ...

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #8)
> (In reply to Andrew Pinski from comment #5)
> > I think there is zero_one_value_p is also accepting signed-boolean:32
> > incorrectly too ...
> 
> No, zero_one_value_p is fine as truth_valued_p only accepts single bit
> integeral types.
> 
> Though the pattern mentioned in comment #3 is still an issue ...

The pattern in comment # 3 is especially an issue with strict enum even.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55442
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55442&action=edit
Patch for the two_value pattern

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #55441|0                           |1
        is obsolete|                            |

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55443
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55443&action=edit
Patch for the original issue updated

This updated the patch for the original issue slightly by using type rather
than TREE_TYPE (@12) as they will be the same but type is shorter and will be
slightly faster (at compile time).

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:2e5c1b123d5bc4b7eca91f1eb2dab88d0bcdfcfa

commit r14-2298-g2e5c1b123d5bc4b7eca91f1eb2dab88d0bcdfcfa
Author: Andrew Pinski <apinski@marvell.com>
Date:   Fri Jun 30 17:50:08 2023 -0700

    Fix PR 110487: invalid signed boolean value

    This fixes the first part of this bug where `a ? -1 : 0`
    would cause a value of 1 into the signed boolean value.
    It fixes the problem by casting to an integer type of
    the same size/signedness before doing the negative and
    then casting to the type of expression.

    OK? Bootstrapped and tested on x86_64.

    gcc/ChangeLog:

            * match.pd (a?-1:0): Cast type an integer type
            rather the type before the negative.
            (a?0:-1): Likewise.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:71b68cc559b5d30d07e6b507df7cc6d008f56aad

commit r14-2299-g71b68cc559b5d30d07e6b507df7cc6d008f56aad
Author: Andrew Pinski <apinski@marvell.com>
Date:   Fri Jun 30 19:22:48 2023 -0700

    PR 110487: `(a !=/== CST1 ? CST2 : CST3)` pattern for type safety

    The problem here is we might produce some values out of the type's
    min/max (and/or valid values, e.g. signed booleans). The fix is to
    use an integer type which has the same precision and signedness
    as the original type.

    Note two_value_replacement in phiopt had the same issue in previous
    versions; though I don't know if a problem will show up there.

    OK? Bootstrapped and tested on x86_64-linux-gnu.

    gcc/ChangeLog:

            PR tree-optimization/110487
            * match.pd (a !=/== CST1 ? CST2 : CST3): Always
            build a nonstandard integer and use that.

[Bug tree-optimization/110487] [12/13/14 Regression] invalid wide Boolean value

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110487

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|12.4                        |14.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Even though this could cause problems in GCC 12.x and GCC 13.x, we have not
seen any reports of causing issues and I don't know of a testcase where the
wrong code gets exposed either.

So closing as fixed.