[Bug tree-optimization/97386] New: [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[Bug tree-optimization/97386] New: [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[zsojka at seznam dot cz]

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

            Bug ID: 97386
           Summary: [8/9/10/11 Regression] wrong code with
                    __builtin_bswap16() of rotated 64bit value
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---

Created attachment 49353
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49353&action=edit
reduced testcase

Output:
$ x86_64-pc-linux-gnu-gcc -O testcase.c
$ ./a.out 
Aborted

The code in fact rotates 0xcccc00 instead of 0xcccc.

foo:
        mov     rcx, QWORD PTR c[rip]
        mov     eax, 13421568 # 0xcccc00
        rol     rax, cl
        ret

$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r11-3806-20201012001625-g2baa36d4911-checking-yes-rtl-df-extra-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/11.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--with-cloog --with-ppl --with-isl --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=x86_64-pc-linux-gnu
--with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r11-3806-20201012001625-g2baa36d4911-checking-yes-rtl-df-extra-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.0 20201012 (experimental) (GCC)

[Bug tree-optimization/97386] [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5
             Status|UNCONFIRMED                 |ASSIGNED
                 CC|                            |jakub at gcc dot gnu.org
   Last reconfirmed|                            |2020-10-12
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with my r8-3824-ga0e4174cd80f8a17026d6c13af2802dc8a0146ad
unsigned int c;

unsigned char
foo (void)
{
  return __builtin_bswap16 ((unsigned long long) (0xccccLLU << c | 0xccccLLU >>
((-c) & 63)));
}

int
main (void)
{
  unsigned char x = foo ();
  if (__CHAR_BIT__ == 8 && x != 0xcc)
    __builtin_abort ();
  return 0;
}

[Bug tree-optimization/97386] [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49355
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49355&action=edit
gcc11-pr97386.patch

Untested fix.

[Bug tree-optimization/97386] [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The actual bug is much older though, as shown e.g. on
__attribute__((noipa)) unsigned
foo (int x)
{
  unsigned long long a = (0x800000000000ccccULL << x) | (0x800000000000ccccULL
>> (64 - x));
  unsigned int b = a;
  return (b << 24) | (b >> 8);
}

int
main ()
{
  if (__CHAR_BIT__ != 8 || __SIZEOF_INT__ != 4 || __SIZEOF_LONG_LONG__ != 8)
    return 0;
  if (foo (1) != 0x99000199U)
    __builtin_abort ();
  return 0;
}
with the first if commented out and -O1 -fno-inline this is miscompiled e.g.
even with r105000.
For rotates, I guess we can swap them only if they have the same modes, or at
least we shouldn't if the outer one is narrower than the inner one.

[Bug tree-optimization/97386] [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 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:f76949cee9560d04d5417481dbcda5ca089c9ebc

commit r11-3855-gf76949cee9560d04d5417481dbcda5ca089c9ebc
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Oct 13 19:13:26 2020 +0200

    combine: Fix up simplify_shift_const_1 for nested ROTATEs [PR97386]

    The following testcases are miscompiled (the first one since my
improvements
    to rotate discovery on GIMPLE, the other one for many years) because
    combiner optimizes nested ROTATEs with narrowing SUBREG in between (i.e.
    the outer rotate is performed in shorter precision than the inner one) to
    just one ROTATE of the rotated constant.  While that (under certain
    conditions) can work for shifts, it can't work for rotates where we can
only
    do that with rotates of the same precision.

    2020-10-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/97386
            * combine.c (simplify_shift_const_1): Don't optimize nested ROTATEs
if
            they have different modes.

            * gcc.c-torture/execute/pr97386-1.c: New test.
            * gcc.c-torture/execute/pr97386-2.c: New test.

[Bug tree-optimization/97386] [8/9/10/11 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:1a98b22b0468214ae8463d075dacaeea1d46df15

commit r10-8902-g1a98b22b0468214ae8463d075dacaeea1d46df15
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Oct 13 19:13:26 2020 +0200

    combine: Fix up simplify_shift_const_1 for nested ROTATEs [PR97386]

    The following testcases are miscompiled (the first one since my
improvements
    to rotate discovery on GIMPLE, the other one for many years) because
    combiner optimizes nested ROTATEs with narrowing SUBREG in between (i.e.
    the outer rotate is performed in shorter precision than the inner one) to
    just one ROTATE of the rotated constant.  While that (under certain
    conditions) can work for shifts, it can't work for rotates where we can
only
    do that with rotates of the same precision.

    2020-10-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/97386
            * combine.c (simplify_shift_const_1): Don't optimize nested ROTATEs
if
            they have different modes.

            * gcc.c-torture/execute/pr97386-1.c: New test.
            * gcc.c-torture/execute/pr97386-2.c: New test.

    (cherry picked from commit f76949cee9560d04d5417481dbcda5ca089c9ebc)

[Bug tree-optimization/97386] [8/9 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10/11 Regression]      |[8/9 Regression] wrong code
                   |wrong code with             |with __builtin_bswap16() of
                   |__builtin_bswap16() of      |rotated 64bit value
                   |rotated 64bit value         |

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.3+ and 11.1+ so far.

[Bug tree-optimization/97386] [8/9 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:1471e383f4909e7d6bd548d010eb96afcf2d241e

commit r9-9390-g1471e383f4909e7d6bd548d010eb96afcf2d241e
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Oct 13 19:13:26 2020 +0200

    combine: Fix up simplify_shift_const_1 for nested ROTATEs [PR97386]

    The following testcases are miscompiled (the first one since my
improvements
    to rotate discovery on GIMPLE, the other one for many years) because
    combiner optimizes nested ROTATEs with narrowing SUBREG in between (i.e.
    the outer rotate is performed in shorter precision than the inner one) to
    just one ROTATE of the rotated constant.  While that (under certain
    conditions) can work for shifts, it can't work for rotates where we can
only
    do that with rotates of the same precision.

    2020-10-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/97386
            * combine.c (simplify_shift_const_1): Don't optimize nested ROTATEs
if
            they have different modes.

            * gcc.c-torture/execute/pr97386-1.c: New test.
            * gcc.c-torture/execute/pr97386-2.c: New test.

    (cherry picked from commit 1a98b22b0468214ae8463d075dacaeea1d46df15)

[Bug tree-optimization/97386] [8/9 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:615b19069a9afd9faf803d6888ac49d03065762d

commit r8-10860-g615b19069a9afd9faf803d6888ac49d03065762d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Oct 13 19:13:26 2020 +0200

    combine: Fix up simplify_shift_const_1 for nested ROTATEs [PR97386]

    The following testcases are miscompiled (the first one since my
improvements
    to rotate discovery on GIMPLE, the other one for many years) because
    combiner optimizes nested ROTATEs with narrowing SUBREG in between (i.e.
    the outer rotate is performed in shorter precision than the inner one) to
    just one ROTATE of the rotated constant.  While that (under certain
    conditions) can work for shifts, it can't work for rotates where we can
only
    do that with rotates of the same precision.

    2020-10-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/97386
            * combine.c (simplify_shift_const_1): Don't optimize nested ROTATEs
if
            they have different modes.

            * gcc.c-torture/execute/pr97386-1.c: New test.
            * gcc.c-torture/execute/pr97386-2.c: New test.

    (cherry picked from commit 1a98b22b0468214ae8463d075dacaeea1d46df15)

[Bug tree-optimization/97386] [8/9 Regression] wrong code with __builtin_bswap16() of rotated 64bit value

[jakub at gcc dot gnu.org]

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

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

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

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