[Bug target/101384] New: wrong code at -Og and above with vector shift/multiply

[Bug target/101384] New: wrong code at -Og and above with vector shift/multiply

[zsojka at seznam dot cz]

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

            Bug ID: 101384
           Summary: wrong code at -Og and above with vector shift/multiply
           Product: gcc
           Version: 12.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: powerpc64le-unknown-linux-gnu

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

Output:
$ powerpc64le-unknown-linux-gnu-gcc -Og testcase.c -Wno-psabi -static
$ qemu-ppc64le -- ./a.out 
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted

The vector is:
80808080808080808080808080808080
instead of
80ffffff80ffffff80ffffff80ffffff

 # testcase.c:13: }
        vspltisw 0,-1    # tmp121
        vslb 0,0,0       # tmp121, tmp121, tmp121

looks strange, but simply replacing vslb->vslw doesn't help, as another operand
with 7s for the shift would be needed

$ powerpc64le-unknown-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-powerpc64le/bin/powerpc64le-unknown-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r12-2137-20210708114755-g4c619132b3f-checking-yes-rtl-df-extra-powerpc64le/bin/../libexec/gcc/powerpc64le-unknown-linux-gnu/12.0.0/lto-wrapper
Target: powerpc64le-unknown-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
--with-sysroot=/usr/powerpc64le-unknown-linux-gnu --build=x86_64-pc-linux-gnu
--host=x86_64-pc-linux-gnu --target=powerpc64le-unknown-linux-gnu
--with-ld=/usr/bin/powerpc64le-unknown-linux-gnu-ld
--with-as=/usr/bin/powerpc64le-unknown-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r12-2137-20210708114755-g4c619132b3f-checking-yes-rtl-df-extra-powerpc64le
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 12.0.0 20210708 (experimental) (GCC)

[Bug target/101384] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Simpler testcase with no vector type casts which is also works for both
endians:

typedef unsigned char __attribute__((__vector_size__ (16))) U;

U u;

U
foo (void)
{
  U y = (U) { 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff,
              0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff } + u;
  return y;
}

int
main ()
{
  U x = (U) foo ();
  for (unsigned i = 0; i < 16; i++)
    if (x[i] != ((i & 3) ? 0xff : 0x80))
      __builtin_abort ();
  return 0;
}

[Bug target/101384] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
r226407 works fine, r230000 already fails (even at -O0 or -O2 or -Og).

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
            Summary|wrong code at -Og and above |[9/10/11/12 Regression]
                   |with vector shift/multiply  |wrong code at -Og and above
                   |                            |with vector shift/multiply
           Priority|P3                          |P2
   Last reconfirmed|                            |2021-07-13
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |9.5

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

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
             Status|NEW                         |ASSIGNED

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

Untested fix.  easy_altivec_constant has code to try construct vector constants
with different element sizes, perhaps different from CONST_VECTOR's mode.  But
as written, that works fine for vspltis[bhw] cases, but not for the vspltisw
x,-1; vsl[bhw] x,x,x case, because that creates always a V16QImode, V8HImode or
V4SImode constant containing broadcasted constant with just the MSB set.
Perhaps for backports it would be best to limit the EASY_VECTOR_MSB case
matching to step == 1 && copies == 1, because that is the only case the
splitter handled correctly, but as can be seen in the gcc.target tests, the
patch
tries to handle it for all the cases.

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[segher at gcc dot gnu.org]

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

--- Comment #4 from Segher Boessenkool <segher at gcc dot gnu.org> ---
The testcase works fine for me?  What does it need to fail?

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> Created attachment 51146 [details]
> gcc12-pr101384.patch

Bootstrap/regtest found a bug in the gcc.dg/pr101384.c test,
  if (y[i] != ((i & 3) ? 0xffff : 0x8000))
should be
  if (y[i] != ((i & 1) ? 0xffff : 0x8000))
Doing bootstrap/regtest now without the patch (just with the new tests) to have
base against which to compare and will see if the test fails there.
When compiling with cross-compiler, just gcc -O2 -S pr101384.c , copying to
GCCFarm and running there reproduces it.

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[segher at gcc dot gnu.org]

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

--- Comment #6 from Segher Boessenkool <segher at gcc dot gnu.org> ---
I did run on the cfarm.  What kind of machine does it need to fail?  p8?  p9?

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
E.g. p8, on gcc112.fsffrance.org, vanilla gcc configured with
../configure --enable-languages=all,obj-c++,lto,go,d
--enable-checking=yes,rtl,extra
with the https://gcc.gnu.org/pipermail/gcc-patches/2021-July/575118.html
testsuite/gcc.dg/pr101384.c testcase:
./xgcc -B ./ -O2 -o pr101384{,.c}; ./pr101384; echo $?
Aborted
134
or
./xgcc -B ./ -O2 -mcpu=power8 -o pr101384{,.c}; ./pr101384; echo $?
Aborted
134
It works fine with the 4.8 system gcc:
gcc -o pr101384{,.c} -std=c99 -Dnoipa='noinline,noclone' -O2; ./pr101384; echo
$?
0
And with the patched compiler:
./xgcc -B ./ -O2 -o pr101384{,.c}; ./pr101384; echo $?
0
The broken compilers will not load the { 0x80, 0xff, 0xff, 0xff, 0x80, 0xff,
0xff, 0xff, 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff } vector (and the
other one too) from .rodata, but computes it using vspltisw reg,-1; vslb
reg,reg,reg but that constructs { 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80,
0x80,  0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80 } vector instead.

[Bug target/101384] [9/10/11/12 Regression] wrong code at -Og and above with vector shift/multiply

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 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:dc386b020869ad0095cf58f8c76a40ea457e7a2c

commit r11-8789-gdc386b020869ad0095cf58f8c76a40ea457e7a2c
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jul 20 16:41:29 2021 +0200

    rs6000: Fix up easy_vector_constant_msb handling [PR101384]

    The following gcc.dg/pr101384.c testcase is miscompiled on
    powerpc64le-linux.
    easy_altivec_constant has code to try construct vector constants with
    different element sizes, perhaps different from CONST_VECTOR's mode.  But
as
    written, that works fine for vspltis[bhw] cases, but not for the vspltisw
    x,-1; vsl[bhw] x,x,x case, because that creates always a V16QImode,
V8HImode
    or V4SImode constant containing broadcasted constant with just the MSB set.
    The vspltis_constant function etc. expects the vspltis[bhw] instructions
    where the small [-16..15] or even [-32..30] constant is sign-extended to
the
    remaining step bytes, but that is not the case for the 0x80...00 constants,
    with step 1 we can't handle e.g.
    { 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff,
0x80, 0xff, 0xff, 0xff }
    vectors but do want to handle e.g.
    { 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80 }
    and similarly with copies 1 we do want to handle e.g.
    { 0x80808080, 0x80808080, 0x80808080, 0x80808080 }.

    This is a simpler version of the fix for backports, which limits the
EASY_VECTOR_MSB case
    matching to step == 1 && copies == 1, because that is the only case the
    splitter handles correctly.

    2021-07-20  Jakub Jelinek  <jakub@redhat.com>

            PR target/101384
            * config/rs6000/rs6000.c (vspltis_constant): Accept EASY_VECTOR_MSB
            only if step and copies are equal to 1.

            * gcc.dg/pr101384.c: New test.

[Bug target/101384] [9/10/12 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|11.1.1                      |11.1.0
      Known to work|                            |11.1.1
            Summary|[9/10/11/12 Regression]     |[9/10/12 Regression] wrong
                   |wrong code at -Og and above |code at -Og and above with
                   |with vector shift/multiply  |vector shift/multiply

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.2 so far, not fixed on trunk yet (for trunk I've posted a larger
patch that hasn't been reviewed yet), and I'll do 10/9 backports later.

[Bug target/101384] [9/10/12 Regression] wrong code at -Og and above with vector shift/multiply

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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:e0e82856d535f56c916382f892ed2435dde54d4d

commit r12-2416-ge0e82856d535f56c916382f892ed2435dde54d4d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jul 20 17:26:10 2021 +0200

    rs6000: Fix up easy_vector_constant_msb handling [PR101384]

    The following gcc.dg/pr101384.c testcase is miscompiled on
    powerpc64le-linux.
    easy_altivec_constant has code to try construct vector constants with
    different element sizes, perhaps different from CONST_VECTOR's mode.  But
as
    written, that works fine for vspltis[bhw] cases, but not for the vspltisw
    x,-1; vsl[bhw] x,x,x case, because that creates always a V16QImode,
V8HImode
    or V4SImode constant containing broadcasted constant with just the MSB set.
    The vspltis_constant function etc. expects the vspltis[bhw] instructions
    where the small [-16..15] or even [-32..30] constant is sign-extended to
the
    remaining step bytes, but that is not the case for the 0x80...00 constants,
    with step > 1 we can't handle e.g.
    { 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff,
0x80, 0xff, 0xff, 0xff }
    vectors but do want to handle e.g.
    { 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80 }
    and similarly with copies > 1 we do want to handle e.g.
    { 0x80808080, 0x80808080, 0x80808080, 0x80808080 }.

    2021-07-20  Jakub Jelinek  <jakub@redhat.com>

            PR target/101384
            * config/rs6000/rs6000-protos.h (easy_altivec_constant): Change
return
            type from bool to int.
            * config/rs6000/rs6000.c (vspltis_constant): Fix up handling the
            EASY_VECTOR_MSB case if either step or copies is not 1.
            (vspltis_shifted): Fix comment typo.
            (easy_altivec_constant): Change return type from bool to int,
instead
            of returning true return byte size of the element mode that should
be
            used to synthetize the constant.
            * config/rs6000/predicates.md (easy_vector_constant_msb): Require
            that vspltis_shifted is 0, handle the case where
easy_altivec_constant
            assumes using different vector mode from CONST_VECTOR's mode.
            * config/rs6000/altivec.md (easy_vector_constant_msb splitter): Use
            easy_altivec_constant to determine mode in which -1 >> -1 should be
            performed, use rs6000_expand_vector_init instead of
gen_vec_initv4sisi.

            * gcc.dg/pr101384.c: New test.
            * gcc.target/powerpc/pr101384-1.c: New test.
            * gcc.target/powerpc/pr101384-2.c: New test.

[Bug target/101384] [9/10 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|12.0                        |
      Known to work|                            |12.0
            Summary|[9/10/12 Regression] wrong  |[9/10 Regression] wrong
                   |code at -Og and above with  |code at -Og and above with
                   |vector shift/multiply       |vector shift/multiply

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

[Bug target/101384] [9/10 Regression] wrong code at -Og and above with vector shift/multiply

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 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:7aca564ed7e7ac54e1aab30805ba7c38309bdfdb

commit r10-10635-g7aca564ed7e7ac54e1aab30805ba7c38309bdfdb
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jul 20 16:41:29 2021 +0200

    rs6000: Fix up easy_vector_constant_msb handling [PR101384]

    The following gcc.dg/pr101384.c testcase is miscompiled on
    powerpc64le-linux.
    easy_altivec_constant has code to try construct vector constants with
    different element sizes, perhaps different from CONST_VECTOR's mode.  But
as
    written, that works fine for vspltis[bhw] cases, but not for the vspltisw
    x,-1; vsl[bhw] x,x,x case, because that creates always a V16QImode,
V8HImode
    or V4SImode constant containing broadcasted constant with just the MSB set.
    The vspltis_constant function etc. expects the vspltis[bhw] instructions
    where the small [-16..15] or even [-32..30] constant is sign-extended to
the
    remaining step bytes, but that is not the case for the 0x80...00 constants,
    with step 1 we can't handle e.g.
    { 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff,
0x80, 0xff, 0xff, 0xff }
    vectors but do want to handle e.g.
    { 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80 }
    and similarly with copies 1 we do want to handle e.g.
    { 0x80808080, 0x80808080, 0x80808080, 0x80808080 }.

    This is a simpler version of the fix for backports, which limits the
EASY_VECTOR_MSB case
    matching to step == 1 && copies == 1, because that is the only case the
    splitter handles correctly.

    2021-07-20  Jakub Jelinek  <jakub@redhat.com>

            PR target/101384
            * config/rs6000/rs6000.c (vspltis_constant): Accept EASY_VECTOR_MSB
            only if step and copies are equal to 1.

            * gcc.dg/pr101384.c: New test.

    (cherry picked from commit dc386b020869ad0095cf58f8c76a40ea457e7a2c)

[Bug target/101384] [9/10 Regression] wrong code at -Og and above with vector shift/multiply

[cvs-commit at gcc dot gnu.org]

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

--- 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:883690ff6bd8e4f16c871b307cebb13d6c14edcb

commit r9-10092-g883690ff6bd8e4f16c871b307cebb13d6c14edcb
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jul 20 16:41:29 2021 +0200

    rs6000: Fix up easy_vector_constant_msb handling [PR101384]

    The following gcc.dg/pr101384.c testcase is miscompiled on
    powerpc64le-linux.
    easy_altivec_constant has code to try construct vector constants with
    different element sizes, perhaps different from CONST_VECTOR's mode.  But
as
    written, that works fine for vspltis[bhw] cases, but not for the vspltisw
    x,-1; vsl[bhw] x,x,x case, because that creates always a V16QImode,
V8HImode
    or V4SImode constant containing broadcasted constant with just the MSB set.
    The vspltis_constant function etc. expects the vspltis[bhw] instructions
    where the small [-16..15] or even [-32..30] constant is sign-extended to
the
    remaining step bytes, but that is not the case for the 0x80...00 constants,
    with step 1 we can't handle e.g.
    { 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff, 0x80, 0xff, 0xff, 0xff,
0x80, 0xff, 0xff, 0xff }
    vectors but do want to handle e.g.
    { 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80, 0, 0, 0, 0x80 }
    and similarly with copies 1 we do want to handle e.g.
    { 0x80808080, 0x80808080, 0x80808080, 0x80808080 }.

    This is a simpler version of the fix for backports, which limits the
EASY_VECTOR_MSB case
    matching to step == 1 && copies == 1, because that is the only case the
    splitter handles correctly.

    2021-07-20  Jakub Jelinek  <jakub@redhat.com>

            PR target/101384
            * config/rs6000/rs6000.c (vspltis_constant): Accept EASY_VECTOR_MSB
            only if step and copies are equal to 1.

            * gcc.dg/pr101384.c: New test.

    (cherry picked from commit dc386b020869ad0095cf58f8c76a40ea457e7a2c)

[Bug target/101384] [9/10 Regression] wrong code at -Og and above with vector shift/multiply

[jakub at gcc dot gnu.org]

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

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

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

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