Re: [PATCH v7 10/42] target/arm: Implement the ADDG, SUBG instructions

[Peter Maydell]

On Thu, 18 Jun 2020 at 17:12, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 6/18/20 6:17 AM, Peter Maydell wrote:
> >> +        imm = (imm >> 6) << LOG2_TAG_GRANULE;
> ...
> >> +            TCGv_i32 tag_offset = tcg_const_i32(imm & 15);
> ...
> > 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().
>
> I'll do that, because...
>
> > But this patch is functionally correct...
>
> ... I've just noticed that it isn't correct.

Heh. I do think it will look nicer this way round.
Don't forget to tidy up disas_add_sub_imm(): its 'shift'
field will then be 1 bit, not 2.

thanks
-- PMM