On Mon, 4 Jul 2022, Richard Biener wrote:

> The following removes a FIXME where we fail(ed) to keep virtual
> SSA up-to-date, patching up the remaining two cases I managed to
> trigger.  I've left an assert so that we pick up cases arising
> for the cases I wasn't able to trigger.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu, I've also
> built SPEC CPU 2017 with and without LTO and zen2 arch.
> 
> I eventually do expect fallout, so sorry for that in advance.

While I read the comment following the new assert multiple times
only after pushing I realized this will actually break load-lanes.
I'm looking for a testcase that hits it right now but I'll followup
with a partial revert for targets supporting load_lanes.

Richard.

> Pushed to trunk.
> 
> Richard.
> 
> 2022-07-04  Richard Biener  <rguent...@suse.de>
> 
>       * tree-vect-loop-manip.cc (vect_do_peeling): Assert that
>       no SSA update is needed instead of updating virtual SSA
>       form.
>       * tree-vect-stmts.cc (vectorizable_load): For hoisted
>       invariant load use the loop entry virtual use.
>       For emulated gather loads use the virtual use of the
>       original stmt like vect_finish_stmt_generation would do.
> ---
>  gcc/tree-vect-loop-manip.cc | 11 ++++-------
>  gcc/tree-vect-stmts.cc      | 15 ++++++++++++---
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/gcc/tree-vect-loop-manip.cc b/gcc/tree-vect-loop-manip.cc
> index ae5533e0f68..81e29d564d6 100644
> --- a/gcc/tree-vect-loop-manip.cc
> +++ b/gcc/tree-vect-loop-manip.cc
> @@ -2683,14 +2683,11 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree 
> niters, tree nitersm1,
>    class loop *first_loop = loop;
>    bool irred_flag = loop_preheader_edge (loop)->flags & 
> EDGE_IRREDUCIBLE_LOOP;
>  
> -  /* We might have a queued need to update virtual SSA form.  As we
> -     delete the update SSA machinery below after doing a regular
> +  /* Historically we might have a queued need to update virtual SSA form.
> +     As we delete the update SSA machinery below after doing a regular
>       incremental SSA update during loop copying make sure we don't
> -     lose that fact.
> -     ???  Needing to update virtual SSA form by renaming is unfortunate
> -     but not all of the vectorizer code inserting new loads / stores
> -     properly assigns virtual operands to those statements.  */
> -  update_ssa (TODO_update_ssa_only_virtuals);
> +     lose that fact.  */
> +  gcc_assert (!need_ssa_update_p (cfun));
>  
>    create_lcssa_for_virtual_phi (loop);
>  
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 346d8ce2804..d6a6fe3fb38 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -9024,9 +9024,16 @@ vectorizable_load (vec_info *vinfo,
>                            "hoisting out of the vectorized loop: %G", stmt);
>         scalar_dest = copy_ssa_name (scalar_dest);
>         tree rhs = unshare_expr (gimple_assign_rhs1 (stmt));
> -       gsi_insert_on_edge_immediate
> -         (loop_preheader_edge (loop),
> -          gimple_build_assign (scalar_dest, rhs));
> +       edge pe = loop_preheader_edge (loop);
> +       gphi *vphi = get_virtual_phi (loop->header);
> +       tree vuse;
> +       if (vphi)
> +         vuse = PHI_ARG_DEF_FROM_EDGE (vphi, pe);
> +       else
> +         vuse = gimple_vuse (gsi_stmt (*gsi));
> +       gimple *new_stmt = gimple_build_assign (scalar_dest, rhs);
> +       gimple_set_vuse (new_stmt, vuse);
> +       gsi_insert_on_edge_immediate (pe, new_stmt);
>       }
>        /* These copies are all equivalent, but currently the representation
>        requires a separate STMT_VINFO_VEC_STMT for each one.  */
> @@ -9769,6 +9776,8 @@ vectorizable_load (vec_info *vinfo,
>                           tree ref = build2 (MEM_REF, ltype, ptr,
>                                              build_int_cst (ref_type, 0));
>                           new_stmt = gimple_build_assign (elt, ref);
> +                         gimple_set_vuse (new_stmt,
> +                                          gimple_vuse (gsi_stmt (*gsi)));
>                           gimple_seq_add_stmt (&stmts, new_stmt);
>                           CONSTRUCTOR_APPEND_ELT (ctor_elts, NULL_TREE, elt);
>                         }
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstra

Reply via email to