[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 10/42] target/arm: Implement the ADDG, SUBG instructions
From: |
Peter Maydell |
Subject: |
Re: [PATCH v7 10/42] target/arm: Implement the ADDG, SUBG instructions |
Date: |
Thu, 18 Jun 2020 14:17:23 +0100 |
On Wed, 3 Jun 2020 at 02:13, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Shift offset in translate; use extract32.
> v6: Implement inline for !ATA.
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 2481561925..a18d71ad98 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -3753,17 +3753,20 @@ static void disas_pc_rel_adr(DisasContext *s,
> uint32_t insn)
> * sf: 0 -> 32bit, 1 -> 64bit
> * op: 0 -> add , 1 -> sub
> * S: 1 -> set flags
> - * shift: 00 -> LSL imm by 0, 01 -> LSL imm by 12
> + * shift: 00 -> LSL imm by 0,
> + * 01 -> LSL imm by 12
> + * 10 -> ADDG, SUBG
> */
> static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
> {
> int rd = extract32(insn, 0, 5);
> int rn = extract32(insn, 5, 5);
> - uint64_t imm = extract32(insn, 10, 12);
> + int imm = extract32(insn, 10, 12);
> int shift = extract32(insn, 22, 2);
> bool setflags = extract32(insn, 29, 1);
> bool sub_op = extract32(insn, 30, 1);
> bool is_64bit = extract32(insn, 31, 1);
> + bool is_tag = false;
>
> TCGv_i64 tcg_rn = cpu_reg_sp(s, rn);
> TCGv_i64 tcg_rd = setflags ? cpu_reg(s, rd) : cpu_reg_sp(s, rd);
> @@ -3775,11 +3778,40 @@ static void disas_add_sub_imm(DisasContext *s,
> uint32_t insn)
> case 0x1:
> imm <<= 12;
> break;
> + case 0x2:
> + /* ADDG, SUBG */
> + if (!is_64bit || setflags || (imm & 0x30) ||
> + !dc_isar_feature(aa64_mte_insn_reg, s)) {
> + goto do_unallocated;
> + }
> + is_tag = true;
> + break;
> default:
> + do_unallocated:
> unallocated_encoding(s);
> return;
> }
>
> + if (is_tag) {
> + imm = (imm >> 6) << LOG2_TAG_GRANULE;
> + if (sub_op) {
> + imm = -imm;
> + }
> +
> + if (s->ata) {
> + TCGv_i32 tag_offset = tcg_const_i32(imm & 15);
> + TCGv_i32 offset = tcg_const_i32(imm);
> +
> + gen_helper_addsubg(tcg_rd, cpu_env, tcg_rn, offset, tag_offset);
> + tcg_temp_free_i32(tag_offset);
> + tcg_temp_free_i32(offset);
> + } else {
> + tcg_gen_addi_i64(tcg_rd, tcg_rn, imm);
> + gen_address_with_allocation_tag0(tcg_rd, tcg_rd);
> + }
> + return;
> + }
Given that we don't really share any of the codegen with the
existing disas_add_sub_imm() insns, and the insn format isn't
the same (uimm6/op3/uimm4 rather than an imm12), I'm tempted
to suggest we should structure this the same way the Arm ARM
decode tables do, where "Add/subtract (immediate, with tags)"
is a separate subtable from "Add/subtract (immediate)": so
instead of disas_data_proc_imm() sending both case
0x22 and 0x23 to disas_add_sub_imm(), it would send 0x23
to a new disas_add_sub_tag().
But this patch is functionally correct, so if you don't
feel like making that change you can have
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
for this version.
thanks
-- PMM
- Re: [PATCH v7 05/42] target/arm: Rename DISAS_UPDATE to DISAS_UPDATE_EXIT, (continued)
- [PATCH v7 06/42] target/arm: Add DISAS_UPDATE_NOCHAIN, Richard Henderson, 2020/06/02
- [PATCH v7 07/42] target/arm: Add MTE system registers, Richard Henderson, 2020/06/02
- [PATCH v7 08/42] target/arm: Add MTE bits to tb_flags, Richard Henderson, 2020/06/02
- [PATCH v7 09/42] target/arm: Implement the IRG instruction, Richard Henderson, 2020/06/02
- [PATCH v7 10/42] target/arm: Implement the ADDG, SUBG instructions, Richard Henderson, 2020/06/02
- Re: [PATCH v7 10/42] target/arm: Implement the ADDG, SUBG instructions,
Peter Maydell <=
- [PATCH v7 11/42] target/arm: Implement the GMI instruction, Richard Henderson, 2020/06/02
- [PATCH v7 13/42] target/arm: Define arm_cpu_do_unaligned_access for user-only, Richard Henderson, 2020/06/02
[PATCH v7 12/42] target/arm: Implement the SUBP instruction, Richard Henderson, 2020/06/02