Re: [PATCH v3 12/15] target/riscv: Add zext.h instructions to Zbb, removing pack/packu/packh

[Richard Henderson]

On 8/23/21 9:40 AM, Philipp Tomsich wrote:
> The 1.0.0 version of Zbb does not contain pack/packu/packh. However, a
> zext.h instruction is provided (built on pack/packh from pre-0.93
> draft-B) is available.
>
> This commit adds zext.h and removes the pack* instructions.
>
> Note that the encodings for zext.h are different between RV32 and
> RV64, which is handled through REQUIRE_32BIT.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>
> Changes in v3:
> - Moved zext.h-addition & pack*-removal to a separate commit.
>
>   target/riscv/insn32.decode              | 10 +++---
>   target/riscv/insn_trans/trans_rvb.c.inc | 45 +++++++------------------
>   target/riscv/translate.c                | 40 ----------------------
>   3 files changed, 18 insertions(+), 77 deletions(-)
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 72b73c6df2..0fd5afc289 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -689,6 +689,7 @@ rori       01100 ............ 101 ..... 0010011 @sh
>   sext_b     011000 000100 ..... 001 ..... 0010011 @r2
>   sext_h     011000 000101 ..... 001 ..... 0010011 @r2
>   xnor       0100000 .......... 100 ..... 0110011 @r
> +zext_h     0000100 00000 ..... 100 ..... 0110011 @r2

Similarly, I think this should be zext_h_32, and

> @@ -225,6 +207,12 @@ static bool trans_orc_b(DisasContext *ctx, arg_orc_b *a)
>       return gen_unary(ctx, a, &gen_orc_b);
>   }
>   
> +static bool trans_zext_h(DisasContext *ctx, arg_sext_h *a)
> +{
> +    REQUIRE_32BIT(ctx);
> +    REQUIRE_ZBB(ctx);
> +    return gen_unary(ctx, a, &tcg_gen_ext16u_tl);
> +}
>   
>   #define GEN_TRANS_SHADD(SHAMT)                                             \
>   static bool trans_sh##SHAMT##add(DisasContext *ctx, arg_sh##SHAMT##add *a) \
...
> @@ -348,6 +322,13 @@ static bool trans_slli_uw(DisasContext *ctx, arg_slli_uw 
> *a)
>       return true;
>   }
>   
> +static bool trans_zext_h_64(DisasContext *ctx, arg_sext_h *a)
> +{
> +    REQUIRE_64BIT(ctx);
> +    REQUIRE_ZBB(ctx);
> +    return gen_unary(ctx, a, &tcg_gen_ext16u_tl);
> +}
> +

... the two functions placed together.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~