[Bug target/99905] New: wrong code with -mgeneral-regs-only

[Bug target/99905] New: wrong code with -mgeneral-regs-only

[zsojka at seznam dot cz]

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

            Bug ID: 99905
           Summary: wrong code with -mgeneral-regs-only
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu
            Target: x86_64-pc-linux-gnu

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

Output:
$ x86_64-pc-linux-gnu-gcc testcase.c -Os -mgeneral-regs-only
$ ./a.out 
Aborted

$ 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-7980-20210403205900-gc3d3bb0f03d-checking-yes-rtl-df-extra-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/11.0.1/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-7980-20210403205900-gc3d3bb0f03d-checking-yes-rtl-df-extra-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.0.1 20210404 (experimental) (GCC) 

Probably present since -mgeneral-regs-only introduction.

[Bug target/99905] wrong code with -mgeneral-regs-only

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-04-06
     Ever confirmed|0                           |1
                 CC|                            |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
> Probably present since -mgeneral-regs-only introduction.

Confirmed that, it's since r7-928-gce3a16ff1f59e6db.

[Bug target/99905] wrong code with -mgeneral-regs-only

[zsojka at seznam dot cz]

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

--- Comment #2 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Martin Liška from comment #1)
> > Probably present since -mgeneral-regs-only introduction.
> 
> Confirmed that, it's since r7-928-gce3a16ff1f59e6db.

Thank you for the bisection.
If -mgeneral-regs-only is not used, the flag can be replaced by -mno-mmx
-mno-sse:

$ x86_64-pc-linux-gnu-gcc -Os -mno-mmx -mno-sse testcase.c
$ ./a.out 
Aborted

[Bug target/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse

[zsojka at seznam dot cz]

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

Zdenek Sojka <zsojka at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|wrong code with             |[8/9/10/11 Regression]
                   |-mgeneral-regs-only         |wrong code with -mno-mmx
                   |                            |-mno-sse

--- Comment #3 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Zdenek Sojka from comment #2)
> (In reply to Martin Liška from comment #1)
> > > Probably present since -mgeneral-regs-only introduction.
> > 
> > Confirmed that, it's since r7-928-gce3a16ff1f59e6db.
> 
> Thank you for the bisection.
> If -mgeneral-regs-only is not used, the flag can be replaced by -mno-mmx
> -mno-sse:
> 
> $ x86_64-pc-linux-gnu-gcc -Os -mno-mmx -mno-sse testcase.c
> $ ./a.out 
> Aborted

... which seems a regression from gcc-6.5.0

[Bug target/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10/11 Regression]      |[8/9/10/11 Regression]
                   |wrong code with -mno-mmx    |wrong code with -mno-mmx
                   |-mno-sse                    |-mno-sse since
                   |                            |r7-4540-gb229ab2a712ccd44
                 CC|                            |matz at gcc dot gnu.org

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
> If -mgeneral-regs-only is not used, the flag can be replaced by -mno-mmx
> -mno-sse:

Good idea! Doing that, it started with r7-4540-gb229ab2a712ccd44.

[Bug target/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
   Target Milestone|---                         |8.5

[Bug target/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It indeed goes wrong during combine.  All the __builtin_bswap (i >> 63) + ...
stuff is gone.  Will have a look.

[Bug target/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Trying 7, 11, 13 -> 14:
    7: r109:TI=[argp:DI]
      REG_EQUIV [argp:DI]
   11: {r110:TI=r109:TI 0>>0x3f;clobber flags:CC;}
      REG_DEAD r109:TI
      REG_UNUSED flags:CC
   13: {r85:HI=r110:TI#0<-<0x8;clobber flags:CC;}
      REG_DEAD r110:TI
      REG_UNUSED flags:CC
   14: r86:SI=zero_extend(r85:HI#0)
      REG_DEAD r85:HI
Successfully matched this instruction:
(set (reg:SI 86 [ _6 ])
    (const_int 0 [0]))

looks like the bug.
We have
(set (reg:SI 86 [ _6 ])
    (zero_extend:SI (subreg:QI (rotate:HI (subreg:HI (lshiftrt:TI (reg/v:TI 109
[ i ])
                        (const_int 63 [0x3f])) 0)
                (const_int 8 [0x8])) 0)))
and are substing (mem/c:TI (reg/f:DI 16 argp) [3 i+0 S16 A128]) into
(reg/v:TI 109 [ i ]) and that gives the SET_SRC of const0_rtx.
subst first makes
(set (reg:SI 86 [ _6 ])
    (and:SI (subreg:SI (rotate:HI (subreg:HI (lshiftrt:TI (mem/c:TI (reg/f:DI
16 argp) [3 i+0 S16 A128])
                        (const_int 63 [0x3f])) 0)
                (const_int 8 [0x8])) 0)
        (const_int 255 [0xff])))
which still LGTM.

[Bug target/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The bug seems to be on extract_compound_operation called on
(zero_extract:DI (mem/c:TI (reg/f:DI 16 argp) [3 i+0 S16 A128])
    (const_int 16 [0x10])
    (const_int 63 [0x3f]))
We have:
  else if (unsignedp && len < HOST_BITS_PER_WIDE_INT)
    tem = simplify_and_const_int (NULL_RTX, mode,
                                  simplify_shift_const (NULL_RTX, LSHIFTRT,
                                                        mode, XEXP (x, 0),
                                                        pos),
                                  (HOST_WIDE_INT_1U << len) - 1);
but that calls simplify_shift_const with DImode and TImode MEM.
I think the shift needs to be done in inner_mode and the result then converted
to mode.

[Bug rtl-optimization/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

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

Untested fix.

[Bug rtl-optimization/99905] [8/9/10/11 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 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:ffc4155b6e45b8ab71d49a4b584f7cacb693e6b9

commit r11-8140-gffc4155b6e45b8ab71d49a4b584f7cacb693e6b9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 13 01:01:45 2021 +0200

    combine: Fix up expand_compound_operation [PR99905]

    The following testcase is miscompiled on x86_64-linux.
    expand_compound_operation is called on
    (zero_extract:DI (mem/c:TI (reg/f:DI 16 argp) [3 i+0 S16 A128])
        (const_int 16 [0x10])
        (const_int 63 [0x3f]))
    so mode is DImode, inner_mode is TImode, pos 63, len 16 and modewidth 64.

    A couple of lines above the problematic spot we have:
      if (modewidth >= pos + len)
        {
          tem = gen_lowpart (mode, XEXP (x, 0));
    where the code uses gen_lowpart and then shift left/right to extract it
    in mode.  But the guarding condition is false - 64 >= 63 + 16
    and so we enter the next condition, where the code shifts XEXP (x, 0)
    right by pos and then adds AND.  It does so incorrectly though.
    Given the modewidth < pos + len, inner_mode must be necessarily larger
    than mode and XEXP (x, 0) has the innermode, but it was calling
    simplify_shift_const with mode rather than inner_mode, which meant
    inconsistent arguments to simplify_shift_const and in this case made
    a DImode MEM shift out of it.

    The following patch fixes it, by doing the shift in inner_mode properly
    and then after the shift doing the lowpart subreg and masking already
    in mode.

    2021-04-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/99905
            * combine.c (expand_compound_operation): If pos + len > modewidth,
            perform the right shift by pos in inner_mode and then convert to
mode,
            instead of trying to simplify a shift of rtx with inner_mode by pos
            as if it was a shift in mode.

            * gcc.target/i386/pr99905.c: New test.

[Bug rtl-optimization/99905] [8/9/10 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10/11 Regression]      |[8/9/10 Regression] wrong
                   |wrong code with -mno-mmx    |code with -mno-mmx -mno-sse
                   |-mno-sse since              |since
                   |r7-4540-gb229ab2a712ccd44   |r7-4540-gb229ab2a712ccd44

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

[Bug rtl-optimization/99905] [8/9/10 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 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:c965254e5af9dc68444e0289250c393ae0cd6131

commit r10-9727-gc965254e5af9dc68444e0289250c393ae0cd6131
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 13 01:01:45 2021 +0200

    combine: Fix up expand_compound_operation [PR99905]

    The following testcase is miscompiled on x86_64-linux.
    expand_compound_operation is called on
    (zero_extract:DI (mem/c:TI (reg/f:DI 16 argp) [3 i+0 S16 A128])
        (const_int 16 [0x10])
        (const_int 63 [0x3f]))
    so mode is DImode, inner_mode is TImode, pos 63, len 16 and modewidth 64.

    A couple of lines above the problematic spot we have:
      if (modewidth >= pos + len)
        {
          tem = gen_lowpart (mode, XEXP (x, 0));
    where the code uses gen_lowpart and then shift left/right to extract it
    in mode.  But the guarding condition is false - 64 >= 63 + 16
    and so we enter the next condition, where the code shifts XEXP (x, 0)
    right by pos and then adds AND.  It does so incorrectly though.
    Given the modewidth < pos + len, inner_mode must be necessarily larger
    than mode and XEXP (x, 0) has the innermode, but it was calling
    simplify_shift_const with mode rather than inner_mode, which meant
    inconsistent arguments to simplify_shift_const and in this case made
    a DImode MEM shift out of it.

    The following patch fixes it, by doing the shift in inner_mode properly
    and then after the shift doing the lowpart subreg and masking already
    in mode.

    2021-04-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/99905
            * combine.c (expand_compound_operation): If pos + len > modewidth,
            perform the right shift by pos in inner_mode and then convert to
mode,
            instead of trying to simplify a shift of rtx with inner_mode by pos
            as if it was a shift in mode.

            * gcc.target/i386/pr99905.c: New test.

    (cherry picked from commit ffc4155b6e45b8ab71d49a4b584f7cacb693e6b9)

[Bug rtl-optimization/99905] [8/9 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[8/9/10 Regression] wrong   |[8/9 Regression] wrong code
                   |code with -mno-mmx -mno-sse |with -mno-mmx -mno-sse
                   |since                       |since
                   |r7-4540-gb229ab2a712ccd44   |r7-4540-gb229ab2a712ccd44

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.4 too.

[Bug rtl-optimization/99905] [8/9 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 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:00fc4d9fff1f38c205b52e0723eb3b03989ee292

commit r9-9446-g00fc4d9fff1f38c205b52e0723eb3b03989ee292
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 13 01:01:45 2021 +0200

    combine: Fix up expand_compound_operation [PR99905]

    The following testcase is miscompiled on x86_64-linux.
    expand_compound_operation is called on
    (zero_extract:DI (mem/c:TI (reg/f:DI 16 argp) [3 i+0 S16 A128])
        (const_int 16 [0x10])
        (const_int 63 [0x3f]))
    so mode is DImode, inner_mode is TImode, pos 63, len 16 and modewidth 64.

    A couple of lines above the problematic spot we have:
      if (modewidth >= pos + len)
        {
          tem = gen_lowpart (mode, XEXP (x, 0));
    where the code uses gen_lowpart and then shift left/right to extract it
    in mode.  But the guarding condition is false - 64 >= 63 + 16
    and so we enter the next condition, where the code shifts XEXP (x, 0)
    right by pos and then adds AND.  It does so incorrectly though.
    Given the modewidth < pos + len, inner_mode must be necessarily larger
    than mode and XEXP (x, 0) has the innermode, but it was calling
    simplify_shift_const with mode rather than inner_mode, which meant
    inconsistent arguments to simplify_shift_const and in this case made
    a DImode MEM shift out of it.

    The following patch fixes it, by doing the shift in inner_mode properly
    and then after the shift doing the lowpart subreg and masking already
    in mode.

    2021-04-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/99905
            * combine.c (expand_compound_operation): If pos + len > modewidth,
            perform the right shift by pos in inner_mode and then convert to
mode,
            instead of trying to simplify a shift of rtx with inner_mode by pos
            as if it was a shift in mode.

            * gcc.target/i386/pr99905.c: New test.

    (cherry picked from commit c965254e5af9dc68444e0289250c393ae0cd6131)

[Bug rtl-optimization/99905] [8/9 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[cvs-commit at gcc dot gnu.org]

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

--- Comment #14 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:ecb5f4b7d68e1e5ac0482bebc886b9273ffe1527

commit r8-10909-gecb5f4b7d68e1e5ac0482bebc886b9273ffe1527
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 13 01:01:45 2021 +0200

    combine: Fix up expand_compound_operation [PR99905]

    The following testcase is miscompiled on x86_64-linux.
    expand_compound_operation is called on
    (zero_extract:DI (mem/c:TI (reg/f:DI 16 argp) [3 i+0 S16 A128])
        (const_int 16 [0x10])
        (const_int 63 [0x3f]))
    so mode is DImode, inner_mode is TImode, pos 63, len 16 and modewidth 64.

    A couple of lines above the problematic spot we have:
      if (modewidth >= pos + len)
        {
          tem = gen_lowpart (mode, XEXP (x, 0));
    where the code uses gen_lowpart and then shift left/right to extract it
    in mode.  But the guarding condition is false - 64 >= 63 + 16
    and so we enter the next condition, where the code shifts XEXP (x, 0)
    right by pos and then adds AND.  It does so incorrectly though.
    Given the modewidth < pos + len, inner_mode must be necessarily larger
    than mode and XEXP (x, 0) has the innermode, but it was calling
    simplify_shift_const with mode rather than inner_mode, which meant
    inconsistent arguments to simplify_shift_const and in this case made
    a DImode MEM shift out of it.

    The following patch fixes it, by doing the shift in inner_mode properly
    and then after the shift doing the lowpart subreg and masking already
    in mode.

    2021-04-13  Jakub Jelinek  <jakub@redhat.com>

            PR rtl-optimization/99905
            * combine.c (expand_compound_operation): If pos + len > modewidth,
            perform the right shift by pos in inner_mode and then convert to
mode,
            instead of trying to simplify a shift of rtx with inner_mode by pos
            as if it was a shift in mode.

            * gcc.target/i386/pr99905.c: New test.

    (cherry picked from commit c965254e5af9dc68444e0289250c393ae0cd6131)

[Bug rtl-optimization/99905] [8/9 Regression] wrong code with -mno-mmx -mno-sse since r7-4540-gb229ab2a712ccd44

[jakub at gcc dot gnu.org]

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

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

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

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