[Bug tree-optimization/105189] New: [9/10/11/12 Regression] Wrong code with -O1

[Bug tree-optimization/105189] New: [9/10/11/12 Regression] Wrong code with -O1

[vsevolod.livinskiy at gmail dot com]

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

            Bug ID: 105189
           Summary: [9/10/11/12 Regression] Wrong code with -O1
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: vsevolod.livinskiy at gmail dot com
  Target Milestone: ---

Link to the Compiler Explorer: https://gcc.godbolt.org/z/zGn1a49KT

Reproducer:
#include <stdio.h>

int var_16 = -1;

int a(int b) { return b; }

int main() {
    int c = (unsigned int)a(var_16) >= 0 && 4;
    printf("%d\n", c);
    if (c != 1)
        __builtin_abort();
}

Error:
>$ g++ -O1 driver.cpp && ./a.out 
0
>$ g++ -O0 driver.cpp && ./a.out 
1

gcc version 12.0.1 20220406 (git://gcc.gnu.org/git/gcc.git:master
9fd377a747375a82912bd81c67b275301489785c)

[Bug tree-optimization/105189] [9/10/11/12 Regression] Wrong code with -O1

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2022-04-06
     Ever confirmed|0                           |1
   Target Milestone|---                         |9.5
             Status|UNCONFIRMED                 |ASSIGNED
           Priority|P3                          |P2
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with my r9-1971-g315aa691f486bfe71bae0a5fc8828db26daebb56

int a = -1;

int
foo (int b)
{
  return b;
}

int
main ()
{
  int c = foo (a) >= 0U && 4;
  if (c != 1)
    __builtin_abort ();
}

[Bug tree-optimization/105189] [9/10/11/12 Regression] Wrong code with -O1

[davidfromonline at gmail dot com]

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

David Stone <davidfromonline at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |davidfromonline at gmail dot com

--- Comment #2 from David Stone <davidfromonline at gmail dot com> ---
Simplified reproducer: https://godbolt.org/z/vbvKzPchP

```
int a() { return -1; }

int f() {
        return (unsigned)a() >= 0 && 1;
}
```

Interestingly, the bug does not occur for C code:
https://godbolt.org/z/7efs1aEEz

[Bug tree-optimization/105189] [9/10/11/12 Regression] Wrong code with -O1

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the bug is in make_range_step.
When we see
(unsigned) foo () >= 0U, first make_range_step determines +[0U, -] range
for (unsigned) foo (), that is correct (though equivalent to +[-, -] aka always
true).
But then we handle the NOP_EXPR in another make_range_step call, exp_type is
unsigned int, arg0_type is int.  There is code that handles signed exp_type and
unsigned arg0_type and we punt when arg0_type is not integral, or has higher
precision than exp_type (i.e. narrowing conversion) or if either of the bounds
(if specified) doesn't fit into arg0_type.
None of that is the case here, so we incorrectly change the range to
foo () +[0, -] (but e.g. (unsigned) foo () +[-, -] would be handled correctly
as foo () +[-, -]).
At least for the TYPE_PRECISION equal case, the unsigned exp_type signed
arg0_type case is IMHO valid as is only if high is non-NULL or both low and
high are NULL (if high is non-NULL and higher than signed maximum, we already
punt), because if high is NULL, the range includes the largest unsigned value,
which is -1 in signed and when low is non-NULL, it is necessarily [0,
largest_signed].

[Bug tree-optimization/105189] [9/10/11/12 Regression] Wrong code with -O1

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52766
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52766&action=edit
gcc12-pr105189.patch

Untested fix.

[Bug tree-optimization/105189] [9/10/11/12 Regression] Wrong code with -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:5e6597064b0c7eb93b8f720afc4aa970eefb0628

commit r12-8056-g5e6597064b0c7eb93b8f720afc4aa970eefb0628
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 8 09:14:44 2022 +0200

    fold-const: Fix up make_range_step [PR105189]

    The following testcase is miscompiled, because fold_truth_andor
    incorrectly folds
    (unsigned) foo () >= 0U && 1
    into
    foo () >= 0
    For the unsigned comparison (which is useless in this case,
    as >= 0U is always true, but hasn't been folded yet), previous
    make_range_step derives exp (unsigned) foo () and +[0U, -]
    range for it.  Next we process the NOP_EXPR.  We have special code
    for unsigned to signed casts, already earlier punt if low or high
    aren't representable in arg0_type or if it is a narrowing conversion.
    For the signed to unsigned casts, I think if high is specified we
    are still fine, as we punt for non-representable values in arg0_type,
    n_high is then still representable and so was smaller or equal to
    signed maximum and either low is not present (equivalent to 0U), or
    low must be smaller or equal to high and so for unsigned exp
    +[low, high] the signed exp +[n_low, n_high] will be correct.
    Similarly, if both low and high aren't specified (always true or
    always false), it is ok too.
    But if we have for unsigned exp +[low, -] or -[low, -], using
    +[n_low, -] or -[n_high, -] is incorrect.  Because low is smaller
    or equal to signed maximum and high is unspecified (i.e. unsigned
    maximum), when signed that range is a union of +[n_low, -] and
    +[-, -1] which is equivalent to -[0, n_low-1], unless low
    is 0, in that case we can treat it as [-, -].

    2022-04-08  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/105189
            * fold-const.cc (make_range_step): Fix up handling of
            (unsigned) x +[low, -] ranges for signed x if low fits into
            typeof (x).

            * g++.dg/torture/pr105189.C: New test.

[Bug tree-optimization/105189] [9/10/11 Regression] Wrong code with -O1

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10/11/12 Regression]     |[9/10/11 Regression] Wrong
                   |Wrong code with -O1         |code with -O1

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug tree-optimization/105189] [9/10/11 Regression] Wrong code with -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:b28307530ecb4d6aea2328fb3e670068e3275868

commit r11-9846-gb28307530ecb4d6aea2328fb3e670068e3275868
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 8 09:14:44 2022 +0200

    fold-const: Fix up make_range_step [PR105189]

    The following testcase is miscompiled, because fold_truth_andor
    incorrectly folds
    (unsigned) foo () >= 0U && 1
    into
    foo () >= 0
    For the unsigned comparison (which is useless in this case,
    as >= 0U is always true, but hasn't been folded yet), previous
    make_range_step derives exp (unsigned) foo () and +[0U, -]
    range for it.  Next we process the NOP_EXPR.  We have special code
    for unsigned to signed casts, already earlier punt if low or high
    aren't representable in arg0_type or if it is a narrowing conversion.
    For the signed to unsigned casts, I think if high is specified we
    are still fine, as we punt for non-representable values in arg0_type,
    n_high is then still representable and so was smaller or equal to
    signed maximum and either low is not present (equivalent to 0U), or
    low must be smaller or equal to high and so for unsigned exp
    +[low, high] the signed exp +[n_low, n_high] will be correct.
    Similarly, if both low and high aren't specified (always true or
    always false), it is ok too.
    But if we have for unsigned exp +[low, -] or -[low, -], using
    +[n_low, -] or -[n_high, -] is incorrect.  Because low is smaller
    or equal to signed maximum and high is unspecified (i.e. unsigned
    maximum), when signed that range is a union of +[n_low, -] and
    +[-, -1] which is equivalent to -[0, n_low-1], unless low
    is 0, in that case we can treat it as [-, -].

    2022-04-08  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/105189
            * fold-const.c (make_range_step): Fix up handling of
            (unsigned) x +[low, -] ranges for signed x if low fits into
            typeof (x).

            * g++.dg/torture/pr105189.C: New test.

    (cherry picked from commit 5e6597064b0c7eb93b8f720afc4aa970eefb0628)

[Bug tree-optimization/105189] [9/10 Regression] Wrong code with -O1

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10/11 Regression] Wrong  |[9/10 Regression] Wrong
                   |code with -O1               |code with -O1

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed also for 11.3.

[Bug tree-optimization/105189] [9/10 Regression] Wrong code with -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:a954df6a075d5d1e91f98e3c686e5da62c829cf9

commit r10-10705-ga954df6a075d5d1e91f98e3c686e5da62c829cf9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 8 09:14:44 2022 +0200

    fold-const: Fix up make_range_step [PR105189]

    The following testcase is miscompiled, because fold_truth_andor
    incorrectly folds
    (unsigned) foo () >= 0U && 1
    into
    foo () >= 0
    For the unsigned comparison (which is useless in this case,
    as >= 0U is always true, but hasn't been folded yet), previous
    make_range_step derives exp (unsigned) foo () and +[0U, -]
    range for it.  Next we process the NOP_EXPR.  We have special code
    for unsigned to signed casts, already earlier punt if low or high
    aren't representable in arg0_type or if it is a narrowing conversion.
    For the signed to unsigned casts, I think if high is specified we
    are still fine, as we punt for non-representable values in arg0_type,
    n_high is then still representable and so was smaller or equal to
    signed maximum and either low is not present (equivalent to 0U), or
    low must be smaller or equal to high and so for unsigned exp
    +[low, high] the signed exp +[n_low, n_high] will be correct.
    Similarly, if both low and high aren't specified (always true or
    always false), it is ok too.
    But if we have for unsigned exp +[low, -] or -[low, -], using
    +[n_low, -] or -[n_high, -] is incorrect.  Because low is smaller
    or equal to signed maximum and high is unspecified (i.e. unsigned
    maximum), when signed that range is a union of +[n_low, -] and
    +[-, -1] which is equivalent to -[0, n_low-1], unless low
    is 0, in that case we can treat it as [-, -].

    2022-04-08  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/105189
            * fold-const.c (make_range_step): Fix up handling of
            (unsigned) x +[low, -] ranges for signed x if low fits into
            typeof (x).

            * g++.dg/torture/pr105189.C: New test.

    (cherry picked from commit 5e6597064b0c7eb93b8f720afc4aa970eefb0628)

[Bug tree-optimization/105189] [9/10 Regression] Wrong code with -O1

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:cddca3b79f813f2140f2f485de33ef90d7a03740

commit r9-10146-gcddca3b79f813f2140f2f485de33ef90d7a03740
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 8 09:14:44 2022 +0200

    fold-const: Fix up make_range_step [PR105189]

    The following testcase is miscompiled, because fold_truth_andor
    incorrectly folds
    (unsigned) foo () >= 0U && 1
    into
    foo () >= 0
    For the unsigned comparison (which is useless in this case,
    as >= 0U is always true, but hasn't been folded yet), previous
    make_range_step derives exp (unsigned) foo () and +[0U, -]
    range for it.  Next we process the NOP_EXPR.  We have special code
    for unsigned to signed casts, already earlier punt if low or high
    aren't representable in arg0_type or if it is a narrowing conversion.
    For the signed to unsigned casts, I think if high is specified we
    are still fine, as we punt for non-representable values in arg0_type,
    n_high is then still representable and so was smaller or equal to
    signed maximum and either low is not present (equivalent to 0U), or
    low must be smaller or equal to high and so for unsigned exp
    +[low, high] the signed exp +[n_low, n_high] will be correct.
    Similarly, if both low and high aren't specified (always true or
    always false), it is ok too.
    But if we have for unsigned exp +[low, -] or -[low, -], using
    +[n_low, -] or -[n_high, -] is incorrect.  Because low is smaller
    or equal to signed maximum and high is unspecified (i.e. unsigned
    maximum), when signed that range is a union of +[n_low, -] and
    +[-, -1] which is equivalent to -[0, n_low-1], unless low
    is 0, in that case we can treat it as [-, -].

    2022-04-08  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/105189
            * fold-const.c (make_range_step): Fix up handling of
            (unsigned) x +[low, -] ranges for signed x if low fits into
            typeof (x).

            * g++.dg/torture/pr105189.C: New test.

    (cherry picked from commit 5e6597064b0c7eb93b8f720afc4aa970eefb0628)

[Bug tree-optimization/105189] [9/10 Regression] Wrong code with -O1

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.