[Bug translation/94500] New: Wrong maximum value with small integer types and AVX-512

[Bug translation/94500] New: Wrong maximum value with small integer types and AVX-512

[ben at rockshrub dot de]

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

            Bug ID: 94500
           Summary: Wrong maximum value with small integer types and
                    AVX-512
           Product: gcc
           Version: 8.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: translation
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ben at rockshrub dot de
  Target Milestone: ---

Created attachment 48211
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48211&action=edit
C++ program that triggers the bug

Both g++ and gfortran produce incorrect code for computing maximum values of
arrays of integer types smaller than 32 bit when optimizing for usage of
AVX-512. This can be triggered by using std::max_element in C++ or Fortran's
maxval.

The generated program uses 32-bit shuffles to reduce a 512-bit vector into a
final 8-bit or 16-bit result (set smallint to int8_t or int16_t), so if the
true maximum value never ends up in the least significant bytes of one of the
32-bit elements in the vector, it is never found.

The issue persists on GCC 8.4.0, but seems to be fixed on 9.X versions of GCC.

The attached program fills a vector in a way that triggers the error and then
computes the maximum value once "by hand" and once using std::max_element. It
prints the results of the two computations which should be identical.

Compiler command lines:
g++ -O3 -march=skylake-avx512 -mprefer-vector-width=512 -o bug_cxx bug_cxx.cxx
gfortran -O3 -march=skylake-avx512 -mprefer-vector-width=512 -o bug_f90
bug_f90.f90

No errors or warnings are encountered at -Wall -Wextra.

gcc -v:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/gpfs/software/juwels/stages/2019a/software/GCCcore/8.3.0/libexec/gcc/x86_64-pc-linux-gnu/8.3.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --enable-languages=c,c++,fortran --enable-lto
--enable-checking=release --disable-multilib --enable-shared=yes
--enable-static=yes --enable-threads=posix --enable-plugins
--enable-gold=default --enable-ld --with-plugin-ld=ld.gold
--prefix=/gpfs/software/juwels/stages/2019a/software/GCCcore/8.3.0
--with-local-prefix=/gpfs/software/juwels/stages/2019a/software/GCCcore/8.3.0
--enable-bootstrap
--with-isl=/dev/shm/swmanage/GCCcore/8.3.0/dummy-/gcc-8.3.0/stage2_stuff
Thread model: posix
gcc version 8.3.0 (GCC)

[Bug translation/94500] Wrong maximum value with small integer types and AVX-512

[ben at rockshrub dot de]

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

--- Comment #1 from Benedikt Steinbusch <ben at rockshrub dot de> ---
Created attachment 48212
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48212&action=edit
Fortran program that triggers the bug

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-04-06
                 CC|                            |jakub at gcc dot gnu.org
           Keywords|needs-bisection             |
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
At least the #c0 testcase works in 9/10 since
r9-23-g1c53fa8ca9912f6acf8c3c8cec31d647bd8ba691 , though that might just mean
it is latent.
The #c1 testcase works in 9/10 since
r9-3393-g6cdd5aecfb4e062354db8f7253240a371ba418af
Even GCC 5 does this at least on the #c1 testcase, and 4.9 didn't support
-mavx512bw which is needed to reproduce this.

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This looks like a severe bug in emit_reduc_half for V{64QI,32HI}mode where it
handles correctly the i 512, 256, 128, 64 steps, but not the i == 32 step
needed as last for V32HImode or both the i == 16 and i == 8 steps needed for
V64QImode.

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48214
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48214&action=edit
gcc8-pr94500.patch

Untested fix.  Richi's r265004 commit has been an optimization which
essentially causes ix86_expand_reduc not to be called with these modes (but the
bug is still there if it were ever called with those modes).

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[cvs-commit at gcc dot gnu.org]

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

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

commit r10-7581-gbee27152f7e6651f38c25ac68db13370382147e0
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 7 08:27:49 2020 +0200

    i386: Fix emit_reduc_half on V{64Q,32H}Imode [PR94500]

    The following testcase is miscompiled in 8.x, because emit_reduc_half is
    prepared to handle for 512-bit modes only i equal to 512, 256, 128 and 64.
    V32HImode also needs i equal to 32 and V64QImode i equal to 32 and 16,
    but emit_reduc_half in that case performs a redundant permutation exactly
    like i == 32.  In 9+ the testcase works because Richard in r9-3393
    changed the reduc_* expanders so that they actually don't call
    ix86_expand_reduc on 512-bit modes, but only 128-bit ones.

    The patch fixes emit_reduc_half to handle also i of 32 and 16 similarly to
    how V32QImode/V16HImode are handled for AVX2.  I think it shouldn't hurt
    to fix the function even on the trunk and 9 branch even when nothing uses
    it ATM.

    2020-04-07  Jakub Jelinek  <jakub@redhat.com>

            PR target/94500
            * config/i386/i386-expand.c (emit_reduc_half): For V{64QI,32HI}mode
            handle i < 64 using avx512bw_lshrv4ti3.  Formatting fixes.

            * gcc.target/i386/avx512bw-pr94500.c: New test.

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 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:1168240fbe41ee509795a67d5b9107f233d17df9

commit r9-8459-g1168240fbe41ee509795a67d5b9107f233d17df9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 7 08:27:49 2020 +0200

    i386: Fix emit_reduc_half on V{64Q,32H}Imode [PR94500]

    The following testcase is miscompiled in 8.x, because emit_reduc_half is
    prepared to handle for 512-bit modes only i equal to 512, 256, 128 and 64.
    V32HImode also needs i equal to 32 and V64QImode i equal to 32 and 16,
    but emit_reduc_half in that case performs a redundant permutation exactly
    like i == 32.  In 9+ the testcase works because Richard in r9-3393
    changed the reduc_* expanders so that they actually don't call
    ix86_expand_reduc on 512-bit modes, but only 128-bit ones.

    The patch fixes emit_reduc_half to handle also i of 32 and 16 similarly to
    how V32QImode/V16HImode are handled for AVX2.  I think it shouldn't hurt
    to fix the function even on the trunk and 9 branch even when nothing uses
    it ATM.

    2020-04-07  Jakub Jelinek  <jakub@redhat.com>

            PR target/94500
            * config/i386/i386.c (emit_reduc_half): For V{64QI,32HI}mode
            handle i < 64 using avx512bw_lshrv4ti3.  Formatting fixes.

            * gcc.target/i386/avx512bw-pr94500.c: New test.

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[cvs-commit at gcc dot gnu.org]

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

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

commit r8-10170-g11b19dfc082607e5049f72f1ef954fda7b42e8d4
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Apr 7 08:27:49 2020 +0200

    i386: Fix emit_reduc_half on V{64Q,32H}Imode [PR94500]

    The following testcase is miscompiled in 8.x, because emit_reduc_half is
    prepared to handle for 512-bit modes only i equal to 512, 256, 128 and 64.
    V32HImode also needs i equal to 32 and V64QImode i equal to 32 and 16,
    but emit_reduc_half in that case performs a redundant permutation exactly
    like i == 32.  In 9+ the testcase works because Richard in r9-3393
    changed the reduc_* expanders so that they actually don't call
    ix86_expand_reduc on 512-bit modes, but only 128-bit ones.

    The patch fixes emit_reduc_half to handle also i of 32 and 16 similarly to
    how V32QImode/V16HImode are handled for AVX2.  I think it shouldn't hurt
    to fix the function even on the trunk and 9 branch even when nothing uses
    it ATM.

    2020-04-07  Jakub Jelinek  <jakub@redhat.com>

            PR target/94500
            * config/i386/i386.c (emit_reduc_half): For V{64QI,32HI}mode
            handle i < 64 using avx512bw_lshrv4ti3.  Formatting fixes.

            * gcc.target/i386/avx512bw-pr94500.c: New test.

[Bug tree-optimization/94500] Wrong maximum value with small integer types and AVX-512

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 8.5+.