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

[Richard Henderson]

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~