[Bug tree-optimization/106787] New: ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648

[Bug tree-optimization/106787] New: ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648

[marxin at gcc dot gnu.org]

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

            Bug ID: 106787
           Summary: ICE in vect_schedule_slp_node, at
                    tree-vect-slp.cc:8648
           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: marxin at gcc dot gnu.org
                CC: rsandifo at gcc dot gnu.org
  Target Milestone: ---

Created attachment 53524
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53524&action=edit
gcda file

Seen in 511.povray_r benchmark when run with PGO:

$ g++ matrices.ii -Ofast -march=skylake -fprofile-use -c -w
during GIMPLE pass: slp
matrices.ii: In function 'void pov::MTimesB(double (*)[4], double (*)[4])':
matrices.ii:4:6: internal compiler error: in vect_schedule_slp_node, at
tree-vect-slp.cc:8648
    4 | void MTimesB(MATRIX matrix1, MATRIX result) {
      |      ^~~~~~~
0x9085a4 vect_schedule_slp_node
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:8648
0x155ce23 vect_schedule_scc
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:9018
0x155cb58 vect_schedule_scc
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:8999
0x155d450 vect_schedule_slp(vec_info*, vec<_slp_instance*, va_heap, vl_ptr>
const&)
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:9135
0x155e93f vect_slp_region
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:7387
0x155ff2c vect_slp_bbs
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:7499
0x15601b3 vect_slp_function(function*)
        /home/marxin/Programming/gcc/gcc/tree-vect-slp.cc:7587
0x15697d9 execute
        /home/marxin/Programming/gcc/gcc/tree-vectorizer.cc:1532
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.

$ cat matrices.ii
typedef double MATRIX[][4];
double MTimesB_t0, MTimesB_t1, MTimesB_t2, MTimesB_t3;
namespace pov {
void MTimesB(MATRIX matrix1, MATRIX result) {
  MTimesB_t0 = result[0][3];
  MTimesB_t1 = result[1][3];
  MTimesB_t2 = result[2][3];
  MTimesB_t3 = result[3][3];
  result[2][3] = matrix1[2][0] * MTimesB_t0 + matrix1[2][1] * MTimesB_t1 +
                 matrix1[2][2] * MTimesB_t2 + matrix1[2][3] * MTimesB_t3;
  result[3][3] = matrix1[3][0] * MTimesB_t0 + matrix1[3][1] * MTimesB_t1 +
                 matrix1[3][2] * MTimesB_t2 + matrix1[3][3] * MTimesB_t3;
}
} // namespace pov

[Bug tree-optimization/106787] [13 Regression] ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648 since r13-2288-g61c4c989034548f4

[marxin at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-08-31
     Ever confirmed|0                           |1
            Summary|ICE in                      |[13 Regression] ICE in
                   |vect_schedule_slp_node, at  |vect_schedule_slp_node, at
                   |tree-vect-slp.cc:8648       |tree-vect-slp.cc:8648 since
                   |                            |r13-2288-g61c4c989034548f4
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r13-2288-g61c4c989034548f4.

[Bug tree-optimization/106787] [13 Regression] ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648 since r13-2288-g61c4c989034548f4

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.0

[Bug tree-optimization/106787] [13 Regression] ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648 since r13-2288-g61c4c989034548f4

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #2 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/106787] [13 Regression] ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648 since r13-2288-g61c4c989034548f4

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:

https://gcc.gnu.org/g:eab511df13ca6abb24c3c2abb0f420a89c91e310

commit r13-2377-geab511df13ca6abb24c3c2abb0f420a89c91e310
Author: Richard Sandiford <richard.sandiford@arm.com>
Date:   Fri Sep 2 14:00:14 2022 +0100

    vect: Ensure SLP nodes don't end up in multiple BB partitions [PR106787]

    In the PR we have two REDUC_PLUS SLP instances that share a common
    load of stride 4.  Each instance also has a unique contiguous load.

    Initially all three loads are out of order, so have a nontrivial
    load permutation.  The layout pass puts them in order instead,
    For the two contiguous loads it is possible to do this by adjusting the
    SLP_LOAD_PERMUTATION to be { 0, 1, 2, 3 }.  But a SLP_LOAD_PERMUTATION
    of { 0, 4, 8, 12 } is rejected as unsupported, so the pass creates a
    separate VEC_PERM_EXPR instead.

    Later the 4-stride load's initial SLP_LOAD_PERMUTATION is rejected too,
    so that the load gets replaced by an external node built from scalars.
    We then have an external node feeding a VEC_PERM_EXPR.

    VEC_PERM_EXPRs created in this way do not have any associated
    SLP_TREE_SCALAR_STMTS.  This means that they do not affect the
    decision about which nodes should be in which subgraph for costing
    purposes.  If the VEC_PERM_EXPR is fed by a vect_external_def,
    then the VEC_PERM_EXPR's input doesn't affect that decision either.

    The net effect is that a shared VEC_PERM_EXPR fed by an external def
    can appear in more than one subgraph.  This triggered an ICE in
    vect_schedule_node, which (rightly) expects to be called no more
    than once for the same internal def.

    There seemed to be many possible fixes, including:

    (1) Replace unsupported loads with external defs *before* doing
        the layout optimisation.  This would avoid the need for the
        VEC_PERM_EXPR altogether.

    (2) If the target doesn't support a load in its original layout,
        stop the layout optimisation from checking whether the target
        supports loads in any new candidate layout.  In other words,
        treat all layouts as if they were supported whenever the
        original layout is not in fact supported.

        I'd rather not do this.  In principle, the layout optimisation
        could convert an unsupported layout to a supported one.
        Selectively ignoring target support would work against that.

        We could try to look specifically for loads that will need
        to be decomposed, but that just seems like admitting that
        things are happening in the wrong order.

    (3) Add SLP_TREE_SCALAR_STMTS to VEC_PERM_EXPRs.

        That would be OK for this case, but wouldn't be possible
        for external defs that represent existing vectors.

    (4) Make vect_schedule_slp share SCC info between subgraphs.

        It feels like that's working around the partitioning problem
        rather than a real fix though.

    (5) Directly ensure that internal def nodes belong to a single
        subgraph.

    (1) is probably the best long-term fix, but (5) is much simpler.
    The subgraph partitioning code already has a hash set to record
    which nodes have been visited; we just need to convert that to a
    map from nodes to instances instead.

    gcc/
            PR tree-optimization/106787
            * tree-vect-slp.cc (vect_map_to_instance): New function, split out
            from...
            (vect_bb_partition_graph_r): ...here.  Replace the visited set
            with a map from nodes to instances.  Ensure that a node only
            appears in one partition.
            (vect_bb_partition_graph): Update accordingly.

    gcc/testsuite/
            * gcc.dg/vect/bb-slp-layout-19.c: New test.

[Bug tree-optimization/106787] [13 Regression] ICE in vect_schedule_slp_node, at tree-vect-slp.cc:8648 since r13-2288-g61c4c989034548f4

[rsandifo at gcc dot gnu.org]

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

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

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

--- Comment #4 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed.