Re: [PATCH 42/67] target/arm: Convert handle_rev to decodetree

[Peter Maydell]

On Sun, 1 Dec 2024 at 15:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> This includes REV16, REV32, REV64.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

> @@ -10070,10 +10003,6 @@ static void disas_simd_two_reg_misc(DisasContext *s, 
> uint32_t insn)
>      TCGv_ptr tcg_fpstatus;
>
>      switch (opcode) {
> -    case 0x0: /* REV64, REV32 */
> -    case 0x1: /* REV16 */
> -        handle_rev(s, opcode, u, is_q, size, rn, rd);
> -        return;
>      case 0x12: /* XTN, XTN2, SQXTUN, SQXTUN2 */
>      case 0x14: /* SQXTN, SQXTN2, UQXTN, UQXTN2 */
>          if (size == 3) {
> @@ -10276,6 +10205,8 @@ static void disas_simd_two_reg_misc(DisasContext *s, 
> uint32_t insn)
>          break;
>      }
>      default:
> +    case 0x0: /* REV64 */
> +    case 0x1: /* REV16, REV32 */

REV32 is case 0x0, not 0x1, per the comments deleted above.

>      case 0x3: /* SUQADD, USQADD */
>      case 0x4: /* CLS, CLZ */
>      case 0x5: /* CNT, NOT, RBIT */
> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
> index 4f8231d07a..2531809096 100644
> --- a/target/arm/tcg/a64.decode
> +++ b/target/arm/tcg/a64.decode
> @@ -73,6 +73,7 @@
>
>  @qrr_b          . q:1 ...... .. ...... ...... rn:5 rd:5  &qrr_e esz=0
>  @qrr_h          . q:1 ...... .. ...... ...... rn:5 rd:5  &qrr_e esz=1
> +@qrr_bh         . q:1 ...... . esz:1 ...... ...... rn:5 rd:5  &qrr_e
>  @qrr_e          . q:1 ...... esz:2 ...... ...... rn:5 rd:5  &qrr_e
>
>  @qrrr_b         . q:1 ...... ... rm:5 ...... rn:5 rd:5  &qrrr_e esz=0
> @@ -1657,3 +1658,7 @@ CMGE0_v         0.10 1110 ..1 00000 10001 0 ..... ..... 
>     @qrr_e
>  CMEQ0_v         0.00 1110 ..1 00000 10011 0 ..... .....     @qrr_e
>  CMLE0_v         0.10 1110 ..1 00000 10011 0 ..... .....     @qrr_e
>  CMLT0_v         0.00 1110 ..1 00000 10101 0 ..... .....     @qrr_e
> +
> +REV16_v         0.00 1110 001 00000 00011 0 ..... .....     @qrr_b
> +REV32_v         0.10 1110 0.1 00000 00011 0 ..... .....     @qrr_bh
> +REV64_v         0.00 1110 ..1 00000 00001 0 ..... .....     @qrr_e

This doesn't look quite right -- in the decode table in C4.1.96.21,
2-reg misc REV32 is opcode 00000, like REV64, not 00001 like REV16.

--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -1660,5 +1660,5 @@ CMLE0_v         0.10 1110 ..1 00000 10011 0
..... .....     @qrr_e
 CMLT0_v         0.00 1110 ..1 00000 10101 0 ..... .....     @qrr_e

 REV16_v         0.00 1110 001 00000 00011 0 ..... .....     @qrr_b
-REV32_v         0.10 1110 0.1 00000 00011 0 ..... .....     @qrr_bh
+REV32_v         0.10 1110 0.1 00000 00001 0 ..... .....     @qrr_bh
 REV64_v         0.00 1110 ..1 00000 00001 0 ..... .....     @qrr_e

should I think be the right fixup.

thanks
-- PMM