[Bug tree-optimization/61306] New: wrong code at -Os and above on x86_64-linux-gnu

[Bug tree-optimization/61306] New: wrong code at -Os and above on x86_64-linux-gnu

[su at cs dot ucdavis.edu]

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

            Bug ID: 61306
           Summary: wrong code at -Os and above on x86_64-linux-gnu
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: su at cs dot ucdavis.edu

The current gcc trunk miscompiles the following code on x86_64-linux at -Os and
above in both 32-bit and 64-bit modes.

This is a regression from 4.9.x. 

$ gcc-trunk -v
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/usr/local/gcc-trunk/libexec/gcc/x86_64-unknown-linux-gnu/4.10.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with: ../gcc-trunk/configure --prefix=/usr/local/gcc-trunk
--enable-languages=c,c++ --disable-werror --enable-multilib
Thread model: posix
gcc version 4.10.0 20140524 (experimental) [trunk revision 210888] (GCC) 
$ 
$ gcc-trunk -O1 small.c; a.out
$ gcc-4.9.0 -Os small.c; a.out
$ 
$ gcc-trunk -Os small.c
$ a.out
Aborted (core dumped)
$ 

---------------------------

short a = -1;
int b;
char c;

int
main ()
{
  c = a;
  b = a | c;
  if (b != -1) 
    __builtin_abort (); 
  return 0;
}

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
      Known to work|                            |4.9.0
            Version|unknown                     |4.10.0
           Keywords|                            |wrong-code
   Last reconfirmed|                            |2014-05-26
                 CC|                            |thomas.preudhomme at arm dot com
     Ever confirmed|0                           |1
            Summary|wrong code at -Os and above |[4.10 Regression] wrong
                   |on x86_64-linux-gnu         |code at -Os and above on
                   |                            |x86_64-linux-gnu
   Target Milestone|---                         |4.10.0

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  Looks like fallout from the bswap improvements.

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
16 bit load in host endianness found at: b.3_7 = (int) load_dst_10;

at least the dumping is confusing as well ;)

But the issue seems to be that we are missing that a.0_2 and c.1_3
sign-extend.

  a.0_2 = a;
  c.1_3 = (char) a.0_2;
  c = c.1_3;
  _5 = (int) a.0_2;
  _6 = (int) c.1_3;
  b.3_7 = _6 | _5;

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thomas.preudhomme at arm dot com]

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

--- Comment #3 from Thomas Preud'homme <thomas.preudhomme at arm dot com> ---
Indeed. I also noticed that the original bswap code would happily accept signed
ssa value and signed cast which can lead to disaster. I worked out a patch for
this issue that check the sign of the lhs of the bitwise or expression and use
the (unsigned_)?int.I_type_node accordingly but I now get bootstrap failure.
I'll provide a patch asap.

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thomas.preudhomme at arm dot com]

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

--- Comment #4 from Thomas Preud'homme <thomas.preudhomme at arm dot com> ---
I finally managed to find the root cause for the bootstrap failure with my
current fix. I shall be able to improve my fix and should hopefully be ready
tomorrow.

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thomas.preudhomme at arm dot com]

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

--- Comment #5 from Thomas Preud'homme <thomas.preudhomme at arm dot com> ---
I have a working patch that also pass bootstrap. I'll do a bit more testing and
post it for review.

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thopre01 at gcc dot gnu.org]

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

--- Comment #6 from thopre01 at gcc dot gnu.org ---
Author: thopre01
Date: Wed Jun 11 10:04:33 2014
New Revision: 211444

URL: http://gcc.gnu.org/viewcvs?rev=211444&root=gcc&view=rev
Log:
2014-06-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    gcc/
    PR tree-optimization/61306
    * tree-ssa-math-opts.c (struct symbolic_number): Store type of
    expression instead of its size.
    (do_shift_rotate): Adapt to change in struct symbolic_number. Return
    false to prevent optimization when the result is unpredictable due to
    arithmetic right shift of signed type with highest byte is set.
    (verify_symbolic_number_p): Adapt to change in struct symbolic_number.
    (init_symbolic_number): Likewise.
    (find_bswap_or_nop_1): Likewise. Return NULL to prevent optimization
    when the result is unpredictable due to sign extension.

    gcc/testsuite/
    * gcc.c-torture/execute/pr61306-1.c: New test.
    * gcc.c-torture/execute/pr61306-2.c: Likewise.
    * gcc.c-torture/execute/pr61306-3.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c
    trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-math-opts.c

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thopre01 at gcc dot gnu.org]

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

thopre01 at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|4.9.0                       |4.10.0
      Known to fail|                            |4.8.3, 4.9.0

--- Comment #7 from thopre01 at gcc dot gnu.org ---
Although the number of code impacted is bigger since the recent improvement to
bswap pass to support memory source, this bug is actually quite old. I'm thus
updating the known to work/fail versions accordingly

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thopre01 at gcc dot gnu.org]

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

--- Comment #8 from thopre01 at gcc dot gnu.org ---
Author: thopre01
Date: Mon Jun 30 01:58:45 2014
New Revision: 212133

URL: https://gcc.gnu.org/viewcvs?rev=212133&root=gcc&view=rev
Log:
2014-06-30  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    Backport from mainline
    2014-06-20  Jakub Jelinek  <jakub@redhat.com>
    2014-06-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    gcc/
    PR tree-optimization/61306
    * tree-ssa-math-opts.c (struct symbolic_number): Store type of
    expression instead of its size.
    (do_shift_rotate): Adapt to change in struct symbolic_number. Return
    false to prevent optimization when the result is unpredictable due to
    arithmetic right shift of signed type with highest byte is set.
    (verify_symbolic_number_p): Adapt to change in struct symbolic_number.
    (find_bswap_1): Likewise. Return NULL to prevent optimization when the
    result is unpredictable due to sign extension.
    (find_bswap): Adapt to change in struct symbolic_number.

    gcc/testsuite/
    * gcc.c-torture/execute/pr61306-1.c: New test.
    * gcc.c-torture/execute/pr61306-2.c: Likewise.
    * gcc.c-torture/execute/pr61306-3.c: Likewise.

Added:
    branches/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c
    branches/gcc-4_9-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c
Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_9-branch/gcc/tree-ssa-math-opts.c

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thopre01 at gcc dot gnu.org]

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

--- Comment #9 from thopre01 at gcc dot gnu.org ---
Author: thopre01
Date: Mon Jun 30 02:11:21 2014
New Revision: 212134

URL: https://gcc.gnu.org/viewcvs?rev=212134&root=gcc&view=rev
Log:
2014-06-30  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    Backport from mainline
    2014-06-20  Jakub Jelinek  <jakub@redhat.com>
    2014-06-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>

    gcc/
    PR tree-optimization/61306
    * tree-ssa-math-opts.c (struct symbolic_number): Store type of
    expression instead of its size.
    (do_shift_rotate): Adapt to change in struct symbolic_number. Return
    false to prevent optimization when the result is unpredictable due to
    arithmetic right shift of signed type with highest byte is set.
    (verify_symbolic_number_p): Adapt to change in struct symbolic_number.
    (find_bswap_1): Likewise. Return NULL to prevent optimization when the
    result is unpredictable due to sign extension.
    (find_bswap): Adapt to change in struct symbolic_number.

    gcc/testsuite/
    * gcc.c-torture/execute/pr61306-1.c: New test.
    * gcc.c-torture/execute/pr61306-2.c: Likewise.
    * gcc.c-torture/execute/pr61306-3.c: Likewise.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c
    branches/gcc-4_8-branch/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_8-branch/gcc/tree-ssa-math-opts.c

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[olegendo at gcc dot gnu.org]

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

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |olegendo at gcc dot gnu.org

--- Comment #10 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to thopre01 from comment #6)
> Author: thopre01
> Date: Wed Jun 11 10:04:33 2014
> New Revision: 211444
> 
> URL: http://gcc.gnu.org/viewcvs?rev=211444&root=gcc&view=rev
> Log:
> 2014-06-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>     gcc/
>     PR tree-optimization/61306
>     * tree-ssa-math-opts.c (struct symbolic_number): Store type of
>     expression instead of its size.
>     (do_shift_rotate): Adapt to change in struct symbolic_number. Return
>     false to prevent optimization when the result is unpredictable due to
>     arithmetic right shift of signed type with highest byte is set.
>     (verify_symbolic_number_p): Adapt to change in struct symbolic_number.
>     (init_symbolic_number): Likewise.
>     (find_bswap_or_nop_1): Likewise. Return NULL to prevent optimization
>     when the result is unpredictable due to sign extension.
> 
>     gcc/testsuite/
>     * gcc.c-torture/execute/pr61306-1.c: New test.
>     * gcc.c-torture/execute/pr61306-2.c: Likewise.
>     * gcc.c-torture/execute/pr61306-3.c: Likewise.
> 
> Added:
>     trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-1.c
>     trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-2.c
>     trunk/gcc/testsuite/gcc.c-torture/execute/pr61306-3.c
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/testsuite/ChangeLog
>     trunk/gcc/tree-ssa-math-opts.c

It seems that after this commit, one SH bswap32 test started to fail.
Comparing the two test reports:
https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg00961.html
https://gcc.gnu.org/ml/gcc-testresults/2014-06/msg01086.html

shows a new failure for gcc.target/sh/pr53568-1.c.
The function

int
test_func_02 (int a)
{
  /* 1x swap.w
     2x swap.b  */
  return (((a >> 0) & 0xFF) << 24)
     | (((a >> 8) & 0xFF) << 16)
     | (((a >> 16) & 0xFF) << 8)
     | (((a >> 24) & 0xFF) << 0);
}

now fails to produce a bswap32.

[Bug tree-optimization/61306] [4.10 Regression] wrong code at -Os and above on x86_64-linux-gnu

[thopre01 at gcc dot gnu.org]

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

--- Comment #11 from thopre01 at gcc dot gnu.org ---
Confirmed. This is because the compiler will detect that the result of (a >> 8)
depends on the sign of a and thus prevent the optimization. Before this check
incorrect code could be generated. Of course in this case since the high bit or
discarded with a bitwise AND it is safe.

It is possible to detect this by marking the byte with unknown content with a
special marker (say 0xff) instead of cancelling the optimization and modify a
few places which checks for marker. This can be done for 4.10 but seems to me
to invasive for 4.8 and 4.9 where this patch was backported. If you disagree
with this we can discuss with the release manager once the patch made to see if
they would agree for a backport.

Thanks for your report.

[Bug tree-optimization/61306] [5 Regression] wrong code at -Os and above on x86_64-linux-gnu

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
      Known to work|4.10.0                      |5.0
         Resolution|---                         |FIXED

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Original bug fixed.