Re: [PATCH 04/39] target/riscv: Add vclmulh.vv decoding, translation and execution support

[Philipp Tomsich]

On Thu, 2 Feb 2023 at 13:42, Lawrence Hunter
<lawrence.hunter@codethink.co.uk> wrote:
>
> Signed-off-by: Lawrence Hunter <lawrence.hunter@codethink.co.uk>
> ---
>  target/riscv/helper.h                      |  1 +
>  target/riscv/insn32.decode                 |  1 +
>  target/riscv/insn_trans/trans_rvzvkb.c.inc |  1 +
>  target/riscv/vcrypto_helper.c              | 12 ++++++++++++
>  4 files changed, 15 insertions(+)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 6c786ef6f3..a155272701 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -1140,3 +1140,4 @@ DEF_HELPER_FLAGS_3(sm4ks, TCG_CALL_NO_RWG_SE, tl, tl, 
> tl, tl)
>  /* Vector crypto functions */
>  DEF_HELPER_6(vclmul_vv, void, ptr, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_6(vclmul_vx, void, ptr, ptr, tl, ptr, env, i32)
> +DEF_HELPER_6(vclmulh_vv, void, ptr, ptr, ptr, ptr, env, i32)
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 4a7421354d..e26ea1df08 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -894,3 +894,4 @@ sm4ks       .. 11010 ..... ..... 000 ..... 0110011 @k_aes
>  # *** RV64 Zvkb vector crypto extension ***
>  vclmul_vv       001100 . ..... ..... 010 ..... 1010111 @r_vm
>  vclmul_vx       001100 . ..... ..... 110 ..... 1010111 @r_vm
> +vclmulh_vv      001101 . ..... ..... 010 ..... 1010111 @r_vm
> diff --git a/target/riscv/insn_trans/trans_rvzvkb.c.inc 
> b/target/riscv/insn_trans/trans_rvzvkb.c.inc
> index 6e8b81136c..19ce4c7431 100644
> --- a/target/riscv/insn_trans/trans_rvzvkb.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzvkb.c.inc
> @@ -39,6 +39,7 @@ static bool vclmul_vv_check(DisasContext *s, arg_rmrr *a)
>  }
>
>  GEN_VV_MASKED_TRANS(vclmul_vv, vclmul_vv_check)
> +GEN_VV_MASKED_TRANS(vclmulh_vv, vclmul_vv_check)
>
>  #define GEN_VX_MASKED_TRANS(NAME, CHECK)                                \
>  static bool trans_##NAME(DisasContext *s, arg_rmrr *a)                  \
> diff --git a/target/riscv/vcrypto_helper.c b/target/riscv/vcrypto_helper.c
> index c453d348ad..022b941131 100644
> --- a/target/riscv/vcrypto_helper.c
> +++ b/target/riscv/vcrypto_helper.c
> @@ -31,5 +31,17 @@ static void do_vclmul_vx(void *vd, target_long rs1, void 
> *vs2, int i)
>      ((uint64_t *)vd)[i] = result;
>  }
>
> +static void do_vclmulh_vv(void *vd, void *vs1, void *vs2, int i)
> +{
> +    __uint128_t result = 0;
> +    for (int j = 63; j >= 0; j--) {
> +        if ((((uint64_t *)vs1)[i] >> j) & 1) {
> +            result ^= (((__uint128_t)(((uint64_t *)vs2)[i])) << j);

Why are we computing a 128 bit result, if we reduce it to 64b bits anyway?

> +        }
> +    }
> +    ((uint64_t *)vd)[i] = (result >> 64);
> +}

Please simplify in the same way as for clmul (i.e. a single function
that computes uint64 x uint64 -> uint64 ... and 2 calls to the RVVCALL
generator for OPIVV2 and OPIVX2):

static uint64_t clmulh64(uint64_t x, uint64_t y)
{
    target_ulong result = 0;
    const unsigned int elem_width = 64;

    for (unsigned int i = 1; i < elem_width; ++i)
        if ((y >> i) & 1)
            result ^= (x >> (elem_width - i));

    return result;
}

/* vclmulh.vv */
RVVCALL(OPIVV2, vclmulh_vv_d, OP_UUU_D, H8, H8, H8, clmulh64)
GEN_VEXT_VV(vclmulh_vv_d, 8)

/* vclmulh.vx */
RVVCALL(OPIVX2, vclmulh_vx_d, OP_UUU_D, H8, H8, clmulh64)
GEN_VEXT_VX(vclmulh_vx_d, 8)


> +
>  GEN_VEXT_VV(vclmul_vv, 8)
>  GEN_VEXT_VX(vclmul_vx, 8)
> +GEN_VEXT_VV(vclmulh_vv, 8)
> --
> 2.39.1
>