Re: [Qemu-devel] [PATCH v2 66/68] target/arm: Convert T16, long branches

[Peter Maydell]

On Mon, 19 Aug 2019 at 22:39, Richard Henderson
<address@hidden> wrote:
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate.c | 89 +++++++++++++++++++-----------------------
>  target/arm/t16.decode  |  3 ++
>  2 files changed, 43 insertions(+), 49 deletions(-)

> +static bool trans_BLX_suffix(DisasContext *s, arg_BLX_suffix *a)
> +{
> +    TCGv_i32 tmp;
> +
> +    assert(!arm_dc_feature(s, ARM_FEATURE_THUMB2));
> +    if (!ENABLE_ARCH_5) {
> +        return false;
> +    }
> +    tmp = tcg_temp_new_i32();
> +    tcg_gen_addi_i32(tmp, cpu_R[14], a->imm << 1);
> +    tcg_gen_andi_i32(tmp, tmp, -4);

Minor nit, but can we use 0xfffffffc like the old code did,
to avoid the reader having to do 2s-complement arithmetic
in their head to figure out that we're clearing the low 2 bits?

> +    tcg_gen_movi_i32(cpu_R[14], s->base.pc_next | 1);
> +    gen_bx(s, tmp);
> +    return true;
> +}

> diff --git a/target/arm/t16.decode b/target/arm/t16.decode
> index 35a5b03118..5ee8457efb 100644
> --- a/target/arm/t16.decode
> +++ b/target/arm/t16.decode
> @@ -274,3 +274,6 @@ LDM_t16         1011 110 ......... \
>  %imm11_0x2      0:s11 !function=times_2
>
>  B               11100 ...........               &i imm=%imm11_0x2

This would be a good place to put a comment equivalent to that
in the old decoder:

# thumb_insn_is_16bit() ensures we won't be decoding these as
# T16 instructions for a Thumb2 CPU, so these patterns must be
# a Thumb1 split BL/BLX.

> +BLX_suffix      11101 imm:11                    &i
> +BL_BLX_prefix   11110 imm:s11                   &i
> +BL_suffix       11111 imm:11                    &i

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM