On Mon, 27 Nov 2023, Tamar Christina wrote:

> Ping
> 
> > -----Original Message-----
> > From: Tamar Christina <tamar.christ...@arm.com>
> > Sent: Monday, November 6, 2023 7:40 AM
> > To: gcc-patches@gcc.gnu.org
> > Cc: nd <n...@arm.com>; rguent...@suse.de; j...@ventanamicro.com
> > Subject: [PATCH 10/21]middle-end: implement relevancy analysis support for
> > control flow
> > 
> > Hi All,
> > 
> > This updates relevancy analysis to support marking gcond's belonging to 
> > early
> > breaks as relevant for vectorization.
> > 
> > Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> > 
> > Ok for master?
> > 
> > Thanks,
> > Tamar
> > 
> > gcc/ChangeLog:
> > 
> >     * tree-vect-stmts.cc (vect_stmt_relevant_p,
> >     vect_mark_stmts_to_be_vectorized, vect_analyze_stmt,
> > vect_is_simple_use,
> >     vect_get_vector_types_for_stmt): Support early breaks.
> > 
> > --- inline copy of patch --
> > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index
> > 4809b822632279493a843d402a833c9267bb315e..31474e923cc3feb2604
> > ca2882ecfb300cd211679 100644
> > --- a/gcc/tree-vect-stmts.cc
> > +++ b/gcc/tree-vect-stmts.cc
> > @@ -359,9 +359,14 @@ vect_stmt_relevant_p (stmt_vec_info stmt_info,
> > loop_vec_info loop_vinfo,
> >    *live_p = false;
> > 
> >    /* cond stmt other than loop exit cond.  */
> > -  if (is_ctrl_stmt (stmt_info->stmt)
> > -      && STMT_VINFO_TYPE (stmt_info) != loop_exit_ctrl_vec_info_type)
> > -    *relevant = vect_used_in_scope;
> > +  gimple *stmt = STMT_VINFO_STMT (stmt_info);
> > +  if (is_ctrl_stmt (stmt) && is_a <gcond *> (stmt))

is_ctrl_stmt (stmt) is redundant

> > +    {
> > +      gcond *cond = as_a <gcond *> (stmt);

in total better written as

       if (gcond *cond = dyn_cast <gcond *> (stmt))
         {

> > +      if (LOOP_VINFO_LOOP_CONDS (loop_vinfo).contains (cond)

linear search ...

> > +     && LOOP_VINFO_LOOP_IV_COND (loop_vinfo) != cond)
> > +   *relevant = vect_used_in_scope;

but why not simply mark all gconds as vect_used_in_scope?

> > +    }
> > 
> >    /* changing memory.  */
> >    if (gimple_code (stmt_info->stmt) != GIMPLE_PHI) @@ -374,6 +379,11 @@
> > vect_stmt_relevant_p (stmt_vec_info stmt_info, loop_vec_info loop_vinfo,
> >     *relevant = vect_used_in_scope;
> >        }
> > 
> > +  auto_vec<edge> exits = get_loop_exit_edges (loop);  auto_bitmap
> > + exit_bbs;  for (edge exit : exits)

is it your mail client messing patches up?  missing line-break
again.

> > +    bitmap_set_bit (exit_bbs, exit->dest->index);
> > +

you don't seem to use the bitmap?

> >    /* uses outside the loop.  */
> >    FOR_EACH_PHI_OR_STMT_DEF (def_p, stmt_info->stmt, op_iter,
> > SSA_OP_DEF)
> >      {
> > @@ -392,7 +402,6 @@ vect_stmt_relevant_p (stmt_vec_info stmt_info,
> > loop_vec_info loop_vinfo,
> >           /* We expect all such uses to be in the loop exit phis
> >              (because of loop closed form)   */
> >           gcc_assert (gimple_code (USE_STMT (use_p)) == GIMPLE_PHI);
> > -         gcc_assert (bb == single_exit (loop)->dest);
> > 
> >                *live_p = true;
> >         }
> > @@ -793,6 +802,20 @@ vect_mark_stmts_to_be_vectorized (loop_vec_info
> > loop_vinfo, bool *fatal)
> >                     return res;
> >                 }
> >                   }
> > +       }
> > +     else if (gcond *cond = dyn_cast <gcond *> (stmt_vinfo->stmt))
> > +       {
> > +         enum tree_code rhs_code = gimple_cond_code (cond);
> > +         gcc_assert (TREE_CODE_CLASS (rhs_code) == tcc_comparison);
> > +         opt_result res
> > +           = process_use (stmt_vinfo, gimple_cond_lhs (cond),
> > +                          loop_vinfo, relevant, &worklist, false);
> > +         if (!res)
> > +           return res;
> > +         res = process_use (stmt_vinfo, gimple_cond_rhs (cond),
> > +                           loop_vinfo, relevant, &worklist, false);
> > +         if (!res)
> > +           return res;
> >              }

I guess we're missing an

  else
    gcc_unreachable ();

to catch not handled stmt kinds (do we have gcond patterns yet?)

> >       else if (gcall *call = dyn_cast <gcall *> (stmt_vinfo->stmt))
> >         {
> > @@ -13043,11 +13066,15 @@ vect_analyze_stmt (vec_info *vinfo,
> >                          node_instance, cost_vec);
> >        if (!res)
> >     return res;
> > -   }
> > +    }
> > +
> > +  if (is_ctrl_stmt (stmt_info->stmt))
> > +    STMT_VINFO_DEF_TYPE (stmt_info) = vect_early_exit_def;

I think it should rather be vect_condition_def.  It's also not
this functions business to set STMT_VINFO_DEF_TYPE.  If we ever
get to handle not if-converted code (or BB vectorization of that)
then a gcond would define the mask stmts are under.

> >    switch (STMT_VINFO_DEF_TYPE (stmt_info))
> >      {
> >        case vect_internal_def:
> > +      case vect_early_exit_def:
> >          break;
> > 
> >        case vect_reduction_def:
> > @@ -13080,6 +13107,7 @@ vect_analyze_stmt (vec_info *vinfo,
> >      {
> >        gcall *call = dyn_cast <gcall *> (stmt_info->stmt);
> >        gcc_assert (STMT_VINFO_VECTYPE (stmt_info)
> > +             || gimple_code (stmt_info->stmt) == GIMPLE_COND
> >               || (call && gimple_call_lhs (call) == NULL_TREE));
> >        *need_to_vectorize = true;
> >      }
> > @@ -13835,6 +13863,14 @@ vect_is_simple_use (vec_info *vinfo,
> > stmt_vec_info stmt, slp_tree slp_node,
> >       else
> >         *op = gimple_op (ass, operand + 1);
> >     }
> > +      else if (gcond *cond = dyn_cast <gcond *> (stmt->stmt))
> > +   {
> > +     gimple_match_op m_op;
> > +     if (!gimple_extract_op (cond, &m_op))
> > +       return false;
> > +     gcc_assert (m_op.code.is_tree_code ());
> > +     *op = m_op.ops[operand];
> > +   }

Please do not use gimple_extract_op, use

  *op = gimple_op (cond, operand);

> >        else if (gcall *call = dyn_cast <gcall *> (stmt->stmt))
> >     *op = gimple_call_arg (call, operand);
> >        else
> > @@ -14445,6 +14481,8 @@ vect_get_vector_types_for_stmt (vec_info
> > *vinfo, stmt_vec_info stmt_info,
> >    *nunits_vectype_out = NULL_TREE;
> > 
> >    if (gimple_get_lhs (stmt) == NULL_TREE
> > +      /* Allow vector conditionals through here.  */
> > +      && !is_ctrl_stmt (stmt)

!is_a <gcond *> (stmt)

> >        /* MASK_STORE has no lhs, but is ok.  */
> >        && !gimple_call_internal_p (stmt, IFN_MASK_STORE))
> >      {
> > @@ -14461,7 +14499,7 @@ vect_get_vector_types_for_stmt (vec_info
> > *vinfo, stmt_vec_info stmt_info,
> >     }
> > 
> >        return opt_result::failure_at (stmt,
> > -                                "not vectorized: irregular stmt.%G", stmt);
> > +                                "not vectorized: irregular stmt: %G", 
> > stmt);
> >      }
> > 
> >    tree vectype;
> > @@ -14490,6 +14528,14 @@ vect_get_vector_types_for_stmt (vec_info
> > *vinfo, stmt_vec_info stmt_info,
> >     scalar_type = TREE_TYPE (DR_REF (dr));
> >        else if (gimple_call_internal_p (stmt, IFN_MASK_STORE))
> >     scalar_type = TREE_TYPE (gimple_call_arg (stmt, 3));
> > +      else if (is_ctrl_stmt (stmt))

else if (gcond *cond = dyn_cast <...>)

> > +   {
> > +     gcond *cond = dyn_cast <gcond *> (stmt);
> > +     if (!cond)
> > +       return opt_result::failure_at (stmt, "not vectorized: unsupported"
> > +                                      " control flow statement.\n");
> > +     scalar_type = TREE_TYPE (gimple_cond_rhs (stmt));

As said in the other patch STMT_VINFO_VECTYPE of the gcond should
be the _mask_ type the compare produces, not the vector type of
the inputs (the nunits_vectype might be that one though).
You possibly need to adjust vect_get_smallest_scalar_type for this.

Richard.

> > +   }
> >        else
> >     scalar_type = TREE_TYPE (gimple_get_lhs (stmt));
> > 
> > 
> > 
> > 
> > 
> > --
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to