Re: [PATCH v4 11/12] tcg/riscv: Implement vector roti/v/x ops

[Richard Henderson <richard.henderson@linaro.org>]

On 9/11/24 06:26, LIU Zhiwei wrote:
> From: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> 
> Signed-off-by: TANG Tiancheng <tangtiancheng.ttc@alibaba-inc.com>
> Reviewed-by: Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
> ---
>   tcg/riscv/tcg-target.c.inc | 35 +++++++++++++++++++++++++++++++++++
>   tcg/riscv/tcg-target.h     |  6 +++---
>   2 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/tcg/riscv/tcg-target.c.inc b/tcg/riscv/tcg-target.c.inc
> index 16785ebe8e..afc9747780 100644
> --- a/tcg/riscv/tcg-target.c.inc
> +++ b/tcg/riscv/tcg-target.c.inc
> @@ -2494,6 +2494,33 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
>           set_vtype_len_sew(s, type, vece);
>           tcg_out_vshifti(s, OPC_VSRA_VI, OPC_VSRA_VX, a0, a1, a2);
>           break;
> +    case INDEX_op_rotli_vec:
> +        set_vtype_len_sew(s, type, vece);
> +        tcg_out_vshifti(s, OPC_VSLL_VI, OPC_VSLL_VX, TCG_REG_V0, a1, a2);
> +        tcg_out_vshifti(s, OPC_VSRL_VI, OPC_VSRL_VX, a0, a1, -a2);

You will want to mask -a2, because otherwise it will always fail to match imm < 32 within 
tcg_out_vshifti:

     -a2 & ((8 << vece) - 1)

> +    case INDEX_op_rotlv_vec:
> +        set_vtype_len_sew(s, type, vece);
> +        tcg_out_opc_vv(s, OPC_VSLL_VV, TCG_REG_V0, a1, a2, true);
> +        tcg_out_opc_vi(s, OPC_VRSUB_VI, TCG_REG_V0, a2, 0, true);
> +        tcg_out_opc_vv(s, OPC_VSRL_VV, a0, a1, TCG_REG_V0, true);

You have written to V0 twice, clobbering the result.
Need to swap the shifts:

	vrsub.vi	v0, a2, 0
	vsrl.vv		v0, a1, v0
	vsll.vv		a0, a1, a2
	vor.vv		a0, a0, v0


r~