On Tue, 13 Aug 2024, Jeff Law wrote:
>
>
> On 8/11/24 3:00 PM, Robin Dapp wrote:
> > When predicating a load we implicitly assume that the else value is
> > zero. In order to formalize this this patch queries the target for
> > its supported else operand and uses that for the maskload call.
> > Subsequently, if the else operand is nonzero, a cond_expr enforcing
> > a zero else value is emitted.
Why? I don't think the vectorizer relies on a particular else
value? I'd say it would be appropriate for if-conversion to
use "ANY" and for the vectorizer to then pick a supported
version and/or enforce the else value it needs via a blend?
Richard.
> > gcc/ChangeLog:
> >
> > * tree-if-conv.cc (predicate_load_or_store): Enforce zero else
> > value.
> > (predicate_statements): Use sequence instead of statement.
> > ---
> > gcc/tree-if-conv.cc | 78 +++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 62 insertions(+), 16 deletions(-)
> >
> > diff --git a/gcc/tree-if-conv.cc b/gcc/tree-if-conv.cc
> > index 57992b6deca..54cb9ef0ef1 100644
> > --- a/gcc/tree-if-conv.cc
> > +++ b/gcc/tree-if-conv.cc
> > @@ -2452,10 +2452,12 @@ mask_exists (int size, const vec<int> &vec)
> > write and it needs to be predicated by MASK. Return a statement
> > that does so. */
> > -static gimple *
> > -predicate_load_or_store (gimple_stmt_iterator *gsi, gassign *stmt, tree
> > mask)
> > +static gimple_seq
> > +predicate_load_or_store (gimple_stmt_iterator *gsi, gassign *stmt, tree
> > mask,
> > + hash_set<tree_ssa_name_hash> *ssa_names)
> Need an update to the function comment for the new SSA_NAMES argument.
>
>
> > @@ -2789,11 +2829,17 @@ predicate_statements (loop_p loop)
> > vect_masks.safe_push (mask);
> > }
> > if (gimple_assign_single_p (stmt))
> > - new_stmt = predicate_load_or_store (&gsi, stmt, mask);
> > - else
> > - new_stmt = predicate_rhs_code (stmt, mask, cond, &ssa_names);
> > + {
> > + gimple_seq call_seq
> > + = predicate_load_or_store (&gsi, stmt, mask, &ssa_names);
> > - gsi_replace (&gsi, new_stmt, true);
> > + gsi_replace_with_seq (&gsi, call_seq, true);
> > + }
> > + else
> > + {
> > + new_stmt = predicate_rhs_code (stmt, mask, cond,
> > &ssa_names);
> > + gsi_replace (&gsi, new_stmt, true);
> > + }
> Consider moving NEW_STMT's declaration into the scope where it's only set/use
> occur.
>
>
> Overall it looks quite reasonable to me. OK with the nits fixed.
>
> jeff
>
>
--
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)
