From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12878 invoked by alias); 21 Oct 2014 12:49:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 12863 invoked by uid 89); 21 Oct 2014 12:49:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 21 Oct 2014 12:49:56 +0000 Received: from svr-orw-fem-05.mgc.mentorg.com ([147.34.97.43]) by relay1.mentorg.com with esmtp id 1XgYsr-0007FB-0F from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Tue, 21 Oct 2014 05:49:53 -0700 Received: from GreenOnly (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.3.181.6; Tue, 21 Oct 2014 05:49:52 -0700 From: Yao Qi To: CC: "'gdb-patches@sourceware.org'" Subject: Re: [PATCH, v2] Fix ARM machine state testcase failures References: <54464B82.6030703@codesourcery.com> Date: Tue, 21 Oct 2014 12:49:00 -0000 In-Reply-To: <54464B82.6030703@codesourcery.com> (Luis Machado's message of "Tue, 21 Oct 2014 10:03:14 -0200") Message-ID: <87oat5ioae.fsf@codesourcery.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-10/txt/msg00547.txt.bz2 Luis Machado 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 > > * 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. >=20=20 > -/* Handling opcode 100 insns. */ > +/* Handle ARM mode instructions with opcode 100. */ >=20=20 > static int > arm_record_ld_st_multiple (insn_decode_record *arm_insn_r) > { > struct regcache *reg_cache =3D arm_insn_r->regcache; > - > - uint32_t register_list[16] =3D {0}, register_count =3D 0, register_bit= s =3D 0; > - uint32_t reg_src1 =3D 0, addr_mode =3D 0, no_of_regs =3D 0; > - uint32_t start_address =3D 0, index =3D 0; > + uint32_t register_count =3D 0, register_bits; > + uint32_t reg_base, addr_mode; > uint32_t record_buf[24], record_buf_mem[48]; > + uint32_t wback; > + ULONGEST u_regval; >=20=20 > - ULONGEST u_regval[2] =3D {0}; > + /* Fetch the list of registers. */ > + register_bits =3D bits (arm_insn_r->arm_insn, 0, 15); > + arm_insn_r->reg_rec_count =3D 0; > + > + /* Fetch the base register that contains the address we are loading da= ta > + to. */ > + reg_base =3D bits (arm_insn_r->arm_insn, 16, 19); >=20=20 > - /* 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 =3D (bit (arm_insn_r->arm_insn, 24) =3D=3D 0) > + || (bit (arm_insn_r->arm_insn, 21) =3D=3D 1); Bit 24 is zero, so we only need to check bit 21. >=20=20 > 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. */ >=20=20 > - if (bit (arm_insn_r->arm_insn, 20) && !bit (arm_insn_r->arm_insn, = 22)) > - { > - register_bits =3D bits (arm_insn_r->arm_insn, 0, 15); > - no_of_regs =3D 15; > - } > - else > - { > - register_bits =3D bits (arm_insn_r->arm_insn, 0, 14); > - no_of_regs =3D 14; > - } > - /* Get Rn. */ > - reg_src1 =3D 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++] =3D register_count; > - register_bits =3D register_bits >> 1; > - register_count++; > - } > + { > + if (register_bits & 0x00000001) > + record_buf[arm_insn_r->reg_rec_count++] =3D register_count; > + register_bits =3D register_bits >> 1; > + register_count++; > + } >=20=20 > - /* Extra space for Base Register and CPSR; wihtout optimization.= */ > - record_buf[index++] =3D reg_src1; > - record_buf[index++] =3D ARM_PS_REGNUM; > - arm_insn_r->reg_rec_count =3D index; > + /* Extra space for Base Register and CPSR; wihtout optimization. = */ s/wihtout/without/ --=20 Yao (=E9=BD=90=E5=B0=A7)