[Bug tree-optimization/105591] New: [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2

[Bug tree-optimization/105591] New: [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2

[zsojka at seznam dot cz]

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

            Bug ID: 105591
           Summary: [13 Regression] ICE: in tree_to_poly_uint64, at
                    tree.cc:3250 with -O -mavx512f -mno-avx2
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zsojka at seznam dot cz
  Target Milestone: ---
              Host: x86_64-pc-linux-gnu

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

Compiler output:
$ x86_64-pc-linux-gnu-gcc -O -mavx512f -mno-avx2 testcase.c
during GIMPLE pass: forwprop
testcase.c: In function 'foo':
testcase.c:5:1: internal compiler error: in tree_to_poly_uint64, at
tree.cc:3250
    5 | foo (U u)
      | ^~~
0x7ffb50 tree_to_poly_uint64(tree_node const*)
        /repo/gcc-trunk/gcc/tree.cc:3250
0x7ffb50 tree_to_poly_uint64(tree_node const*)
        /repo/gcc-trunk/gcc/tree.cc:3248
0x14ed216 bit_field_offset(tree_node const*)
        /repo/gcc-trunk/gcc/tree.h:5370
0x14ed216 simplify_vector_constructor
        /repo/gcc-trunk/gcc/tree-ssa-forwprop.cc:2763
0x14ed216 execute
        /repo/gcc-trunk/gcc/tree-ssa-forwprop.cc:3755
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

$ 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-r13-413-20220513111901-g7b844206ec5-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/13.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
--disable-bootstrap --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-r13-413-20220513111901-g7b844206ec5-checking-yes-rtl-df-extra-nobootstrap-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.0.0 20220513 (experimental) (GCC)

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-05-13
     Ever confirmed|0                           |1
                 CC|                            |liuhongt at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.  The offset is very large negative, built by simplify_bitfield_ref
here:

2416      tem = build3 (BIT_FIELD_REF, TREE_TYPE (op),
2417                    p, op1, bitsize_int (idx * elem_size));
(gdb) p idx
$11 = 13704120150654661628

because we have a non-sensical permute:

x_4 = VEC_PERM_EXPR <u_3(D), u_3(D), { 13704120150654661630, 0 }>;

some sanity checking in simplify_bitfield_ref might be in order.  Rejecting the
__builtin_shuffle might be another option, it's surely going to cause issues
elsewhere.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2

[crazylht at gmail dot com]

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

Hongtao.liu <crazylht at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |crazylht at gmail dot com

--- Comment #2 from Hongtao.liu <crazylht at gmail dot com> ---
Mine.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We certainly can't reject the __builtin_shuffle, it is valid.
"The elements of the input vectors are numbered in memory ordering of
VEC0 beginning at 0 and VEC1 beginning at N.  The elements of MASK are
considered modulo N in the single-operand case and modulo 2*N in the
two-operand case."

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[zsojka at seznam dot cz]

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

--- Comment #4 from Zdenek Sojka <zsojka at seznam dot cz> ---
(In reply to Richard Biener from comment #1)
> Confirmed.  The offset is very large negative, built by simplify_bitfield_ref
> here:
> 
> 2416      tem = build3 (BIT_FIELD_REF, TREE_TYPE (op),
> 2417                    p, op1, bitsize_int (idx * elem_size));
> (gdb) p idx
> $11 = 13704120150654661628
> 
> because we have a non-sensical permute:
> 
> x_4 = VEC_PERM_EXPR <u_3(D), u_3(D), { 13704120150654661630, 0 }>;
> 
> some sanity checking in simplify_bitfield_ref might be in order.  Rejecting
> the __builtin_shuffle might be another option, it's surely going to cause
> issues elsewhere.

Indeed I am triggering probably related failures elsewhere:

testcase-min0.i:54:5: error: invalid position or size operand to
'bit_field_ref'
   54 | int main(void)
      |     ^~~~
_66 = BIT_FIELD_REF <{ 2305807824841605120, 18446462598732840960, 65535,
268431360, 65535, 68718428160, 18446181123756130304, 72056494526300160 }, 64,
0xfffffffffffffffff0a1a0c0f3732940>;
testcase-min0.i:54:5: error: position plus size exceeds size of referenced
object in 'bit_field_ref'
_89 = BIT_FIELD_REF <{ 2305807824841605120, 18446462598732840960, 65535,
268431360, 65535, 68718428160, 18446181123756130304, 72056494526300160 }, 64,
5392423424>;
...

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, it is correct that the code uses TREE_INT_CST_LOW (at least as long as we
only support power of 2 nelts vectors), but in all places it uses it it should
mask it with nelts-1 or 2*nelts-1 depending on whether it is a 2 or 3 arg
VEC_PERM_EXPR.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[crazylht at gmail dot com]

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

--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
Maybe we should add an canonicalization in match.pd to make sure index is in
range of 0 - 2*N, and the general code need't to do check idx % 2*N.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Hongtao.liu from comment #6)
> Maybe we should add an canonicalization in match.pd to make sure index is in
> range of 0 - 2*N, and the general code need't to do check idx % 2*N.

That wouldn't work.  You can't rely on optimization happening somewhere else.
The VEC_PERM_EXPR operand could be substituted somewhere else and match.pd
could trigger on it only later or not at all.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[jakub at gcc dot gnu.org]

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

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
And doing it in match.pd would only hide bugs in handling it properly.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[jakub at gcc dot gnu.org]

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

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, APIs like vec-perm-indices.h do the clamping already, it is just about
code that doesn't use those APIs.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[crazylht at gmail dot com]

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

--- Comment #10 from Hongtao.liu <crazylht at gmail dot com> ---
Understand code like builtin_shuffle may have out-range index which needs to be
clamped, but why vec_perm_expr also needs to accept that.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[jakub at gcc dot gnu.org]

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

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Because VEC_PERM_EXPR doesn't require the mask argument to be constant (and
neither does __builtin_shuffle, unlike e.g. __builtin_shufflevector).
If the mask argument remains non-constant until end of compilation, the modulo
N or 2*N needs to be done at runtime (of course unless used hw instruction
performs something like that itself).
And that is the reason why it is defined this way.  During compilation there
are way too many spots where a constant could be propagated into a formerly
non-constant operand of the VEC_PERM_EXPR, and no guarantee that all such
propagations (it isn't in a single spot in a single pass, it is really many)
will do some extra code to canonicalize it.  It can be, but we can't guarantee
it.
For __builtin_shufflevector, we supposedly want to introduce some VEC_PERM_EXPR
variant which would only allow constant mask argument and which would have
different behavior, -1 standing for I don't care rather than -1 % N or -1 %
(2*N).  If we introduce something like that, we could certainly require that
the new expr's operand is only -1..N-1 or -1..2*N-1 and could have then say
match.pd that turns a VEC_PERM_EXPR with a constant argument into the new expr
with canonical argument.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[crazylht at gmail dot com]

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

--- Comment #12 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Jakub Jelinek from comment #11)
> Because VEC_PERM_EXPR doesn't require the mask argument to be constant (and
> neither does __builtin_shuffle, unlike e.g. __builtin_shufflevector).
> If the mask argument remains non-constant until end of compilation, the
> modulo N or 2*N needs to be done at runtime (of course unless used hw
> instruction performs something like that itself).
> And that is the reason why it is defined this way.  During compilation there
> are way too many spots where a constant could be propagated into a formerly
> non-constant operand of the VEC_PERM_EXPR, and no guarantee that all such
> propagations (it isn't in a single spot in a single pass, it is really many)
> will do some extra code to canonicalize it.  It can be, but we can't
> guarantee it.
> For __builtin_shufflevector, we supposedly want to introduce some
> VEC_PERM_EXPR variant which would only allow constant mask argument and
> which would have different behavior, -1 standing for I don't care rather
> than -1 % N or -1 % (2*N).  If we introduce something like that, we could
> certainly require that the new expr's operand is only -1..N-1 or -1..2*N-1
> and could have then say match.pd that turns a VEC_PERM_EXPR with a constant
> argument into the new expr with canonical argument.

Makes sense, thanks for the explanation.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[cvs-commit at gcc dot gnu.org]

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

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by hongtao Liu <liuhongt@gcc.gnu.org>:

https://gcc.gnu.org/g:1fba0608d12a209a5d76d65bcb1dec1c07bc33e9

commit r13-517-g1fba0608d12a209a5d76d65bcb1dec1c07bc33e9
Author: liuhongt <hongtao.liu@intel.com>
Date:   Mon May 16 09:06:04 2022 +0800

    Clamp vec_perm_expr index in simplify_bitfield_ref to avoid ICE.

    gcc/ChangeLog:

            PR tree-optimization/105591
            * tree-ssa-forwprop.cc (simplify_bitfield_ref): Clamp
            vec_perm_expr index.

    gcc/testsuite/ChangeLog:

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

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[crazylht at gmail dot com]

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

--- Comment #14 from Hongtao.liu <crazylht at gmail dot com> ---
Fixed in GCC13.

[Bug tree-optimization/105591] [13 Regression] ICE: in tree_to_poly_uint64, at tree.cc:3250 with -O -mavx512f -mno-avx2 since r13-379

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #15 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.