Re: [PATCH v6 06/14] target/riscv: rvk: add support for zknd/zkne extension in RV32

[Richard Henderson]

On 2/27/22 04:25, Weiwei Li wrote:
> +#define AES_SHIFROWS_LO(RS1, RS2) ( \
> +    (((RS1 >> 24) & 0xFF) << 56) | (((RS2 >> 48) & 0xFF) << 48) | \
> +    (((RS2 >> 8) & 0xFF) << 40) | (((RS1 >> 32) & 0xFF) << 32) | \
> +    (((RS2 >> 56) & 0xFF) << 24) | (((RS2 >> 16) & 0xFF) << 16) | \
> +    (((RS1 >> 40) & 0xFF) << 8) | (((RS1 >> 0) & 0xFF) << 0))
> +
> +#define AES_INVSHIFROWS_LO(RS1, RS2) ( \
> +    (((RS2 >> 24) & 0xFF) << 56) | (((RS2 >> 48) & 0xFF) << 48) | \
> +    (((RS1 >> 8) & 0xFF) << 40) | (((RS1 >> 32) & 0xFF) << 32) | \
> +    (((RS1 >> 56) & 0xFF) << 24) | (((RS2 >> 16) & 0xFF) << 16) | \
> +    (((RS2 >> 40) & 0xFF) << 8) | (((RS1 >> 0) & 0xFF) << 0))

Unused.  Whcih is good, because it shifts an unknown type by 56.


> +DEF_HELPER_3(aes32esmi, tl, tl, tl, tl)
> +DEF_HELPER_3(aes32esi, tl, tl, tl, tl)
> +DEF_HELPER_3(aes32dsmi, tl, tl, tl, tl)
> +DEF_HELPER_3(aes32dsi, tl, tl, tl, tl)

DEF_HELPER_FLAGS_3, with TCG_CALL_NO_RWG_SE.

> +static inline target_ulong aes32_operation(target_ulong bs, target_ulong rs1,
> +                                           target_ulong rs2, bool enc,
> +                                           bool mix)
> +{
> +    uint8_t shamt = bs << 3;

Just thinking it would be better to pass in shamt, since bs is otherwise unused.

> +    mixed = (mixed << shamt) | (mixed >> (32 - shamt));

This is broken for shamt == 0.  This is intending a rol32(), I assume?


> +static bool trans_aes32esmi(DisasContext *ctx, arg_aes32esmi *a)
> +{
> +    REQUIRE_ZKNE(ctx);
> +
> +    TCGv bs = tcg_const_tl(a->bs);
> +    TCGv dest = dest_gpr(ctx, a->rd);
> +    TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
> +    TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
> +
> +    gen_helper_aes32esmi(dest, src1, src2, bs);
> +    gen_set_gpr(ctx, a->rd, dest);
> +
> +    tcg_temp_free(bs);
> +    return true;
> +}
> +

tcg_constant_tl, which then need not be freed.
You should create a common helper for these 4 functions, so that you do not replicate so 
much code between them.


r~