Re: [PATCH 58/67] target/arm: Convert [US]CVTF (vector, fixed-point) scalar to decodetree

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

On 12/6/24 10:27, Peter Maydell wrote:
> On Sun, 1 Dec 2024 at 15:12, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Remove disas_simd_scalar_shift_imm as these were the
>> last insns decoded by that function.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/tcg/translate-a64.c | 47 ----------------------------------
>>   target/arm/tcg/a64.decode      |  8 ++++++
>>   2 files changed, 8 insertions(+), 47 deletions(-)
>>
>> diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
>> index 9808b976fd..ea178f85c2 100644
>> --- a/target/arm/tcg/translate-a64.c
>> +++ b/target/arm/tcg/translate-a64.c
>> @@ -9543,52 +9543,6 @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar,
>>       gen_restore_rmode(tcg_rmode, tcg_fpstatus);
>>   }
>>
>> -/* AdvSIMD scalar shift by immediate
>> - *  31 30  29 28         23 22  19 18  16 15    11  10 9    5 4    0
>> - * +-----+---+-------------+------+------+--------+---+------+------+
>> - * | 0 1 | U | 1 1 1 1 1 0 | immh | immb | opcode | 1 |  Rn  |  Rd  |
>> - * +-----+---+-------------+------+------+--------+---+------+------+
>> - *
>> - * This is the scalar version so it works on a fixed sized registers
>> - */
>> -static void disas_simd_scalar_shift_imm(DisasContext *s, uint32_t insn)
>> -{
>> -    int rd = extract32(insn, 0, 5);
>> -    int rn = extract32(insn, 5, 5);
>> -    int opcode = extract32(insn, 11, 5);
>> -    int immb = extract32(insn, 16, 3);
>> -    int immh = extract32(insn, 19, 4);
>> -    bool is_u = extract32(insn, 29, 1);
>> -
>> -    if (immh == 0) {
>> -        unallocated_encoding(s);
>> -        return;
>> -    }
>> -
>> -    switch (opcode) {
>> -    case 0x1c: /* SCVTF, UCVTF */
>> -        handle_simd_shift_intfp_conv(s, true, false, is_u, immh, immb,
>> -                                     opcode, rn, rd);
>> -        break;
>> -    default:
>> -    case 0x00: /* SSHR / USHR */
>> -    case 0x02: /* SSRA / USRA */
>> -    case 0x04: /* SRSHR / URSHR */
>> -    case 0x06: /* SRSRA / URSRA */
>> -    case 0x08: /* SRI */
>> -    case 0x0a: /* SHL / SLI */
>> -    case 0x0c: /* SQSHLU */
>> -    case 0x0e: /* SQSHL, UQSHL */
>> -    case 0x10: /* SQSHRUN */
>> -    case 0x11: /* SQRSHRUN */
>> -    case 0x12: /* SQSHRN, UQSHRN */
>> -    case 0x13: /* SQRSHRN, UQRSHRN */
>> -    case 0x1f: /* FCVTZS, FCVTZU */
>> -        unallocated_encoding(s);
>> -        break;
>> -    }
>> -}
>> -
>>   static void handle_2misc_64(DisasContext *s, int opcode, bool u,
>>                               TCGv_i64 tcg_rd, TCGv_i64 tcg_rn,
>>                               TCGv_i32 tcg_rmode, TCGv_ptr tcg_fpstatus)
>> @@ -10489,7 +10443,6 @@ static const AArch64DecodeTable data_proc_simd[] = {
>>       { 0x0e200800, 0x9f3e0c00, disas_simd_two_reg_misc },
>>       { 0x0f000400, 0x9f800400, disas_simd_shift_imm },
>>       { 0x5e200800, 0xdf3e0c00, disas_simd_scalar_two_reg_misc },
>> -    { 0x5f000400, 0xdf800400, disas_simd_scalar_shift_imm },
>>       { 0x0e780800, 0x8f7e0c00, disas_simd_two_reg_misc_fp16 },
>>       { 0x00000000, 0x00000000, NULL }
>>   };
>> diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
>> index 707715f433..197555555e 100644
>> --- a/target/arm/tcg/a64.decode
>> +++ b/target/arm/tcg/a64.decode
>> @@ -1699,6 +1699,14 @@ FCVTAU_f        0111 1110 0.1 00001 11001 0 ..... .....     @icvt_sd
>>   @fcvt_fixed_d   .... .... . 1 ...... ...... rn:5 rd:5       \
>>                   &fcvt sf=0 esz=3 shift=%fcvt_f_sh_d
>>
>> +SCVTF_f         0101 1111 0 ....... 111001 ..... .....      @fcvt_fixed_h
>> +SCVTF_f         0101 1111 0 ....... 111001 ..... .....      @fcvt_fixed_s
>> +SCVTF_f         0101 1111 0 ....... 111001 ..... .....      @fcvt_fixed_d
>> +
>> +UCVTF_f         0111 1111 0 ....... 111001 ..... .....      @fcvt_fixed_h
>> +UCVTF_f         0111 1111 0 ....... 111001 ..... .....      @fcvt_fixed_s
>> +UCVTF_f         0111 1111 0 ....... 111001 ..... .....      @fcvt_fixed_d
>> +
>>   FCVTZS_f        0101 1111 0 ....... 111111 ..... .....      @fcvt_fixed_h
>>   FCVTZS_f        0101 1111 0 ....... 111111 ..... .....      @fcvt_fixed_s
>>   FCVTZS_f        0101 1111 0 ....... 111111 ..... .....      @fcvt_fixed_d
> 
> Aren't we missing the new trans functions for these insns ?
No, we're sharing the trans functions with the previous patch.
The (vector, integer) insns decode with shift=0.
These (vector, fixed-point) insns decode a proper shift field.


r~