]> git.ipfire.org Git - thirdparty/gcc.git/commit
vect: Ensure SLP nodes don't end up in multiple BB partitions [PR106787]
authorRichard Sandiford <richard.sandiford@arm.com>
Fri, 2 Sep 2022 13:00:14 +0000 (14:00 +0100)
committerRichard Sandiford <richard.sandiford@arm.com>
Fri, 2 Sep 2022 13:00:14 +0000 (14:00 +0100)
commiteab511df13ca6abb24c3c2abb0f420a89c91e310
treeb4b1af4bb20cbdea497d90e5a584157e13d7c0fe
parent805c91843f2f8fc73d17a07d1545cc6ddcbd8935
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.
gcc/testsuite/gcc.dg/vect/bb-slp-layout-19.c [new file with mode: 0644]
gcc/tree-vect-slp.cc