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

[Richard Henderson]

On 9/11/24 06:26, LIU Zhiwei wrote:
From: TANG Tiancheng <tangtiancheng....@alibaba-inc.com>
Signed-off-by: TANG Tiancheng <tangtiancheng....@alibaba-inc.com>
Reviewed-by: Liu Zhiwei <zhiwei_...@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~