Re: [Qemu-devel] [PATCH 09/17] target/arm: Implement the SUBP instruction

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:12, Richard Henderson
<address@hidden> wrote:
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate-a64.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 6583ad93b1..98ff60c161 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -5111,19 +5111,39 @@ static void handle_crc32(DisasContext *s,
>   */
>  static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
>  {
> -    unsigned int sf, rm, opcode, rn, rd;
> +    unsigned int sf, rm, opcode, rn, rd, setflag;
>      sf = extract32(insn, 31, 1);
> +    setflag = extract32(insn, 29, 1);
>      rm = extract32(insn, 16, 5);
>      opcode = extract32(insn, 10, 6);
>      rn = extract32(insn, 5, 5);
>      rd = extract32(insn, 0, 5);
>
> -    if (extract32(insn, 29, 1)) {
> +    if (setflag && opcode != 0) {
>          unallocated_encoding(s);
>          return;
>      }
>
>      switch (opcode) {
> +    case 0: /* SUBP(S) */
> +        if (sf == 0 || !dc_isar_feature(aa64_mte_insn_reg, s)) {
> +            goto do_unallocated;
> +        } else {
> +            TCGv_i64 tcg_n, tcg_m, tcg_d;
> +
> +            tcg_n = read_cpu_reg_sp(s, rn, true);
> +            tcg_m = read_cpu_reg_sp(s, rm, true);
> +            tcg_gen_sextract_i64(tcg_n, tcg_n, 0, 55);
> +            tcg_gen_sextract_i64(tcg_m, tcg_m, 0, 55);

Shouldn't the lengths here be 56, not 55 ? We're doing the
sign-extend of bits [55:0] to 64 bits.

> +            tcg_d = cpu_reg(s, rd);
> +
> +            if (setflag) {
> +                gen_sub_CC(true, tcg_d, tcg_n, tcg_m);
> +            } else {
> +                tcg_gen_sub_i64(tcg_d, tcg_n, tcg_m);
> +            }
> +        }
> +        break;
>      case 2: /* UDIV */
>          handle_div(s, false, sf, rm, rn, rd);
>          break;
> --
> 2.17.2

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

thanks
-- PMM