This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH, v2] Fix ARM machine state testcase failures
- From: Yao Qi <yao at codesourcery dot com>
- To: <lgustavo at codesourcery dot com>
- Cc: "'gdb-patches at sourceware dot org'" <gdb-patches at sourceware dot org>
- Date: Tue, 21 Oct 2014 20:46:01 +0800
- Subject: Re: [PATCH, v2] Fix ARM machine state testcase failures
- Authentication-results: sourceware.org; auth=none
- References: <54464B82 dot 6030703 at codesourcery dot com>
Luis Machado <lgustavo@codesourcery.com> writes:
> While at it, i noticed the original functions had some issues like
> unused variables and broken identation, so i went ahead and completely
> rewrote the functions handling ARM mode opcodes 010 and 100.
It's great to make code cleaner, but better to put code refactor and bug
fixing into separate patches.
> I see the same results as before, with no regressions.
>
> How does it look now?
>
> Luis
>
> 2014-10-21 Luis Machado <lgustavo@codesourcery.com>
>
> * arm-tdep.c (INSN_S_L_BIT_NUM): Document.
> (arm_record_ld_st_imm_offset): Reimplement to cover all
> load/store cases for ARM opcode 010.
> (arm_record_ld_st_multiple): Reimplement to cover all
> load/store cases for ARM opcode 100.
>
With these comments below addressed, this patch is OK to me.
>
> -/* Handling opcode 100 insns. */
> +/* Handle ARM mode instructions with opcode 100. */
>
> static int
> arm_record_ld_st_multiple (insn_decode_record *arm_insn_r)
> {
> struct regcache *reg_cache = arm_insn_r->regcache;
> -
> - uint32_t register_list[16] = {0}, register_count = 0, register_bits = 0;
> - uint32_t reg_src1 = 0, addr_mode = 0, no_of_regs = 0;
> - uint32_t start_address = 0, index = 0;
> + uint32_t register_count = 0, register_bits;
> + uint32_t reg_base, addr_mode;
> uint32_t record_buf[24], record_buf_mem[48];
> + uint32_t wback;
> + ULONGEST u_regval;
>
> - ULONGEST u_regval[2] = {0};
> + /* Fetch the list of registers. */
> + register_bits = bits (arm_insn_r->arm_insn, 0, 15);
> + arm_insn_r->reg_rec_count = 0;
> +
> + /* Fetch the base register that contains the address we are loading data
> + to. */
> + reg_base = bits (arm_insn_r->arm_insn, 16, 19);
>
> - /* This mode is exclusively for load and store multiple. */
> - /* Handle incremenrt after/before and decrment after.before mode;
> - Rn is changing depending on W bit, but as of now we store Rn too
> - without optimization. */
> + /* Calculate wback. */
> + wback = (bit (arm_insn_r->arm_insn, 24) == 0)
> + || (bit (arm_insn_r->arm_insn, 21) == 1);
Bit 24 is zero, so we only need to check bit 21.
>
> if (bit (arm_insn_r->arm_insn, INSN_S_L_BIT_NUM))
> {
> - /* LDM (1,2,3) where LDM (3) changes CPSR too. */
> + /* LDM/LDMIA/LDMFD, LDMDA/LDMFA, LDMDB and LDMIB. */
>
> - if (bit (arm_insn_r->arm_insn, 20) && !bit (arm_insn_r->arm_insn, 22))
> - {
> - register_bits = bits (arm_insn_r->arm_insn, 0, 15);
> - no_of_regs = 15;
> - }
> - else
> - {
> - register_bits = bits (arm_insn_r->arm_insn, 0, 14);
> - no_of_regs = 14;
> - }
> - /* Get Rn. */
> - reg_src1 = bits (arm_insn_r->arm_insn, 16, 19);
> + /* Find out which registers are going to be loaded from memory. */
> while (register_bits)
> - {
> - if (register_bits & 0x00000001)
> - record_buf[index++] = register_count;
> - register_bits = register_bits >> 1;
> - register_count++;
> - }
> + {
> + if (register_bits & 0x00000001)
> + record_buf[arm_insn_r->reg_rec_count++] = register_count;
> + register_bits = register_bits >> 1;
> + register_count++;
> + }
>
> - /* Extra space for Base Register and CPSR; wihtout optimization. */
> - record_buf[index++] = reg_src1;
> - record_buf[index++] = ARM_PS_REGNUM;
> - arm_insn_r->reg_rec_count = index;
> + /* Extra space for Base Register and CPSR; wihtout optimization. */
s/wihtout/without/
--
Yao (éå)