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.
@@ -10881,13 +10921,11 @@ static void disas_simd_two_reg_misc_fp16(DisasContext
*s, uint32_t insn)
case 0x7b: /* FCVTZU */
gen_helper_advsimd_f16touinth(tcg_res, tcg_op, tcg_fpstatus);
break;
- case 0x6f: /* FNEG */
- tcg_gen_xori_i32(tcg_res, tcg_op, 0x8000);
- break;
case 0x7d: /* FRSQRTE */
gen_helper_rsqrte_f16(tcg_res, tcg_op, tcg_fpstatus);
break;
default:
+ case 0x6f: /* FNEG */
g_assert_not_reached();
}
What's this change about? This bit of decode is not in the area that
corresponds to the new patterns you add to a64.decode.
Is this a bug in our existing decode where we decode something that
should be undef? I think that 0x6f here corresponds to the decode
table in section C4.1.96.6 ("Advanced SIMD scalar two-register
miscellaneous FP16"), where it is U:a:opcode == 1 1 01111. But
in that table that encoding is marked unallocated. A similar
thing is true for case 0x2f FABS and case 07f FSQRT. All three
of these should have set only_in_vector to true and not had the
code handling in the is_scalar branch of the function.
We should fix that bug in a separate patch before we do the
decodetree conversion, I think.
You're right. I had thought there was something weird here, in that FNEG was present but
not FABS. The only scalar FNEG is in fp data-processing 1-src.
r~