[Reposting to CC the RISC-V CI]

vect_slp_prefer_store_lanes_p allows an SLP tree to be split even
if the tree could use store-lanes, provided that one of the new
groups would operate on full vectors for each scalar iteration.
That heuristic is no longer firing for gcc.target/aarch64/pr99873_2.c.

The test contains:

void __attribute ((noipa))
foo (uint64_t *__restrict x, uint64_t *__restrict y, int n)
{
  for (int i = 0; i < n; i += 4)
    {
      x[i] += y[i];
      x[i + 1] += y[i + 1];
      x[i + 2] |= y[i + 2];
      x[i + 3] |= y[i + 3];
    }
}

and wants us to use V2DI for the first two elements and V2DI for
the second two elements, rather than LD4s and ST4s.  This gives:

.L3:
        ldp     q31, q0, [x0]
        add     w3, w3, 1
        ldp     q29, q30, [x1], 32
        orr     v30.16b, v0.16b, v30.16b
        add     v31.2d, v29.2d, v31.2d
        stp     q31, q30, [x0], 32
        cmp     w2, w3
        bhi     .L3

instead of:

.L4:
        ld4     {v28.2d - v31.2d}, [x2]
        ld4     {v24.2d - v27.2d}, [x3], 64
        add     v24.2d, v28.2d, v24.2d
        add     v25.2d, v29.2d, v25.2d
        orr     v26.16b, v30.16b, v26.16b
        orr     v27.16b, v31.16b, v27.16b
        st4     {v24.2d - v27.2d}, [x2], 64
        cmp     x2, x5
        bne     .L4

The first loop only handles half the amount of data per iteration,
but it requires far fewer internal permutations.

One reason the heuristic no longer fired looks like a typo: the call
to vect_slp_prefer_store_lanes_p was passing "1" as the new group size,
instead of "i".

However, even with that fixed, vect_analyze_slp later falls back on
single-lane SLP with load/store lanes.  I think that heuristic too
should use vect_slp_prefer_store_lanes_p (but it otherwise looks good).

The question is whether every load should pass
vect_slp_prefer_store_lanes_p or whether just one is enough.
I don't have an example that would make the call either way,
so I went for the latter, given that it's the smaller change
from the status quo.

This also appears to improve fotonik3d and roms from SPEC2017
(cross-checked against two different systems).

gcc/
        * tree-vect-slp.cc (vect_build_slp_instance): Pass the new group
        size (i) rather than 1 to vect_slp_prefer_store_lanes_p.
        (vect_analyze_slp): Only force the use of load-lanes and
        store-lanes if that is preferred for at least one load/store pair.
---
 gcc/tree-vect-slp.cc | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc
index 9e09f8e980b..ecb4a6521de 100644
--- a/gcc/tree-vect-slp.cc
+++ b/gcc/tree-vect-slp.cc
@@ -4161,7 +4161,7 @@ vect_build_slp_instance (vec_info *vinfo,
               && ! STMT_VINFO_SLP_VECT_ONLY (stmt_info)
               && compare_step_with_zero (vinfo, stmt_info) > 0
               && vect_slp_prefer_store_lanes_p (vinfo, stmt_info, NULL_TREE,
-                                                masked_p, group_size, 1));
+                                                masked_p, group_size, i));
          if (want_store_lanes || force_single_lane)
            i = 1;
 
@@ -5095,7 +5095,7 @@ vect_analyze_slp (vec_info *vinfo, unsigned max_tree_size,
          && !SLP_INSTANCE_TREE (instance)->ldst_lanes)
        {
          slp_tree slp_root = SLP_INSTANCE_TREE (instance);
-         int group_size = SLP_TREE_LANES (slp_root);
+         unsigned int group_size = SLP_TREE_LANES (slp_root);
          tree vectype = SLP_TREE_VECTYPE (slp_root);
 
          stmt_vec_info rep_info = SLP_TREE_REPRESENTATIVE (slp_root);
@@ -5138,6 +5138,7 @@ vect_analyze_slp (vec_info *vinfo, unsigned max_tree_size,
          if (loads_permuted)
            {
              bool can_use_lanes = true;
+             bool prefer_load_lanes = false;
              FOR_EACH_VEC_ELT (loads, j, load_node)
                if (STMT_VINFO_GROUPED_ACCESS
                      (SLP_TREE_REPRESENTATIVE (load_node)))
@@ -5165,9 +5166,21 @@ vect_analyze_slp (vec_info *vinfo, unsigned 
max_tree_size,
                        can_use_lanes = false;
                        break;
                      }
+                   /* Make sure that the target would prefer store-lanes
+                      for at least one of the loads.
+
+                      ??? Perhaps we should instead require this for
+                      all loads?  */
+                   prefer_load_lanes
+                     = (prefer_load_lanes
+                        || SLP_TREE_LANES (load_node) == group_size
+                        || (vect_slp_prefer_store_lanes_p
+                            (vinfo, stmt_vinfo,
+                             STMT_VINFO_VECTYPE (stmt_vinfo), masked,
+                             group_size, SLP_TREE_LANES (load_node))));
                  }
 
-             if (can_use_lanes)
+             if (can_use_lanes && prefer_load_lanes)
                {
                  if (dump_enabled_p ())
                    dump_printf_loc (MSG_NOTE, vect_location,
-- 
2.25.1

Reply via email to