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.
