[Bug c/58564] New: possible wrong code bug at -O0

[Bug c/58564] New: possible wrong code bug at -O0

[regehr at cs dot utah.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

            Bug ID: 58564
           Summary: possible wrong code bug at -O0
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: regehr at cs dot utah.edu

regehr@john-home ~ $ clang -O0 -w small.c ; ./a.out
0
regehr@john-home ~ $ gcc-4.4 -O0 -w small.c ; ./a.out
0
regehr@john-home ~ $ gcc-4.6 -O0 -w small.c ; ./a.out
0
regehr@john-home ~ $ gcc-4.7 -O0 -w small.c ; ./a.out
1
regehr@john-home ~ $ gcc -O0 -w small.c ; ./a.out
1
regehr@john-home ~ $ cat small.c 
int printf(const char *, ...);
int a, b;
short *c;
short **d = &c;
int main() {
  b = (0, 0 > ((&c == d) & (1 && a ^ 1))) | 0U;
  printf("%d\n", b);
  return 0;
}
regehr@john-home ~ $ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/home/regehr/z/compiler-install/gcc-r203005-install/libexec/gcc/x86_64-unknown-linux-gnu/4.9.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: /home/regehr/z/compiler-source/gcc/configure
--prefix=/home/regehr/z/compiler-install/gcc-r203005-install
--enable-languages=c,c++ --disable-multilib
Thread model: posix
gcc version 4.9.0 20130928 (experimental) (GCC)

[Bug c/58564] possible wrong code bug at -O0

[mikpelinux at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

Mikael Pettersson <mikpelinux at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ktietz at gcc dot gnu.org,
                   |                            |mikpelinux at gmail dot com

--- Comment #1 from Mikael Pettersson <mikpelinux at gmail dot com> ---
4.6 works, 4.7 to trunk don't, started with Kai's r176563.

[Bug c/58564] possible wrong code bug at -O0

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #2 from Kai Tietz <ktietz at gcc dot gnu.org> ---
This sample is invalid.  And before 4.7 there seems to be wrong result.

You missed the following here in your expression
0 > ((&c == d) & (1 && a ^ 1))

The term (1 && a ^ 1)  is not the same as (1 & (a ^ 1))

instead it means in fact (1 != 0 && (a ^ 1) != 0.  By this term reduces correct
to:
 a ^ 1 != 0 and this is indentical to a != 1.

by this we see 0 > (&c == d) & (a != 1).  And this is exactly that what AST
generates (see here -fdump-tree-original).

So error is invalid.  But indeed this testcase demonstrate, that we seem to
have still pointer-arthimetic optimization issues, due the term (&c == d)  was
resolved ...

[Bug c/58564] possible wrong code bug at -O0

[regehr at cs dot utah.edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #3 from John Regehr <regehr at cs dot utah.edu> ---
Kai, this is a real bug, please reopen it.

Here is what I get out of -fdump-tree-original:

  b = (int) (d == &c && a != 1);

This is wrong.

One way to illustrate the problem is to remove the useless comma operator from
the test case, which now looks like this (I've also added parens around the ^):

int printf(const char *, ...);
int a, b;
short *c;
short **d = &c;
int main() {
  b = (0 > ((&c == d) & (1 && (a ^ 1)))) | 0U;
  printf("%d\n", b);
  return 0;
}

Now -fdump-tree-original does a better job:

 b = (int) (d == &c && a != 1) < 0;

And now all compilers agree on the output:

[regehr@imp r102]$ gcc-4.4 small2.c ; ./a.out 
0
[regehr@imp r102]$ gcc-4.5 small2.c ; ./a.out 
0
[regehr@imp r102]$ gcc-4.6 small2.c ; ./a.out 
0
[regehr@imp r102]$ gcc small2.c ; ./a.out 
0
[regehr@imp r102]$ clang small2.c ; ./a.out 
0

[Bug c/58564] possible wrong code bug at -O0

[mikpelinux at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #4 from Mikael Pettersson <mikpelinux at gmail dot com> ---
(In reply to John Regehr from comment #3)
> Kai, this is a real bug, please reopen it.
> 
> Here is what I get out of -fdump-tree-original:
> 
>   b = (int) (d == &c && a != 1);
> 
> This is wrong.
> 
> One way to illustrate the problem is to remove the useless comma operator
> from the test case, which now looks like this (I've also added parens around
> the ^):

I can confirm this observation.  Adding parentheses around the "a ^ 1" makes no
difference, but removing the comma operator and its left operand "0, " makes
4.7 to trunk compute the same result as 4.6 and older did.

[Bug c/58564] possible wrong code bug at -O0

[ktietz at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

Kai Tietz <ktietz at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2013-09-30
         Resolution|INVALID                     |---
     Ever confirmed|0                           |1

--- Comment #5 from Kai Tietz <ktietz at gcc dot gnu.org> ---
Sorry, missed that outer condition is 0 > ... boolean-typed-expr.

Can confirm this issue.  Btw this condition has to be always false for unsigned
1-bit precision typed integrals.

[Bug middle-end/58564] [4.7/4.8/4.9 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
          Component|c                           |middle-end
   Target Milestone|---                         |4.7.4
            Summary|possible wrong code bug at  |[4.7/4.8/4.9 Regression]
                   |-O0                         |possible wrong code bug at
                   |                            |-O0

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the bug is in the
      /* A < 0 ? <sign bit of A> : 0 is simply (A & <sign bit of A>).  */
      if (TREE_CODE (arg0) == LT_EXPR
          && integer_zerop (TREE_OPERAND (arg0, 1))
          && integer_zerop (op2)
          && (tem = sign_bit_p (TREE_OPERAND (arg0, 0), arg1)))
...
transformation in fold_ternary_loc or sign_bit_p or combination thereof.
We get here with arg0 ((int) (d == &c && a != 1)) < 0, arg1 1U and arg2 0U,
sign_bit_p (surprisingly) looks through zero extensions (of bool to int)
and returns (d == &c && a != 1) and we incorrectly fold it as
(d == &c && a != 1) & 1.

[Bug middle-end/58564] [4.7/4.8/4.9 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30932
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30932&action=edit
gcc49-pr58564.patch

Untested fix.

Note, this patch doesn't attempt to fold the zero extension of something < 0
into 0 resp. side-effects of something, 0, because I think we are generally
trying to move away from adding further new stuff to fold-const.c, and rather
should improve GIMPLE optimizations.

[Bug middle-end/58564] [4.7/4.8/4.9 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 30933
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30933&action=edit
gcc49-pr58564-nonnegative.patch

Actually, teaching fold that it should fold that < 0 into 0 is easy, just needs
making some checks less strict (they were considering only INTEGER_TYPE,
forgetting about BOOLEAN_TYPE or ENUMERAL_TYPE which IMHO can be handled the
same).  This latter patch I'm obviously not going to propose for the older
branches.

[Bug middle-end/58564] [4.7/4.8/4.9 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Sep 30 20:15:20 2013
New Revision: 203042

URL: http://gcc.gnu.org/viewcvs?rev=203042&root=gcc&view=rev
Log:
    PR middle-end/58564
    * fold-const.c (fold_ternary_loc): For A < 0 : <sign bit of A> : 0
    optimization, punt if sign_bit_p looked through any zero extension.

    * gcc.c-torture/execute/pr58564.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr58564.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/58564] [4.7/4.8/4.9 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Sep 30 20:16:14 2013
New Revision: 203043

URL: http://gcc.gnu.org/viewcvs?rev=203043&root=gcc&view=rev
Log:
    PR middle-end/58564
    * fold-const.c (fold_ternary_loc): For A < 0 : <sign bit of A> : 0
    optimization, punt if sign_bit_p looked through any zero extension.

    * gcc.c-torture/execute/pr58564.c: New test.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr58564.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/fold-const.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug middle-end/58564] [4.7/4.8/4.9 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Mon Sep 30 20:17:07 2013
New Revision: 203044

URL: http://gcc.gnu.org/viewcvs?rev=203044&root=gcc&view=rev
Log:
    PR middle-end/58564
    * fold-const.c (tree_unary_nonnegative_warnv_p): Use
    INTEGRAL_TYPE_P (t) instead of TREE_CODE (t) == INTEGER_TYPE.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c

[Bug middle-end/58564] [4.7 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.7/4.8/4.9 Regression]    |[4.7 Regression] possible
                   |possible wrong code bug at  |wrong code bug at -O0
                   |-O0                         |

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Should be fixed for 4.8.2+ so far.

[Bug middle-end/58564] [4.7 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Author: jakub
Date: Wed May  7 16:05:38 2014
New Revision: 210174

URL: http://gcc.gnu.org/viewcvs?rev=210174&root=gcc&view=rev
Log:
    Backported from mainline
    2013-09-30  Jakub Jelinek  <jakub@redhat.com>

    PR middle-end/58564
    * fold-const.c (fold_ternary_loc): For A < 0 : <sign bit of A> : 0
    optimization, punt if sign_bit_p looked through any zero extension.

    * gcc.c-torture/execute/pr58564.c: New test.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.c-torture/execute/pr58564.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/fold-const.c
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog

[Bug middle-end/58564] [4.7 Regression] possible wrong code bug at -O0

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58564

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

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

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