Re: [PATCH 23/67] target/arm: Convert FMOV, FABS, FNEG (scalar) to decodetree

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

On 12/5/24 19:52, Richard Henderson wrote:
> On 12/5/24 15:12, Peter Maydell wrote:
>>> @@ -8295,15 +8356,6 @@ static void handle_fp_1src_half(DisasContext *s, int opcode, int 
>>> rd, int rn)
>>>       TCGv_i32 tcg_res = tcg_temp_new_i32();
>>>
>>>       switch (opcode) {
>>> -    case 0x0: /* FMOV */
>>> -        tcg_gen_mov_i32(tcg_res, tcg_op);
>>> -        break;
>>> -    case 0x1: /* FABS */
>>> -        gen_vfp_absh(tcg_res, tcg_op);
>>> -        break;
>>> -    case 0x2: /* FNEG */
>>> -        gen_vfp_negh(tcg_res, tcg_op);
>>> -        break;
>>>       case 0x3: /* FSQRT */
>>>           fpst = fpstatus_ptr(FPST_FPCR_F16);
>>>           gen_helper_sqrt_f16(tcg_res, tcg_op, fpst);
>>> @@ -8331,6 +8383,9 @@ static void handle_fp_1src_half(DisasContext *s, int opcode, int 
>>> rd, int rn)
>>>           gen_helper_advsimd_rinth(tcg_res, tcg_op, fpst);
>>>           break;
>>>       default:
>>> +    case 0x0: /* FMOV */
>>> +    case 0x1: /* FABS */
>>> +    case 0x2: /* FNEG */
>>>           g_assert_not_reached();
>>>       }
>>>
>>
>> In these changes to the "handle this op" functions we make the
>> function assert if it's passed an op we've converted. But shouldn't
>> there also be a change which makes the calling function disas_fp_1src()
>> call unallocated_encoding() for the ops ?
> 
> Yes.  I missed that because the line is
> 
>      case 0x0 ... 0x3:
> 
> without the usual set of comments.

The next two patches have the same problem.  All fixed now.

r~