Hi Lehua,

no concerns here, just tiny remarks but in general LGTM as is.

> +(define_insn_and_split "*copysign<mode>_neg"
> +  [(set (match_operand:VF 0 "register_operand")
> +        (neg:VF
> +          (unspec:VF [
> +            (match_operand:VF 1 "register_operand")
> +            (match_operand:VF 2 "register_operand")
> +          ] UNSPEC_VCOPYSIGN)))]
> +  "TARGET_VECTOR && can_create_pseudo_p ()"
> +  "#"
> +  "&& 1"
> +  [(const_int 0)]
> +{
> +  riscv_vector::emit_vlmax_insn (code_for_pred_ncopysign (<MODE>mode),
> +                                 riscv_vector::RVV_BINOP, operands);
> +  DONE;
> +})

It's a bit unfortunate that we need this now but well, no way around it.

> -  emit_insn (gen_vcond_mask (vmode, vmode, d->target, d->op0, d->op1, mask));
> +  /* swap op0 and op1 since the order is opposite to pred_merge.  */
> +  rtx ops2[] = {d->target, d->op1, d->op0, mask};
> +  emit_vlmax_merge_insn (code_for_pred_merge (vmode), 
> riscv_vector::RVV_MERGE_OP, ops2);
>    return true;
>  }

This seems a separate, general fix that just surfaced in the course of
this patch?  Would be nice to have this factored out but as we already have
it, no need I guess.

> +  if (is_dummy_mask)
> +    {
> +      /* Use TU, MASK ANY policy.  */
> +      if (needs_fp_rounding (code, mode))
> +     emit_nonvlmax_fp_tu_insn (icode, RVV_UNOP_TU, cond_ops, len);
> +      else
> +     emit_nonvlmax_tu_insn (icode, RVV_UNOP_TU, cond_ops, len);
> +    }

We have quite a bit of code duplication across the expand_cond_len functions
now (binop, ternop, unop).  Not particular to your patch but I'd suggest to
unify this later. 

> +TEST_ALL (DEF_LOOP)
> +
> +/* NOTE: int abs operator is converted to vmslt + vneg.v */
> +/* { dg-final { scan-assembler-times {\tvneg\.v\tv[0-9]+,v[0-9]+,v0\.t} 12 { 
> xfail { any-opts "--param riscv-autovec-lmul=m2" } } } } */

Why does this fail with LMUL == 2 (also in the following tests)?  A comment
would be nice here.

Regards
 Robin

Reply via email to