[Bug tree-optimization/103029] [12 regression] gcc.dg/vect/pr82436.c ICEs on r12-4818

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
The gimple_lv_adjust_loop_header_phi comment isn't relevant but what is is the
association between PHI node argument and SLP node index from the analysis
phase,
in particular for backedges.  See vect_schedule_scc which does

  /* Now fixup the backedge def of the vectorized PHIs in this SCC.  */
  slp_tree phi_node;
  FOR_EACH_VEC_ELT (phis_to_fixup, i, phi_node)
    {
      gphi *phi = as_a <gphi *> (SLP_TREE_REPRESENTATIVE (phi_node)->stmt);
      edge_iterator ei;
      edge e;
      FOR_EACH_EDGE (e, ei, gimple_bb (phi)->preds)
        {
          unsigned dest_idx = e->dest_idx;
          child = SLP_TREE_CHILDREN (phi_node)[dest_idx];
          if (!child || SLP_TREE_DEF_TYPE (child) != vect_internal_def)
            continue;

but here

(gdb) p e
$14 = <edge 0x7ffff63c0690 (10 -> 9)>
(gdb) p e->dest_idx
$12 = 0
(gdb) p debug (phi_node)
pr82436.c:20:5: note: node 0x3448ba8 (max_nunits=2, refcnt=1)
pr82436.c:20:5: note: op template: w_lsm.7_73 = PHI <_58(10), 0.0(25)>
pr82436.c:20:5: note:   stmt 0 w_lsm.7_73 = PHI <_58(10), 0.0(25)>
pr82436.c:20:5: note:   stmt 1 y_lsm.6_74 = PHI <_61(10), 0.0(25)>
pr82436.c:20:5: note:   children (nil) 0x34487f0

so in fact the entry edge edge now has the backedge SLP node.

I think the former dance redirecting edges back and forth made the edge
order consistent but nothing really forced it the same order as the
original loop which could have either order as well.

In fact the old code seems to reliably put the entry edge first
(but we start with the entry edge second with the very first version)
but the new code manages to swap the edges on the _old_ loop
which I think is something we should try to avoid.

That happens when we loop_redirect_edge (latch_edge, nloop->header);

I suppose without making loop_version entirely SSA dependent we can
for the moment mostly ensure we end up with the same order but not
necessarily avoid changing the original IL order.

diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 8ed8c69b5b1..81b0dcb7006 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -9044,6 +9044,23 @@ gimple_lv_adjust_loop_header_phi (basic_block first,
basic_block second,
       def = PHI_ARG_DEF (phi2, e2->dest_idx);
       add_phi_arg (phi1, def, e, gimple_phi_arg_location_from_edge (phi2,
e2));
     }
+
+  /* If the edges into the old and new loop end up at different PHI arg
+     indices swap one by redirecting it to its own destination.  */
+  if (e->dest_idx != e2->dest_idx)
+    {
+      if (e->dest_idx == 0)
+       {
+         ssa_redirect_edge (e, e->dest);
+         flush_pending_stmts (e);
+       }
+      else
+       {
+         gcc_assert (e2->dest_idx == 0);
+         ssa_redirect_edge (e2, e2->dest);
+         flush_pending_stmts (e2);
+       }
+    }
 }

ensures this and fixes the testcase.  As said it would be desirable to
preserve the original loop PHI argument order.

A way to have the vectorizer less fragile would of course be also appreciated.

I am testing the above.