[Bug target/99767] New: [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[Bug target/99767] New: [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[asolokha at gmx dot com]

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

            Bug ID: 99767
           Summary: [9/10/11 Regression] ICE in expand_direct_optab_fn, at
                    internal-fn.c:3360
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: ice-on-invalid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: asolokha at gmx dot com
  Target Milestone: ---
            Target: aarch64-linux-gnu

gcc-11.0.1-alpha20210321 snapshot (g:fc24ea2374259d401a46ce3526688b7e79d4cc13)
ICEs when compiling the following testcase w/ -march=armv8-a+sve -O1
-fno-tree-dce:

int __attribute__ ((simd))
foo (void)
{
  int v = 1 / 0;

  return 0;
}

% aarch64-linux-gnu-gcc-11.0.1 -march=armv8-a+sve -O1 -fno-tree-dce -w -c
yiptql8b.c
during RTL pass: expand
yiptql8b.c: In function 'foo.simdclone.1':
yiptql8b.c:4:7: internal compiler error: in expand_direct_optab_fn, at
internal-fn.c:3360
    4 |   int v = 1 / 0;
      |       ^
0x70a2a1 expand_direct_optab_fn
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210321/work/gcc-11-20210321/gcc/internal-fn.c:3360
0xa15697 expand_call_stmt
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210321/work/gcc-11-20210321/gcc/cfgexpand.c:2749
0xa15697 expand_gimple_stmt_1
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210321/work/gcc-11-20210321/gcc/cfgexpand.c:3844
0xa15697 expand_gimple_stmt
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210321/work/gcc-11-20210321/gcc/cfgexpand.c:4008
0xa1b41a expand_gimple_basic_block
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210321/work/gcc-11-20210321/gcc/cfgexpand.c:6045
0xa1cfbf execute
       
/var/tmp/portage/cross-aarch64-linux-gnu/gcc-11.0.1_alpha20210321/work/gcc-11-20210321/gcc/cfgexpand.c:6729

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.4
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-03-25

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
OMP lowering makes the dead division conditional and we produce and want to
expand

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  MEM <vector(2) int> [(int *)&mask.4] = mask.5_9(D);
  vect__5.21_31 = .MASK_LOAD (&mask.4, 32B, { -1, -1, 0, 0, ... });
  _5 = mask.4[0];
  mask__26.22_33 = vect__5.21_31 != { 0, ... };
  _26 = _5 != 0;
  v_1 = .COND_DIV (_26, 1, 0, 1);
  vec_mask_and_37 = mask__26.22_33 & { -1, -1, 0, 0, ... };
  .MASK_STORE (&retval.3, 32B, vec_mask_and_37, { 0, ... });
  vectp_mask.19_29 = &mask.4 + POLY_INT_CST [16, 16];
  vectp_retval.23_36 = &retval.3 + POLY_INT_CST [16, 16];
  _6 = VIEW_CONVERT_EXPR<vector(2) int>(retval.3);
  return _6;
;;    succ:       EXIT

but that's still scalar, so sth forgot to "vectorize" it?  Indeed when
returning 'v' we instead get

  _26 = _5 != 0;
  vec_mask_and_37 = mask__26.34_33 & { -1, -1, -1, -1, 0, 0, 0, 0, ... };
  vect_v_1.35_38 = .COND_DIV (vec_mask_and_37, { 1, ... }, { 0, ... }, { 1, ...
});
  .MASK_STORE (&retval.9, 32B, vec_mask_and_37, vect_v_1.35_38);

The .COND_DIV is generated by if-conversion which supposedly expects we
will vectorize it (or throw it away) but it doesn't ignore (as the vectorizer)
obviously dead stmts.  So the bug is this disconnect between if-conversion
and vectorization.

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|ice-on-invalid-code         |ice-on-valid-code
           Priority|P3                          |P2

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Yeah, I don't see a problem on the omp simd cloning side, one could write such
code by hand too.  The thing is the DCE disabling, ifcvt adds .COND_DIV on the
to be vectorized loop copy only, but in the end the loop is vectorized but
.COND_DIV is not because it is dead and the vectorizer supposedly only
considers statements that are actually used.
So, either the vectorizer should do something for the dead statements in the
loop (whatever, including failing the vectorization, or vectorizing them too,
...), or ifcvt shouldn't add those for the dead statements, or we need to lower
those ifns back into scalar statements at the end of vectorization, or be able
to expand them even when scalar.

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
E.g. as
int a[1024], b[1024];

void
foo (void)
{
  #pragma omp simd
  for (int i = 0; i < 1024; i++)
    if (b[i] > 23) {
      a[i] = b[i] + 1;
      int v = 1 / 0;
    }
}
(omp simd is there only to convince it to vectorize it and not give up).

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Note for quite some of the failed-DCE PRs we could "fix up" at RTL expansion
time by teaching rewrite_out_of_ssa to DCE all zero-use defs where it already
does

  /* Eliminate PHIs which are of no use, such as virtual or dead phis.  */
  eliminate_useless_phis ();

for example by walking all SSA names and seeding a bitmap for
simple_dce_from_worklist.

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, we have already:
      /* Stub out scalar statements that must not survive vectorization.
         Doing this here helps with grouped statements, or statements that
         are involved in patterns.  */
      for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
           !gsi_end_p (gsi); gsi_next (&gsi))
        {
          gcall *call = dyn_cast <gcall *> (gsi_stmt (gsi));
          if (call && gimple_call_internal_p (call, IFN_MASK_LOAD))
            {
              tree lhs = gimple_get_lhs (call);
              if (!VECTOR_TYPE_P (TREE_TYPE (lhs)))
                {
                  tree zero = build_zero_cst (TREE_TYPE (lhs));
                  gimple *new_stmt = gimple_build_assign (lhs, zero);
                  gsi_replace (&gsi, new_stmt, true);
                }
            }
        }
in tree-vect-loop.c.
Perhaps we should handle similarly IFN_COND_* ?
Instead of using zero for those perhaps we could just use the last argument of
those calls (i.e. the ELSE part).

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

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 #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50604
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50604&action=edit
gcc11-pr99767.patch

Untested fix that does that.

[Bug target/99767] [9/10/11 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 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:b44ab138b7d4aaa5a9ee7956121ffc94777f6a42

commit r11-8211-gb44ab138b7d4aaa5a9ee7956121ffc94777f6a42
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 16 11:44:04 2021 +0200

    vectorizer: Remove dead scalar .COND_* calls from vectorized loops
[PR99767]

    The following testcase ICEs because disabling of DCE means there are dead
    stmts in the loop (though, in theory they could become dead only shortly
    before if-conv through some optimization), ifcvt which goes through all
    stmts in the loop if-converts them into .COND_DIV etc. internal fn calls
    in the copy of the loop meant for vectorization only, the loop is
    successfully vectorized but the particular .COND_* call is not because
    it isn't a live statement and the scalar .COND_* remains in the IL until
    expansion where it ICEs because these ifns only support vectors and not
    scalars.

    These ifns are similar to .MASK_{LOAD,STORE} in this behavior.

    One possible fix could be to expand scalar versions of them during
    expansion, basically undoing what if-conv did to create them, i.e.
    expand them as the lhs = else; if (mask) { lhs = statement; } or so.

    For .MASK_LOAD we have code to replace them in vect_transform_loop already
    though (not needed for .MASK_STORE, as stores should be always live
    and thus always vectorized), so this patch instead replaces .COND_*
    similarly to .MASK_LOAD in that loop, with the small difference
    that lhs = .MASK_LOAD (...); is replaced by lhs = 0; while
    lhs = .COND_* (..., else_arg); is replaced by lhs = else_arg.
    The statement must be dead, otherwise it would be vectorized, so I think
    it is not a big deal we don't turn it back into multiple basic blocks etc.
    (and it might be not possible to do that at that point).

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

            PR target/99767
            * tree-vect-loop.c (vect_transform_loop): Don't remove just
            dead scalar .MASK_LOAD calls, but also dead .COND_* calls - replace
            them by their last argument.

            * gcc.target/aarch64/pr99767.c: New test.

[Bug target/99767] [9/10 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10/11 Regression] ICE in |[9/10 Regression] ICE in
                   |expand_direct_optab_fn, at  |expand_direct_optab_fn, at
                   |internal-fn.c:3360          |internal-fn.c:3360

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

[Bug target/99767] [9/10 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[cvs-commit at gcc dot gnu.org]

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

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

commit r10-9729-g1730b5d6793127b1a47970f44d60da8082bab514
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 16 11:44:04 2021 +0200

    vectorizer: Remove dead scalar .COND_* calls from vectorized loops
[PR99767]

    The following testcase ICEs because disabling of DCE means there are dead
    stmts in the loop (though, in theory they could become dead only shortly
    before if-conv through some optimization), ifcvt which goes through all
    stmts in the loop if-converts them into .COND_DIV etc. internal fn calls
    in the copy of the loop meant for vectorization only, the loop is
    successfully vectorized but the particular .COND_* call is not because
    it isn't a live statement and the scalar .COND_* remains in the IL until
    expansion where it ICEs because these ifns only support vectors and not
    scalars.

    These ifns are similar to .MASK_{LOAD,STORE} in this behavior.

    One possible fix could be to expand scalar versions of them during
    expansion, basically undoing what if-conv did to create them, i.e.
    expand them as the lhs = else; if (mask) { lhs = statement; } or so.

    For .MASK_LOAD we have code to replace them in vect_transform_loop already
    though (not needed for .MASK_STORE, as stores should be always live
    and thus always vectorized), so this patch instead replaces .COND_*
    similarly to .MASK_LOAD in that loop, with the small difference
    that lhs = .MASK_LOAD (...); is replaced by lhs = 0; while
    lhs = .COND_* (..., else_arg); is replaced by lhs = else_arg.
    The statement must be dead, otherwise it would be vectorized, so I think
    it is not a big deal we don't turn it back into multiple basic blocks etc.
    (and it might be not possible to do that at that point).

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

            PR target/99767
            * tree-vect-loop.c (vect_transform_loop): Don't remove just
            dead scalar .MASK_LOAD calls, but also dead .COND_* calls - replace
            them by their last argument.

            * gcc.target/aarch64/pr99767.c: New test.

    (cherry picked from commit b44ab138b7d4aaa5a9ee7956121ffc94777f6a42)

[Bug target/99767] [9 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10 Regression] ICE in    |[9 Regression] ICE in
                   |expand_direct_optab_fn, at  |expand_direct_optab_fn, at
                   |internal-fn.c:3360          |internal-fn.c:3360

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

[Bug target/99767] [9 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[cvs-commit at gcc dot gnu.org]

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

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

commit r9-9448-g7cbe3b2fa21524dd2a3ba6d104f231ff88821622
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Apr 16 11:44:04 2021 +0200

    vectorizer: Remove dead scalar .COND_* calls from vectorized loops
[PR99767]

    The following testcase ICEs because disabling of DCE means there are dead
    stmts in the loop (though, in theory they could become dead only shortly
    before if-conv through some optimization), ifcvt which goes through all
    stmts in the loop if-converts them into .COND_DIV etc. internal fn calls
    in the copy of the loop meant for vectorization only, the loop is
    successfully vectorized but the particular .COND_* call is not because
    it isn't a live statement and the scalar .COND_* remains in the IL until
    expansion where it ICEs because these ifns only support vectors and not
    scalars.

    These ifns are similar to .MASK_{LOAD,STORE} in this behavior.

    One possible fix could be to expand scalar versions of them during
    expansion, basically undoing what if-conv did to create them, i.e.
    expand them as the lhs = else; if (mask) { lhs = statement; } or so.

    For .MASK_LOAD we have code to replace them in vect_transform_loop already
    though (not needed for .MASK_STORE, as stores should be always live
    and thus always vectorized), so this patch instead replaces .COND_*
    similarly to .MASK_LOAD in that loop, with the small difference
    that lhs = .MASK_LOAD (...); is replaced by lhs = 0; while
    lhs = .COND_* (..., else_arg); is replaced by lhs = else_arg.
    The statement must be dead, otherwise it would be vectorized, so I think
    it is not a big deal we don't turn it back into multiple basic blocks etc.
    (and it might be not possible to do that at that point).

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

            PR target/99767
            * tree-vect-loop.c (vect_transform_loop): Don't remove just
            dead scalar .MASK_LOAD calls, but also dead .COND_* calls - replace
            them by their last argument.

            * gcc.target/aarch64/pr99767.c: New test.

    (cherry picked from commit 1730b5d6793127b1a47970f44d60da8082bab514)

[Bug target/99767] [9 Regression] ICE in expand_direct_optab_fn, at internal-fn.c:3360

[jakub at gcc dot gnu.org]

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

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

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

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