On Fri, 26 Jul 2019 at 18:50, Richard Henderson <richard.hender...@linaro.org> wrote: > > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > target/arm/translate.c | 214 ++++++++++++++++++++++++----------------- > target/arm/a32.decode | 17 ++++ > target/arm/t32.decode | 19 ++++ > 3 files changed, 163 insertions(+), 87 deletions(-) > > diff --git a/target/arm/translate.c b/target/arm/translate.c > index ee7d53dfa5..354a52d36c 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -7376,21 +7376,6 @@ static void gen_storeq_reg(DisasContext *s, int rlow, > int rhigh, TCGv_i64 val) > store_reg(s, rhigh, tmp); > } > > -/* load a 32-bit value from a register and perform a 64-bit accumulate. */ > -static void gen_addq_lo(DisasContext *s, TCGv_i64 val, int rlow) > -{ > - TCGv_i64 tmp; > - TCGv_i32 tmp2; > - > - /* Load value and extend to 64 bits. */ > - tmp = tcg_temp_new_i64(); > - tmp2 = load_reg(s, rlow); > - tcg_gen_extu_i32_i64(tmp, tmp2); > - tcg_temp_free_i32(tmp2); > - tcg_gen_add_i64(val, val, tmp); > - tcg_temp_free_i64(tmp); > -} > -
> +static bool trans_UMAAL(DisasContext *s, arg_UMAAL *a) > +{ > + TCGv_i32 t0, t1, t2, zero; > + > + if (s->thumb > + ? !arm_dc_feature(s, ARM_FEATURE_THUMB_DSP) > + : !ENABLE_ARCH_6) { > + return false; > + } > + > + t0 = load_reg(s, a->rm); > + t1 = load_reg(s, a->rn); > + tcg_gen_mulu2_i32(t0, t1, t0, t1); > + zero = tcg_const_i32(0); > + t2 = load_reg(s, a->ra); > + tcg_gen_add2_i32(t0, t1, t0, t1, t2, zero); > + tcg_temp_free_i32(t2); > + t2 = load_reg(s, a->rd); > + tcg_gen_add2_i32(t0, t1, t0, t1, t2, zero); > + tcg_temp_free_i32(t2); > + tcg_temp_free_i32(zero); > + store_reg(s, a->ra, t0); > + store_reg(s, a->rd, t1); > + return true; > + Is using mulu2/add2/add2 like this really generating better code than the mulu_i64_i32 and 2 64-bit adds that we had before? If we're going to change how we're generating code it would be nice to at least mention it in the commit message... > @@ -10292,13 +10337,8 @@ static void disas_thumb2_insn(DisasContext *s, > uint32_t insn) > } > } > if (op & 4) { > - /* umaal */ > - if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DSP)) { > - tcg_temp_free_i64(tmp64); > - goto illegal_op; > - } > - gen_addq_lo(s, tmp64, rs); > - gen_addq_lo(s, tmp64, rd); > + /* umaal, in decodetree */ > + goto illegal_op; > } else if (op & 0x40) { > /* 64-bit accumulate. */ > gen_addq(s, tmp64, rs, rd); This doesn't seem to remove all of the Thumb insns we implement in the decode tree from the legacy decoder. Otherwise Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> thanks -- PMM