Re: [Qemu-devel] [PATCH v2 19/68] target/arm: Convert T32 ADDW/SUBW

[Peter Maydell <peter.maydell@linaro.org>]

On Mon, 19 Aug 2019 at 22:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate.c | 24 +++++++++++++-----------
>  target/arm/a32.decode  |  1 +
>  target/arm/t32.decode  | 19 +++++++++++++++++++
>  3 files changed, 33 insertions(+), 11 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index cb6296dc12..0e51289928 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -7626,6 +7626,11 @@ static void arm_skip_unless(DisasContext *s, uint32_t cond)
>   * Constant expanders for the decoders.
>   */
>
> +static int negate(DisasContext *s, int x)
> +{
> +    return -x;
> +}
> +
>  static int times_2(DisasContext *s, int x)
>  {
>      return x * 2;
> @@ -7975,6 +7980,12 @@ static bool trans_ORN_rri(DisasContext *s, arg_s_rri_rot *a)
>  #undef DO_ANY2
>  #undef DO_CMP2
>
> +static bool trans_ADR(DisasContext *s, arg_ri *a)
> +{
> +    store_reg_bx(s, a->rd, add_reg_for_lit(s, 15, a->imm));
> +    return true;
> +}
> +
>  /*
>   * Multiply and multiply accumulate
>   */
> @@ -10670,17 +10681,8 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
>                          }
>                          store_reg(s, rd, tmp);
>                      } else {
> -                        /* Add/sub 12-bit immediate.  */
> -                        if (insn & (1 << 23)) {
> -                            imm = -imm;
> -                        }
> -                        tmp = add_reg_for_lit(s, rn, imm);
> -                        if (rn == 13 && rd == 13) {
> -                            /* ADD SP, SP, imm or SUB SP, SP, imm */
> -                            store_sp_checked(s, tmp);
> -                        } else {
> -                            store_reg(s, rd, tmp);
> -                        }
> +                        /* Add/sub 12-bit immediate, in decodetree */
> +                        goto illegal_op;

We seem to have lost the store_sp_checked() handling ?

>                      }
>                  }
>              } else {
> diff --git a/target/arm/a32.decode b/target/arm/a32.decode
> index c7f156be6d..aac991664d 100644
> --- a/target/arm/a32.decode
> +++ b/target/arm/a32.decode
> @@ -30,6 +30,7 @@
>  &rrrr            rd rn rm ra
>  &rrr             rd rn rm
>  &rr              rd rm
> +&ri              rd imm
>  &r               rm
>  &i               imm
>  &msr_reg         rn r mask

Should this change be in some other patch ?

> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> index 5116c6165a..be4e5f087c 100644
> --- a/target/arm/t32.decode
> +++ b/target/arm/t32.decode
> @@ -27,6 +27,7 @@
>  &rrrr            !extern rd rn rm ra
>  &rrr             !extern rd rn rm
>  &rr              !extern rd rm
> +&ri              !extern rd imm
>  &r               !extern rm
>  &i               !extern imm
>  &msr_reg         !extern rn r mask
> @@ -121,6 +122,24 @@ SBC_rri          1111 0.0 1011 . .... 0 ... .... ........     @s_rri_rot
>  }
>  RSB_rri          1111 0.0 1110 . .... 0 ... .... ........     @s_rri_rot
>
> +# Data processing (plain binary immediate)
> +
> +%imm12_26_12_0   26:1 12:3 0:8
> +%neg12_26_12_0   26:1 12:3 0:8 !function=negate
> +@s0_rri_12       .... ... .... . rn:4 . ... rd:4 ........ \
> +                 &s_rri_rot imm=%imm12_26_12_0 rot=0 s=0
> +
> +{
> +  ADR            1111 0.1 0000 0 1111 0 ... rd:4 ........ \
> +                 &ri imm=%imm12_26_12_0
> +  ADD_rri        1111 0.1 0000 0 .... 0 ... .... ........     @s0_rri_12
> +}
> +{
> +  ADR            1111 0.1 0101 0 1111 0 ... rd:4 ........ \
> +                 &ri imm=%neg12_26_12_0
> +  SUB_rri        1111 0.1 0101 0 .... 0 ... .... ........     @s0_rri_12
> +}
> +
>  # Multiply and multiply accumulate
>
>  @s0_rnadm        .... .... .... rn:4 ra:4 rd:4 .... rm:4      &s_rrrr s=0
> --
> 2.17.1


thanks
-- PMM