| Message ID | 20241201150607.12812-38-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: AArch64 decodetree conversion, final part | expand |
On Sun, 1 Dec 2024 at 15:10, Richard Henderson <richard.henderson@linaro.org> wrote: > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/tcg/translate-a64.c | 37 ++++++++++++++++------------------ > target/arm/tcg/a64.decode | 2 ++ > 2 files changed, 19 insertions(+), 20 deletions(-) > > diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c > index 4abc786cf6..312bf48020 100644 > --- a/target/arm/tcg/translate-a64.c > +++ b/target/arm/tcg/translate-a64.c > @@ -8920,6 +8920,20 @@ static bool do_gvec_fn2(DisasContext *s, arg_qrr_e *a, GVecGen2Fn *fn) > TRANS(ABS_v, do_gvec_fn2, a, tcg_gen_gvec_abs) > TRANS(NEG_v, do_gvec_fn2, a, tcg_gen_gvec_neg) > > +static bool do_gvec_fn2_bhs(DisasContext *s, arg_qrr_e *a, GVecGen2Fn *fn) > +{ > + if (a->esz == MO_64) { > + return false; > + } > + if (fp_access_check(s)) { > + gen_gvec_fn2(s, a->q, a->rd, a->rn, fn, a->esz); > + } > + return true; > +} > + > +TRANS(CLS_v, do_gvec_fn2_bhs, a, gen_gvec_cls) > +TRANS(CLZ_v, do_gvec_fn2_bhs, a, gen_gvec_clz) > + > /* Common vector code for handling integer to FP conversion */ > static void handle_simd_intfp_conv(DisasContext *s, int rd, int rn, > int elements, int is_signed, > @@ -9219,13 +9233,6 @@ static void handle_2misc_64(DisasContext *s, int opcode, bool u, > TCGCond cond; > > switch (opcode) { > - case 0x4: /* CLS, CLZ */ > - if (u) { > - tcg_gen_clzi_i64(tcg_rd, tcg_rn, 64); > - } else { > - tcg_gen_clrsb_i64(tcg_rd, tcg_rn); > - } > - break; > case 0x5: /* NOT */ This was dead code, right? We only call handle_2misc_64() for size == 3, but size == 3 is an unallocated encoding for "CLS/CLZ (vector)", which only deals with elements sizes up to 32 bits. Worth mentioning in the commit message, I think. (I was wondering why the new code doesn't have any cases for operating on 64-bit elements whereas this old code did seem to handle it.) > /* This opcode is shared with CNT and RBIT but we have earlier > * enforced that size == 3 if and only if this is the NOT insn. Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 12/6/24 08:40, Peter Maydell wrote: >> @@ -9219,13 +9233,6 @@ static void handle_2misc_64(DisasContext *s, int opcode, bool u, >> TCGCond cond; >> >> switch (opcode) { >> - case 0x4: /* CLS, CLZ */ >> - if (u) { >> - tcg_gen_clzi_i64(tcg_rd, tcg_rn, 64); >> - } else { >> - tcg_gen_clrsb_i64(tcg_rd, tcg_rn); >> - } >> - break; >> case 0x5: /* NOT */ > > This was dead code, right? We only call handle_2misc_64() > for size == 3, but size == 3 is an unallocated encoding for > "CLS/CLZ (vector)", which only deals with elements sizes up > to 32 bits. Worth mentioning in the commit message, I think. > > (I was wondering why the new code doesn't have any cases for > operating on 64-bit elements whereas this old code did seem > to handle it.) I had been wondering if I had failed to remove it earlier, but this has been dead code since the day it was added: b05c3068577. r~
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index 4abc786cf6..312bf48020 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -8920,6 +8920,20 @@ static bool do_gvec_fn2(DisasContext *s, arg_qrr_e *a, GVecGen2Fn *fn) TRANS(ABS_v, do_gvec_fn2, a, tcg_gen_gvec_abs) TRANS(NEG_v, do_gvec_fn2, a, tcg_gen_gvec_neg) +static bool do_gvec_fn2_bhs(DisasContext *s, arg_qrr_e *a, GVecGen2Fn *fn) +{ + if (a->esz == MO_64) { + return false; + } + if (fp_access_check(s)) { + gen_gvec_fn2(s, a->q, a->rd, a->rn, fn, a->esz); + } + return true; +} + +TRANS(CLS_v, do_gvec_fn2_bhs, a, gen_gvec_cls) +TRANS(CLZ_v, do_gvec_fn2_bhs, a, gen_gvec_clz) + /* Common vector code for handling integer to FP conversion */ static void handle_simd_intfp_conv(DisasContext *s, int rd, int rn, int elements, int is_signed, @@ -9219,13 +9233,6 @@ static void handle_2misc_64(DisasContext *s, int opcode, bool u, TCGCond cond; switch (opcode) { - case 0x4: /* CLS, CLZ */ - if (u) { - tcg_gen_clzi_i64(tcg_rd, tcg_rn, 64); - } else { - tcg_gen_clrsb_i64(tcg_rd, tcg_rn); - } - break; case 0x5: /* NOT */ /* This opcode is shared with CNT and RBIT but we have earlier * enforced that size == 3 if and only if this is the NOT insn. @@ -9287,6 +9294,7 @@ static void handle_2misc_64(DisasContext *s, int opcode, bool u, gen_helper_frint64_d(tcg_rd, tcg_rn, tcg_fpstatus); break; default: + case 0x4: /* CLS, CLZ */ case 0x7: /* SQABS, SQNEG */ case 0xb: /* ABS, NEG */ g_assert_not_reached(); @@ -10093,12 +10101,6 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn) handle_2misc_narrow(s, false, opcode, u, is_q, size, rn, rd); return; - case 0x4: /* CLS, CLZ */ - if (size == 3) { - unallocated_encoding(s); - return; - } - break; case 0x2: /* SADDLP, UADDLP */ case 0x6: /* SADALP, UADALP */ if (size == 3) { @@ -10303,6 +10305,7 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn) } default: case 0x3: /* SUQADD, USQADD */ + case 0x4: /* CLS, CLZ */ case 0x7: /* SQABS, SQNEG */ case 0xb: /* ABS, NEG */ unallocated_encoding(s); @@ -10325,13 +10328,6 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn) } switch (opcode) { - case 0x4: /* CLZ, CLS */ - if (u) { - gen_gvec_fn2(s, is_q, rd, rn, gen_gvec_clz, size); - } else { - gen_gvec_fn2(s, is_q, rd, rn, gen_gvec_cls, size); - } - return; case 0x5: if (u && size == 0) { /* NOT */ gen_gvec_fn2(s, is_q, rd, rn, tcg_gen_gvec_not, 0); @@ -10355,6 +10351,7 @@ static void disas_simd_two_reg_misc(DisasContext *s, uint32_t insn) case 0xa: /* CMLT */ gen_gvec_fn2(s, is_q, rd, rn, gen_gvec_clt0, size); return; + case 0x4: /* CLZ, CLS */ case 0xb: g_assert_not_reached(); } diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode index 30cd05030a..c3acb5dc37 100644 --- a/target/arm/tcg/a64.decode +++ b/target/arm/tcg/a64.decode @@ -1641,3 +1641,5 @@ SQABS_v 0.00 1110 ..1 00000 01111 0 ..... ..... @qrr_e SQNEG_v 0.10 1110 ..1 00000 01111 0 ..... ..... @qrr_e ABS_v 0.00 1110 ..1 00000 10111 0 ..... ..... @qrr_e NEG_v 0.10 1110 ..1 00000 10111 0 ..... ..... @qrr_e +CLS_v 0.00 1110 ..1 00000 01001 0 ..... ..... @qrr_e +CLZ_v 0.10 1110 ..1 00000 01001 0 ..... ..... @qrr_e
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/tcg/translate-a64.c | 37 ++++++++++++++++------------------ target/arm/tcg/a64.decode | 2 ++ 2 files changed, 19 insertions(+), 20 deletions(-)