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

[Richard Henderson <richard.henderson@linaro.org>]

On 12/3/24 05:49, Peter Maydell wrote:
> 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.

Yep, my bad.  I remember "fixing" the comment, because I thought it was off.  I must have 
been tired, or something.  Odd that this passes risu... maybe we're missing a pattern.


r~