Re: [PATCH v3 11/15] target/riscv: Add rev8 instruction, removing grev/grevi

[Richard Henderson]

On 8/23/21 9:40 AM, Philipp Tomsich wrote:
> +++ b/target/riscv/insn32.decode
> @@ -682,6 +682,7 @@ min        0000101 .......... 100 ..... 0110011 @r
>   minu       0000101 .......... 101 ..... 0110011 @r
>   orc_b      001010 000111 ..... 101 ..... 0010011 @r2
>   orn        0100000 .......... 110 ..... 0110011 @r
> +rev8       011010 011000 ..... 101 ..... 0010011 @r2

I think it would be clearer to call this version rev8_32.

> +# The encoding for rev8 differs between RV32 and RV64.
> +# When executing on RV64, the encoding used in RV32 is an illegal
> +# instruction, so we use different handler functions to differentiate.
> +rev8_64    011010 111000 ..... 101 ..... 0010011 @r2

... to match this.

> @@ -303,18 +293,11 @@ static bool trans_rolw(DisasContext *ctx, arg_rolw *a)
>       return gen_shiftw(ctx, a, gen_rolw);
>   }
>   
> -static bool trans_grevw(DisasContext *ctx, arg_grevw *a)
> +static bool trans_rev8_64(DisasContext *ctx, arg_rev8 *a)

And I think we should move the two trans_* functions to be sequential.

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


r~