Re: [Qemu-devel] [PATCH v2 08/68] target/arm: Convert Saturating addition and subtraction

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 8/19/19 11:36 PM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Nice :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/arm/translate.c | 75 +++++++++++++++++++++++++++---------------
>  target/arm/a32.decode  | 10 ++++++
>  target/arm/t32.decode  |  9 +++++
>  3 files changed, 67 insertions(+), 27 deletions(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 82bd207799..b731e08fe4 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8099,6 +8099,48 @@ static bool trans_UMAAL(DisasContext *s, arg_UMAAL *a)
>      return true;
>  }
>  
> +/*
> + * Saturating addition and subtraction
> + */
> +
> +static bool op_qaddsub(DisasContext *s, arg_rrr *a, bool add, bool doub)
> +{
> +    TCGv_i32 t0, t1;
> +
> +    if (s->thumb
> +        ? !arm_dc_feature(s, ARM_FEATURE_THUMB_DSP)
> +        : !ENABLE_ARCH_5TE) {
> +        return false;
> +    }
> +
> +    t0 = load_reg(s, a->rm);
> +    t1 = load_reg(s, a->rn);
> +    if (doub) {
> +        gen_helper_add_saturate(t1, cpu_env, t1, t1);
> +    }
> +    if (add) {
> +        gen_helper_add_saturate(t0, cpu_env, t0, t1);
> +    } else {
> +        gen_helper_sub_saturate(t0, cpu_env, t0, t1);
> +    }
> +    tcg_temp_free_i32(t1);
> +    store_reg(s, a->rd, t0);
> +    return true;
> +}
> +
> +#define DO_QADDSUB(NAME, ADD, DOUB) \
> +static bool trans_##NAME(DisasContext *s, arg_rrr *a)    \
> +{                                                        \
> +    return op_qaddsub(s, a, ADD, DOUB);                  \
> +}
> +
> +DO_QADDSUB(QADD, true, false)
> +DO_QADDSUB(QSUB, false, false)
> +DO_QADDSUB(QDADD, true, true)
> +DO_QADDSUB(QDSUB, false, true)
> +
> +#undef DO_QADDSUB
> +
>  /*
>   * Legacy decoder.
>   */
> @@ -8508,21 +8550,10 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
>              store_reg(s, rd, tmp);
>              break;
>          }
> -        case 0x5: /* saturating add/subtract */
> -            ARCH(5TE);
> -            rd = (insn >> 12) & 0xf;
> -            rn = (insn >> 16) & 0xf;
> -            tmp = load_reg(s, rm);
> -            tmp2 = load_reg(s, rn);
> -            if (op1 & 2)
> -                gen_helper_add_saturate(tmp2, cpu_env, tmp2, tmp2);
> -            if (op1 & 1)
> -                gen_helper_sub_saturate(tmp, cpu_env, tmp, tmp2);
> -            else
> -                gen_helper_add_saturate(tmp, cpu_env, tmp, tmp2);
> -            tcg_temp_free_i32(tmp2);
> -            store_reg(s, rd, tmp);
> -            break;
> +        case 0x5:
> +            /* Saturating addition and subtraction.  */
> +            /* All done in decodetree.  Reach here for illegal ops.  */
> +            goto illegal_op;
>          case 0x6: /* ERET */
>              if (op1 != 3) {
>                  goto illegal_op;
> @@ -9989,18 +10020,8 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
>              op = ((insn >> 17) & 0x38) | ((insn >> 4) & 7);
>              if (op < 4) {
>                  /* Saturating add/subtract.  */
> -                if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DSP)) {
> -                    goto illegal_op;
> -                }
> -                tmp = load_reg(s, rn);
> -                tmp2 = load_reg(s, rm);
> -                if (op & 1)
> -                    gen_helper_add_saturate(tmp, cpu_env, tmp, tmp);
> -                if (op & 2)
> -                    gen_helper_sub_saturate(tmp, cpu_env, tmp2, tmp);
> -                else
> -                    gen_helper_add_saturate(tmp, cpu_env, tmp, tmp2);
> -                tcg_temp_free_i32(tmp2);
> +                /* All done in decodetree.  Reach here for illegal ops.  */
> +                goto illegal_op;
>              } else {
>                  switch (op) {
>                  case 0x0a: /* rbit */
> diff --git a/target/arm/a32.decode b/target/arm/a32.decode
> index 87bbb2eec2..7791be5590 100644
> --- a/target/arm/a32.decode
> +++ b/target/arm/a32.decode
> @@ -27,6 +27,7 @@
>  &s_rri_rot       s rn rd imm rot
>  &s_rrrr          s rd rn rm ra
>  &rrrr            rd rn rm ra
> +&rrr             rd rn rm
>  
>  # Data-processing (register)
>  
> @@ -122,3 +123,12 @@ UMULL            .... 0000 100 . .... .... .... 1001 ....     @s_rdamn
>  UMLAL            .... 0000 101 . .... .... .... 1001 ....     @s_rdamn
>  SMULL            .... 0000 110 . .... .... .... 1001 ....     @s_rdamn
>  SMLAL            .... 0000 111 . .... .... .... 1001 ....     @s_rdamn
> +
> +# Saturating addition and subtraction
> +
> +@rndm            ---- .... .... rn:4 rd:4 .... .... rm:4      &rrr
> +
> +QADD             .... 0001 0000 .... .... 0000 0101 ....      @rndm
> +QSUB             .... 0001 0010 .... .... 0000 0101 ....      @rndm
> +QDADD            .... 0001 0100 .... .... 0000 0101 ....      @rndm
> +QDSUB            .... 0001 0110 .... .... 0000 0101 ....      @rndm
> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> index 40cc69aee3..7c6226e0af 100644
> --- a/target/arm/t32.decode
> +++ b/target/arm/t32.decode
> @@ -24,6 +24,7 @@
>  &s_rri_rot       !extern s rn rd imm rot
>  &s_rrrr          !extern s rd rn rm ra
>  &rrrr            !extern rd rn rm ra
> +&rrr             !extern rd rn rm
>  
>  # Data-processing (register)
>  
> @@ -117,6 +118,7 @@ RSB_rri          1111 0.0 1110 . .... 0 ... .... ........     @s_rri_rot
>  @s0_rnadm        .... .... .... rn:4 ra:4 rd:4 .... rm:4      &s_rrrr s=0
>  @s0_rn0dm        .... .... .... rn:4 .... rd:4 .... rm:4      &s_rrrr ra=0 s=0
>  @rnadm           .... .... .... rn:4 ra:4 rd:4 .... rm:4      &rrrr
> +@rndm            .... .... .... rn:4 .... rd:4 .... rm:4      &rrr
>  
>  {
>    MUL            1111 1011 0000 .... 1111 .... 0000 ....      @s0_rn0dm
> @@ -128,3 +130,10 @@ UMULL            1111 1011 1010 .... .... .... 0000 ....      @s0_rnadm
>  SMLAL            1111 1011 1100 .... .... .... 0000 ....      @s0_rnadm
>  UMLAL            1111 1011 1110 .... .... .... 0000 ....      @s0_rnadm
>  UMAAL            1111 1011 1110 .... .... .... 0110 ....      @rnadm
> +
> +# Data-processing (two source registers)
> +
> +QADD             1111 1010 1000 .... 1111 .... 1000 ....      @rndm
> +QSUB             1111 1010 1000 .... 1111 .... 1010 ....      @rndm
> +QDADD            1111 1010 1000 .... 1111 .... 1001 ....      @rndm
> +QDSUB            1111 1010 1000 .... 1111 .... 1011 ....      @rndm
>