Re: [PATCH V2] VECT: Support LEN_MASK_ LOAD/STORE to support flow control for length loop control

[Richard Biener <rguenther@suse.de>]

On Thu, 15 Jun 2023, Robin Dapp wrote:

> > the minus in 'operand 2 - operand 3' should be a plus if the
> > bias is really zero or -1.  I suppose
> 
> Yes, that somehow got lost from when the bias was still +1.  Maybe
> Juzhe can fix this in the course of his patch.
> 
> > that's quite conservative.  I think you can do better when the
> > loads are aligned, reading an extra byte when ignoring the bias
> > is OK and you at least know the very first element is used.
> > For stores you would need to emit compare&jump for all but
> > the first store of a group though ...
> 
> The implementation is a first shot and yes we could do a bit
> better but limiting to a single rgroup is IMHO the more severe
> restriction.  The pattern wasn't hit very often across SPEC
> either way.  I think overall proper masking is  more important for
> fixed-length vectors while length control might be more useful
> for variable-length vectors.  Just my gut feeling though, you're
> the expert there.
> 
> > That said, I'm still not seeing where you actually apply the bias.
> 
> We do
> 
> +
> +  int partial_load_bias = LOOP_VINFO_PARTIAL_LOAD_STORE_BIAS (loop_vinfo);
> +  if (partial_load_bias != 0)
> +    {
> +      tree adjusted_len = rgc->bias_adjusted_ctrl;
> +      gassign *minus = gimple_build_assign (adjusted_len, PLUS_EXPR,
> +                                           rgc->controls[0],
> +                                           build_int_cst
> +                                           (TREE_TYPE (rgc->controls[0]),
> +                                            partial_load_bias));
> +      gimple_seq_add_stmt (header_seq, minus);
> +    }
> +
> 
> as well as
> 
> +         if (use_bias_adjusted_len)
> +           {
> +             gcc_assert (i == 0);
> +             tree adjusted_len =
> +               make_temp_ssa_name (len_type, NULL, "adjusted_loop_len");
> +             SSA_NAME_DEF_STMT (adjusted_len) = gimple_build_nop ();
> +             rgl->bias_adjusted_ctrl = adjusted_len;
> +           }

Ah, OK.  It's a bit odd to have predicates on define_expand.  The
define_expand pattern is expected to only match either literal 0
or literal -1 (and consistently so for all len_ optabs) and thus
operand 2, the length, needs to be adjusted by the middle-end
to match up with the pattern supplied operand 3.

Richard.