[v3,4/6] Implement support for recording extension register ld/st insn
Commit Message
gdb:
2014-08-13 Omair Javaid <omair.javaid@linaro.org>
* arm-tdep.c (arm_record_asimd_vfp_coproc): Updated.
(arm_record_exreg_ld_st_insn): Added record handler for ex-register
load/store instructions.
---
gdb/arm-tdep.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 165 insertions(+), 2 deletions(-)
Comments
On 13 August 2014 14:12, Omair Javaid <omair.javaid@linaro.org> wrote:
> gdb:
>
> 2014-08-13 Omair Javaid <omair.javaid@linaro.org>
>
> * arm-tdep.c (arm_record_asimd_vfp_coproc): Updated.
> (arm_record_exreg_ld_st_insn): Added record handler for ex-register
> load/store instructions.
>
> ---
> gdb/arm-tdep.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 165 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index d003619..315b5b0 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -11990,6 +11990,169 @@ arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
> return -1;
> }
>
> +/* Record handler for extension register load/store instructions. */
> +
> +static int
> +arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
> +{
> + uint32_t opcode, single_reg;
> + uint8_t op_vldm_vstm;
> + uint32_t record_buf[8], record_buf_mem[128];
> + ULONGEST u_regval = 0;
> +
> + struct regcache *reg_cache = arm_insn_r->regcache;
> + const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch);
> +
> + opcode = bits (arm_insn_r->arm_insn, 20, 24);
> + single_reg = bit (arm_insn_r->arm_insn, 8);
> + op_vldm_vstm = opcode & 0x1b;
> +
> + /* Handle VMOV instructions. */
> + if ((opcode & 0x1e) == 0x04)
> + {
> + if (bit (arm_insn_r->arm_insn, 4))
> + {
> + record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15);
> + record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19);
> + arm_insn_r->reg_rec_count = 2;
> + }
> + else
> + {
> + uint8_t reg_m = (bits (arm_insn_r->arm_insn, 0, 3) << 1)
> + | bit (arm_insn_r->arm_insn, 5);
> +
> + if (!single_reg)
> + {
> + record_buf[0] = num_regs + reg_m;
> + record_buf[1] = num_regs + reg_m + 1;
> + arm_insn_r->reg_rec_count = 2;
> + }
> + else
> + {
> + record_buf[0] = reg_m + ARM_D0_REGNUM;
> + arm_insn_r->reg_rec_count = 1;
> + }
> + }
> + }
> + /* Handle VSTM and VPUSH instructions. */
> + else if (op_vldm_vstm == 0x08 || op_vldm_vstm == 0x0a
> + || op_vldm_vstm == 0x12)
> + {
> + uint32_t start_address, reg_rn, imm_off32, imm_off8, memory_count;
> + uint32_t memory_index = 0;
> +
> + reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
> + regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
> + imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
> + imm_off32 = imm_off8 << 24;
> + memory_count = imm_off8;
> +
> + if (bit (arm_insn_r->arm_insn, 23))
> + start_address = u_regval;
> + else
> + start_address = u_regval - imm_off32;
> +
> + if (bit (arm_insn_r->arm_insn, 21))
> + {
> + record_buf[0] = reg_rn;
> + arm_insn_r->reg_rec_count = 1;
> + }
> +
> + while (memory_count)
> + {
> + if (!single_reg)
> + {
> + record_buf_mem[memory_index] = start_address;
> + record_buf_mem[memory_index + 1] = 4;
> + start_address = start_address + 4;
> + memory_index = memory_index + 2;
> + }
> + else
> + {
> + record_buf_mem[memory_index] = start_address;
> + record_buf_mem[memory_index + 1] = 4;
> + record_buf_mem[memory_index + 2] = start_address + 4;
> + record_buf_mem[memory_index + 3] = 4;
> + start_address = start_address + 8;
> + memory_index = memory_index + 4;
> + }
> + memory_count--;
> + }
> + }
> + /* Handle VLDM instructions. */
> + else if (op_vldm_vstm == 0x09 || op_vldm_vstm == 0x0b
> + || op_vldm_vstm == 0x13)
> + {
> + uint32_t reg_count, reg_vd;
> + uint32_t reg_index = 0;
> +
> + reg_vd = bits (arm_insn_r->arm_insn, 12, 15);
> + reg_count = bits (arm_insn_r->arm_insn, 0, 7);
> +
> + if (single_reg)
> + reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 0) << 4);
> + else
> + reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 0);
> +
> + if (bit (arm_insn_r->arm_insn, 21))
> + record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19);
> +
> + while (reg_count)
> + {
> + if (single_reg)
> + record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
> + else
> + record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
> +
> + reg_count--;
> + }
> + }
> + /* VSTR Vector store register. */
> + else if ((opcode & 0x13) == 0x10)
> + {
> + uint32_t start_address, reg_rn, imm_off32, imm_off8, memory_count;
> + uint32_t memory_index = 0;
> +
> + reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
> + regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
> + imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
> + imm_off32 = imm_off8 << 24;
> + memory_count = imm_off8;
> +
> + if (bit (arm_insn_r->arm_insn, 23))
> + start_address = u_regval + imm_off32;
> + else
> + start_address = u_regval - imm_off32;
> +
> + if (single_reg)
> + {
> + record_buf_mem[memory_index] = start_address;
> + record_buf_mem[memory_index + 1] = 4;
> + }
> + else
> + {
> + record_buf_mem[memory_index] = start_address;
> + record_buf_mem[memory_index + 1] = 4;
> + record_buf_mem[memory_index + 2] = start_address + 4;
> + record_buf_mem[memory_index + 3] = 4;
> + }
> + }
> + /* VLDR Vector load register. */
> + else if ((opcode & 0x13) == 0x11)
> + {
> + uint8_t single_reg = 0;
> + uint8_t special_case;
Is there some missing code here? Should there be a TODO comment?
> +
> + record_buf[0] = 0;
> + record_buf[1] = 0;
> + arm_insn_r->reg_rec_count = 2;
> + }
> +
> + REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf);
> + MEM_ALLOC (arm_insn_r->arm_mems, arm_insn_r->mem_rec_count, record_buf_mem);
> + return 0;
> +}
> +
> /* Record handler for arm/thumb mode VFP data processing instructions. */
>
> static int
> @@ -12218,11 +12381,11 @@ arm_record_asimd_vfp_coproc (insn_decode_record *arm_insn_r)
> {
> /* Handle extension register ld/st instructions. */
> if (!(op1 & 0x20))
> - return arm_record_unsupported_insn (arm_insn_r);
> + return arm_record_exreg_ld_st_insn (arm_insn_r);
>
> /* 64-bit transfers between arm core and extension registers. */
> if ((op1 & 0x3e) == 0x04)
> - return arm_record_unsupported_insn (arm_insn_r);
> + return arm_record_exreg_ld_st_insn (arm_insn_r);
> }
> else
> {
> --
> 1.9.1
>
On 13 August 2014 19:10, Will Newton <will.newton@linaro.org> wrote:
> On 13 August 2014 14:12, Omair Javaid <omair.javaid@linaro.org> wrote:
>> gdb:
>>
>> 2014-08-13 Omair Javaid <omair.javaid@linaro.org>
>>
>> * arm-tdep.c (arm_record_asimd_vfp_coproc): Updated.
>> (arm_record_exreg_ld_st_insn): Added record handler for ex-register
>> load/store instructions.
>>
>> ---
>> gdb/arm-tdep.c | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 165 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> index d003619..315b5b0 100644
>> --- a/gdb/arm-tdep.c
>> +++ b/gdb/arm-tdep.c
>> @@ -11990,6 +11990,169 @@ arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
>> return -1;
>> }
>>
>> +/* Record handler for extension register load/store instructions. */
>> +
>> +static int
>> +arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
>> +{
>> + uint32_t opcode, single_reg;
>> + uint8_t op_vldm_vstm;
>> + uint32_t record_buf[8], record_buf_mem[128];
>> + ULONGEST u_regval = 0;
>> +
>> + struct regcache *reg_cache = arm_insn_r->regcache;
>> + const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch);
>> +
>> + opcode = bits (arm_insn_r->arm_insn, 20, 24);
>> + single_reg = bit (arm_insn_r->arm_insn, 8);
>> + op_vldm_vstm = opcode & 0x1b;
>> +
>> + /* Handle VMOV instructions. */
>> + if ((opcode & 0x1e) == 0x04)
>> + {
>> + if (bit (arm_insn_r->arm_insn, 4))
>> + {
>> + record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15);
>> + record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19);
>> + arm_insn_r->reg_rec_count = 2;
>> + }
>> + else
>> + {
>> + uint8_t reg_m = (bits (arm_insn_r->arm_insn, 0, 3) << 1)
>> + | bit (arm_insn_r->arm_insn, 5);
>> +
>> + if (!single_reg)
>> + {
>> + record_buf[0] = num_regs + reg_m;
>> + record_buf[1] = num_regs + reg_m + 1;
>> + arm_insn_r->reg_rec_count = 2;
>> + }
>> + else
>> + {
>> + record_buf[0] = reg_m + ARM_D0_REGNUM;
>> + arm_insn_r->reg_rec_count = 1;
>> + }
>> + }
>> + }
>> + /* Handle VSTM and VPUSH instructions. */
>> + else if (op_vldm_vstm == 0x08 || op_vldm_vstm == 0x0a
>> + || op_vldm_vstm == 0x12)
>> + {
>> + uint32_t start_address, reg_rn, imm_off32, imm_off8, memory_count;
>> + uint32_t memory_index = 0;
>> +
>> + reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
>> + regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
>> + imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
>> + imm_off32 = imm_off8 << 24;
>> + memory_count = imm_off8;
>> +
>> + if (bit (arm_insn_r->arm_insn, 23))
>> + start_address = u_regval;
>> + else
>> + start_address = u_regval - imm_off32;
>> +
>> + if (bit (arm_insn_r->arm_insn, 21))
>> + {
>> + record_buf[0] = reg_rn;
>> + arm_insn_r->reg_rec_count = 1;
>> + }
>> +
>> + while (memory_count)
>> + {
>> + if (!single_reg)
>> + {
>> + record_buf_mem[memory_index] = start_address;
>> + record_buf_mem[memory_index + 1] = 4;
>> + start_address = start_address + 4;
>> + memory_index = memory_index + 2;
>> + }
>> + else
>> + {
>> + record_buf_mem[memory_index] = start_address;
>> + record_buf_mem[memory_index + 1] = 4;
>> + record_buf_mem[memory_index + 2] = start_address + 4;
>> + record_buf_mem[memory_index + 3] = 4;
>> + start_address = start_address + 8;
>> + memory_index = memory_index + 4;
>> + }
>> + memory_count--;
>> + }
>> + }
>> + /* Handle VLDM instructions. */
>> + else if (op_vldm_vstm == 0x09 || op_vldm_vstm == 0x0b
>> + || op_vldm_vstm == 0x13)
>> + {
>> + uint32_t reg_count, reg_vd;
>> + uint32_t reg_index = 0;
>> +
>> + reg_vd = bits (arm_insn_r->arm_insn, 12, 15);
>> + reg_count = bits (arm_insn_r->arm_insn, 0, 7);
>> +
>> + if (single_reg)
>> + reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 0) << 4);
>> + else
>> + reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 0);
>> +
>> + if (bit (arm_insn_r->arm_insn, 21))
>> + record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19);
>> +
>> + while (reg_count)
>> + {
>> + if (single_reg)
>> + record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
>> + else
>> + record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
>> +
>> + reg_count--;
>> + }
>> + }
>> + /* VSTR Vector store register. */
>> + else if ((opcode & 0x13) == 0x10)
>> + {
>> + uint32_t start_address, reg_rn, imm_off32, imm_off8, memory_count;
>> + uint32_t memory_index = 0;
>> +
>> + reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
>> + regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
>> + imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
>> + imm_off32 = imm_off8 << 24;
>> + memory_count = imm_off8;
>> +
>> + if (bit (arm_insn_r->arm_insn, 23))
>> + start_address = u_regval + imm_off32;
>> + else
>> + start_address = u_regval - imm_off32;
>> +
>> + if (single_reg)
>> + {
>> + record_buf_mem[memory_index] = start_address;
>> + record_buf_mem[memory_index + 1] = 4;
>> + }
>> + else
>> + {
>> + record_buf_mem[memory_index] = start_address;
>> + record_buf_mem[memory_index + 1] = 4;
>> + record_buf_mem[memory_index + 2] = start_address + 4;
>> + record_buf_mem[memory_index + 3] = 4;
>> + }
>> + }
>> + /* VLDR Vector load register. */
>> + else if ((opcode & 0x13) == 0x11)
>> + {
>> + uint8_t single_reg = 0;
>> + uint8_t special_case;
>
> Is there some missing code here? Should there be a TODO comment?
Seems like a bit of code missing here will update it in the final patch.
>
>> +
>> + record_buf[0] = 0;
>> + record_buf[1] = 0;
>> + arm_insn_r->reg_rec_count = 2;
>> + }
>> +
>> + REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf);
>> + MEM_ALLOC (arm_insn_r->arm_mems, arm_insn_r->mem_rec_count, record_buf_mem);
>> + return 0;
>> +}
>> +
>> /* Record handler for arm/thumb mode VFP data processing instructions. */
>>
>> static int
>> @@ -12218,11 +12381,11 @@ arm_record_asimd_vfp_coproc (insn_decode_record *arm_insn_r)
>> {
>> /* Handle extension register ld/st instructions. */
>> if (!(op1 & 0x20))
>> - return arm_record_unsupported_insn (arm_insn_r);
>> + return arm_record_exreg_ld_st_insn (arm_insn_r);
>>
>> /* 64-bit transfers between arm core and extension registers. */
>> if ((op1 & 0x3e) == 0x04)
>> - return arm_record_unsupported_insn (arm_insn_r);
>> + return arm_record_exreg_ld_st_insn (arm_insn_r);
>> }
>> else
>> {
>> --
>> 1.9.1
>>
>
>
>
> --
> Will Newton
> Toolchain Working Group, Linaro
Ping! Kindly provide your feedback any further comments will help me
approve this patch series.
On 08/27/2014 10:21 AM, Omair Javaid wrote:
> + while (memory_count)
> + {
> + while (reg_count)
Not implicit boolean coercion please. Write
'memory_count != 0' or 'memory_count > 0'.
>>> >> + /* VLDR Vector load register. */
>>> >> + else if ((opcode & 0x13) == 0x11)
>>> >> + {
>>> >> + uint8_t single_reg = 0;
>>> >> + uint8_t special_case;
>> >
>> > Is there some missing code here? Should there be a TODO comment?
> Seems like a bit of code missing here will update it in the final patch.
It's kind of hard to review code that we don't see... :-)
Please post the updated patch.
> Ping! Kindly provide your feedback any further comments will help me
> approve this patch series.
This is OK if Will is happy with the updated patch.
Thanks,
Pedro Alves
@@ -11990,6 +11990,169 @@ arm_record_unsupported_insn (insn_decode_record *arm_insn_r)
return -1;
}
+/* Record handler for extension register load/store instructions. */
+
+static int
+arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
+{
+ uint32_t opcode, single_reg;
+ uint8_t op_vldm_vstm;
+ uint32_t record_buf[8], record_buf_mem[128];
+ ULONGEST u_regval = 0;
+
+ struct regcache *reg_cache = arm_insn_r->regcache;
+ const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch);
+
+ opcode = bits (arm_insn_r->arm_insn, 20, 24);
+ single_reg = bit (arm_insn_r->arm_insn, 8);
+ op_vldm_vstm = opcode & 0x1b;
+
+ /* Handle VMOV instructions. */
+ if ((opcode & 0x1e) == 0x04)
+ {
+ if (bit (arm_insn_r->arm_insn, 4))
+ {
+ record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15);
+ record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19);
+ arm_insn_r->reg_rec_count = 2;
+ }
+ else
+ {
+ uint8_t reg_m = (bits (arm_insn_r->arm_insn, 0, 3) << 1)
+ | bit (arm_insn_r->arm_insn, 5);
+
+ if (!single_reg)
+ {
+ record_buf[0] = num_regs + reg_m;
+ record_buf[1] = num_regs + reg_m + 1;
+ arm_insn_r->reg_rec_count = 2;
+ }
+ else
+ {
+ record_buf[0] = reg_m + ARM_D0_REGNUM;
+ arm_insn_r->reg_rec_count = 1;
+ }
+ }
+ }
+ /* Handle VSTM and VPUSH instructions. */
+ else if (op_vldm_vstm == 0x08 || op_vldm_vstm == 0x0a
+ || op_vldm_vstm == 0x12)
+ {
+ uint32_t start_address, reg_rn, imm_off32, imm_off8, memory_count;
+ uint32_t memory_index = 0;
+
+ reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
+ regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
+ imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
+ imm_off32 = imm_off8 << 24;
+ memory_count = imm_off8;
+
+ if (bit (arm_insn_r->arm_insn, 23))
+ start_address = u_regval;
+ else
+ start_address = u_regval - imm_off32;
+
+ if (bit (arm_insn_r->arm_insn, 21))
+ {
+ record_buf[0] = reg_rn;
+ arm_insn_r->reg_rec_count = 1;
+ }
+
+ while (memory_count)
+ {
+ if (!single_reg)
+ {
+ record_buf_mem[memory_index] = start_address;
+ record_buf_mem[memory_index + 1] = 4;
+ start_address = start_address + 4;
+ memory_index = memory_index + 2;
+ }
+ else
+ {
+ record_buf_mem[memory_index] = start_address;
+ record_buf_mem[memory_index + 1] = 4;
+ record_buf_mem[memory_index + 2] = start_address + 4;
+ record_buf_mem[memory_index + 3] = 4;
+ start_address = start_address + 8;
+ memory_index = memory_index + 4;
+ }
+ memory_count--;
+ }
+ }
+ /* Handle VLDM instructions. */
+ else if (op_vldm_vstm == 0x09 || op_vldm_vstm == 0x0b
+ || op_vldm_vstm == 0x13)
+ {
+ uint32_t reg_count, reg_vd;
+ uint32_t reg_index = 0;
+
+ reg_vd = bits (arm_insn_r->arm_insn, 12, 15);
+ reg_count = bits (arm_insn_r->arm_insn, 0, 7);
+
+ if (single_reg)
+ reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 0) << 4);
+ else
+ reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 0);
+
+ if (bit (arm_insn_r->arm_insn, 21))
+ record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19);
+
+ while (reg_count)
+ {
+ if (single_reg)
+ record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
+ else
+ record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
+
+ reg_count--;
+ }
+ }
+ /* VSTR Vector store register. */
+ else if ((opcode & 0x13) == 0x10)
+ {
+ uint32_t start_address, reg_rn, imm_off32, imm_off8, memory_count;
+ uint32_t memory_index = 0;
+
+ reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
+ regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
+ imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
+ imm_off32 = imm_off8 << 24;
+ memory_count = imm_off8;
+
+ if (bit (arm_insn_r->arm_insn, 23))
+ start_address = u_regval + imm_off32;
+ else
+ start_address = u_regval - imm_off32;
+
+ if (single_reg)
+ {
+ record_buf_mem[memory_index] = start_address;
+ record_buf_mem[memory_index + 1] = 4;
+ }
+ else
+ {
+ record_buf_mem[memory_index] = start_address;
+ record_buf_mem[memory_index + 1] = 4;
+ record_buf_mem[memory_index + 2] = start_address + 4;
+ record_buf_mem[memory_index + 3] = 4;
+ }
+ }
+ /* VLDR Vector load register. */
+ else if ((opcode & 0x13) == 0x11)
+ {
+ uint8_t single_reg = 0;
+ uint8_t special_case;
+
+ record_buf[0] = 0;
+ record_buf[1] = 0;
+ arm_insn_r->reg_rec_count = 2;
+ }
+
+ REG_ALLOC (arm_insn_r->arm_regs, arm_insn_r->reg_rec_count, record_buf);
+ MEM_ALLOC (arm_insn_r->arm_mems, arm_insn_r->mem_rec_count, record_buf_mem);
+ return 0;
+}
+
/* Record handler for arm/thumb mode VFP data processing instructions. */
static int
@@ -12218,11 +12381,11 @@ arm_record_asimd_vfp_coproc (insn_decode_record *arm_insn_r)
{
/* Handle extension register ld/st instructions. */
if (!(op1 & 0x20))
- return arm_record_unsupported_insn (arm_insn_r);
+ return arm_record_exreg_ld_st_insn (arm_insn_r);
/* 64-bit transfers between arm core and extension registers. */
if ((op1 & 0x3e) == 0x04)
- return arm_record_unsupported_insn (arm_insn_r);
+ return arm_record_exreg_ld_st_insn (arm_insn_r);
}
else
{