Hi Juzhe,

just some small remarks, all in all no major concerns.

> +   vmv%m1r.v\t%0,%1"
> +  "&& (!register_operand (operands[0], <MODE>mode)
> +       || !register_operand (operands[1], <MODE>mode))"
> +  [(const_int 0)]
> +  {
> +    unsigned size = GET_MODE_BITSIZE (<MODE>mode).to_constant ();
> +    if (size <= MAX_BITS_PER_WORD

Any specific reason for MAX_BITS_PER_WORD instead of
GET_MODE_BITSIZE (Pmode)?  In general I like the idea to switch
to scalar moves here but couldn't it already be debatable for
a 64-bit move on rv32 i.e. a question of costing?

> +  gcc_assert (ok_p);
> +  DONE;

Here as well as before, why the assert?  Is this intended for
easier debugging later?

> +
> +(define_expand "@mov<VLS_AVL_REG:mode><P:mode>_lra"
> +  [(parallel
> +    [(set (match_operand:VLS_AVL_REG 0 "reg_or_mem_operand")
> +       (match_operand:VLS_AVL_REG 1 "reg_or_mem_operand"))
> +   (clobber (match_scratch:P 2))])]
> +  "TARGET_VECTOR && (lra_in_progress || reload_completed)"
> +{})
> +
> +(define_insn_and_split "*mov<VLS_AVL_REG:mode><P:mode>_lra"

These are the same as for the fractional modes but we define
the expanders for all modes instead.  legitimize_move will only
create the _lra if the mode is indeed smaller than a vector and
I assume the scratch will prevent us from ever generating the
insn any other way.  Still, couldn't we add the new modes to
V_FRACT?  Or didn't you want to mix VLA and VLS modes there?
The others seem to be mixed, though.
> +#define INCLUDE_ALGORITHM

What do we need this for?

> +      int inner_size = GET_MODE_BITSIZE (GET_MODE_INNER (mode));
> +      if (size < TARGET_MIN_VLEN)
> +     {
> +       int factor = TARGET_MIN_VLEN / size;
> +       if (inner_size == 8)
> +         factor = std::min (factor, 8);

Maybe for this?  Won't MIN suffice?

> +  ;; VLS modes.
> +  (V2QI "TARGET_VECTOR_VLS")

Why do these iterator modes have a condition on VLS while the following
ones don't?  It's probably not terribly important as it works either way
but still sticks out.

Btw. as a general remark.  In the past I also found the single-element
vectors helpful for codegen but that might be obsolete.  Not in scope
for this patch.

Regards
 Robin

Reply via email to