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

[Richard Henderson]

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.

I drop the low 6 bits of the 12-bit "imm" on the first line, and then try to
read the low 4 bits on the second line.

Oops.

r~