From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88542 invoked by alias); 29 Jul 2015 09:42:38 -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 88530 invoked by uid 89); 29 Jul 2015 09:42:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f170.google.com Received: from mail-pd0-f170.google.com (HELO mail-pd0-f170.google.com) (209.85.192.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 29 Jul 2015 09:42:35 +0000 Received: by pdbnt7 with SMTP id nt7so3165862pdb.0 for ; Wed, 29 Jul 2015 02:42:33 -0700 (PDT) X-Received: by 10.70.55.134 with SMTP id s6mr89685152pdp.137.1438162953467; Wed, 29 Jul 2015 02:42:33 -0700 (PDT) Received: from [192.168.1.35] (76-253-1-90.lightspeed.sntcca.sbcglobal.net. [76.253.1.90]) by smtp.gmail.com with ESMTPSA id os7sm39681046pdb.51.2015.07.29.02.42.31 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 29 Jul 2015 02:42:32 -0700 (PDT) References: <1438162390-30381-1-git-send-email-pierre.langlois@arm.com> Mime-Version: 1.0 (1.0) In-Reply-To: <1438162390-30381-1-git-send-email-pierre.langlois@arm.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Message-Id: Cc: "gdb-patches@sourceware.org" From: pinskia@gmail.com Subject: Re: [PATCH] [AArch64] Rename boolean arguments in decoding functions Date: Wed, 29 Jul 2015 09:42:00 -0000 To: Pierre Langlois X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00856.txt.bz2 > On Jul 29, 2015, at 2:33 AM, Pierre Langlois wr= ote: >=20 > Hi all, >=20 > This patch cleans up the decoding functions using booleans when they can > decode two instructions. The boolean argument is used to know which of > the two instructions was decoded. >=20 > The instructions affected are BR/BLR, B/BL, CBZ/CBNZ and TBZ/TBNZ. >=20 > These arguments would be named after a named bit in the instruction > encoding, this patch renames them to 'is_XXX'. Furthermore, the > 'unsigned' type would be used to describe a boolean while > aarch64_decode_cb would use 'int' (see the 'is64' argument). This patch > makes all booleans be 'int' and decoded bitfields be 'unsigned'. Why not use the bool type instead? Since that seems like a better option an= d might even be better optimized.=20 Thanks, Andrew >=20 > Thanks, > Pierre >=20 > gdb/ChangeLog: >=20 > * aarch64-tdep.c (decode_b): Rename link argument to is_bl. > Change its type to int *. > (decode_br): Rename link argument to is_blr. Change its type to > int *. > (decode_cb): Rename op argument to is_cbnz. Change its type to > int *. > (decode_tb): Rename op argument to is_tbnz. Change its type to > int *. Set is_tbnz to either 1 or 0. > (aarch64_analyze_prologue): Change type of is_link to int. Add > new variables is_cbnz and is_tbnz. Adjust call to > aarch64_decode_cb and aarch64_decode_tb. > --- > gdb/aarch64-tdep.c | 47 +++++++++++++++++++++++------------------------ > 1 file changed, 23 insertions(+), 24 deletions(-) >=20 > diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > index cec4d3e..c722dc5 100644 > --- a/gdb/aarch64-tdep.c > +++ b/gdb/aarch64-tdep.c > @@ -299,26 +299,26 @@ decode_adrp (CORE_ADDR addr, uint32_t insn, unsigne= d *rd) >=20 > ADDR specifies the address of the opcode. > INSN specifies the opcode to test. > - LINK receives the 'link' bit from the decoded instruction. > + IS_BL receives the 'op' bit from the decoded instruction. > OFFSET receives the immediate offset from the decoded instruction. >=20 > Return 1 if the opcodes matches and is decoded, otherwise 0. */ >=20 > static int > -decode_b (CORE_ADDR addr, uint32_t insn, unsigned *link, int32_t *offset) > +decode_b (CORE_ADDR addr, uint32_t insn, int *is_bl, int32_t *offset) > { > /* b 0001 01ii iiii iiii iiii iiii iiii iiii */ > /* bl 1001 01ii iiii iiii iiii iiii iiii iiii */ > if (decode_masked_match (insn, 0x7c000000, 0x14000000)) > { > - *link =3D insn >> 31; > + *is_bl =3D (insn >> 31) & 0x1; > *offset =3D extract_signed_bitfield (insn, 26, 0) << 2; >=20 > if (aarch64_debug) > fprintf_unfiltered (gdb_stdlog, > "decode: 0x%s 0x%x %s 0x%s\n", > core_addr_to_string_nz (addr), insn, > - *link ? "bl" : "b", > + *is_bl ? "bl" : "b", > core_addr_to_string_nz (addr + *offset)); >=20 > return 1; > @@ -358,27 +358,27 @@ decode_bcond (CORE_ADDR addr, uint32_t insn, unsign= ed *cond, int32_t *offset) >=20 > ADDR specifies the address of the opcode. > INSN specifies the opcode to test. > - LINK receives the 'link' bit from the decoded instruction. > + IS_BLR receives the 'op' bit from the decoded instruction. > RN receives the 'rn' field from the decoded instruction. >=20 > Return 1 if the opcodes matches and is decoded, otherwise 0. */ >=20 > static int > -decode_br (CORE_ADDR addr, uint32_t insn, unsigned *link, unsigned *rn) > +decode_br (CORE_ADDR addr, uint32_t insn, int *is_blr, unsigned *rn) > { > /* 8 4 0 6 2 8 4 0 */ > /* blr 110101100011111100000000000rrrrr */ > /* br 110101100001111100000000000rrrrr */ > if (decode_masked_match (insn, 0xffdffc1f, 0xd61f0000)) > { > - *link =3D (insn >> 21) & 1; > + *is_blr =3D (insn >> 21) & 1; > *rn =3D (insn >> 5) & 0x1f; >=20 > if (aarch64_debug) > fprintf_unfiltered (gdb_stdlog, > "decode: 0x%s 0x%x %s 0x%x\n", > core_addr_to_string_nz (addr), insn, > - *link ? "blr" : "br", *rn); > + *is_blr ? "blr" : "br", *rn); >=20 > return 1; > } > @@ -390,16 +390,15 @@ decode_br (CORE_ADDR addr, uint32_t insn, unsigned = *link, unsigned *rn) > ADDR specifies the address of the opcode. > INSN specifies the opcode to test. > IS64 receives the 'sf' field from the decoded instruction. > - OP receives the 'op' field from the decoded instruction. > + IS_CBNZ receives the 'op' field from the decoded instruction. > RN receives the 'rn' field from the decoded instruction. > OFFSET receives the 'imm19' field from the decoded instruction. >=20 > Return 1 if the opcodes matches and is decoded, otherwise 0. */ >=20 > static int > -decode_cb (CORE_ADDR addr, > - uint32_t insn, int *is64, unsigned *op, unsigned *rn, > - int32_t *offset) > +decode_cb (CORE_ADDR addr, uint32_t insn, int *is64, int *is_cbnz, > + unsigned *rn, int32_t *offset) > { > if (decode_masked_match (insn, 0x7e000000, 0x34000000)) > { > @@ -408,14 +407,14 @@ decode_cb (CORE_ADDR addr, >=20 > *rn =3D (insn >> 0) & 0x1f; > *is64 =3D (insn >> 31) & 0x1; > - *op =3D (insn >> 24) & 0x1; > + *is_cbnz =3D (insn >> 24) & 0x1; > *offset =3D extract_signed_bitfield (insn, 19, 5) << 2; >=20 > if (aarch64_debug) > fprintf_unfiltered (gdb_stdlog, > "decode: 0x%s 0x%x %s 0x%s\n", > core_addr_to_string_nz (addr), insn, > - *op ? "cbnz" : "cbz", > + *is_cbnz ? "cbnz" : "cbz", > core_addr_to_string_nz (addr + *offset)); > return 1; > } > @@ -632,7 +631,7 @@ decode_stur (CORE_ADDR addr, uint32_t insn, int *is64= , unsigned *rt, >=20 > ADDR specifies the address of the opcode. > INSN specifies the opcode to test. > - OP receives the 'op' field from the decoded instruction. > + IS_TBNZ receives the 'op' field from the decoded instruction. > BIT receives the bit position field from the decoded instruction. > RT receives 'rt' field from the decoded instruction. > IMM receives 'imm' field from the decoded instruction. > @@ -640,9 +639,8 @@ decode_stur (CORE_ADDR addr, uint32_t insn, int *is64= , unsigned *rt, > Return 1 if the opcodes matches and is decoded, otherwise 0. */ >=20 > static int > -decode_tb (CORE_ADDR addr, > - uint32_t insn, unsigned *op, unsigned *bit, unsigned *rt, > - int32_t *imm) > +decode_tb (CORE_ADDR addr, uint32_t insn, int *is_tbnz, unsigned *bit, > + unsigned *rt, int32_t *imm) > { > if (decode_masked_match (insn, 0x7e000000, 0x36000000)) > { > @@ -650,7 +648,7 @@ decode_tb (CORE_ADDR addr, > /* tbnz B011 0111 bbbb biii iiii iiii iiir rrrr */ >=20 > *rt =3D (insn >> 0) & 0x1f; > - *op =3D insn & (1 << 24); > + *is_tbnz =3D (insn >> 24) & 0x1; > *bit =3D ((insn >> (31 - 4)) & 0x20) | ((insn >> 19) & 0x1f); > *imm =3D extract_signed_bitfield (insn, 14, 5) << 2; >=20 > @@ -658,7 +656,7 @@ decode_tb (CORE_ADDR addr, > fprintf_unfiltered (gdb_stdlog, > "decode: 0x%s 0x%x %s x%u, #%u, 0x%s\n", > core_addr_to_string_nz (addr), insn, > - *op ? "tbnz" : "tbz", *rt, *bit, > + *is_tbnz ? "tbnz" : "tbz", *rt, *bit, > core_addr_to_string_nz (addr + *imm)); > return 1; > } > @@ -698,8 +696,9 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, > int32_t imm; > unsigned cond; > int is64; > - unsigned is_link; > - unsigned op; > + int is_link; > + int is_cbnz; > + int is_tbnz; > unsigned bit; > int32_t offset; >=20 > @@ -724,7 +723,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, > /* Stop analysis on branch. */ > break; > } > - else if (decode_cb (start, insn, &is64, &op, &rn, &offset)) > + else if (decode_cb (start, insn, &is64, &is_cbnz, &rn, &offset)) > { > /* Stop analysis on branch. */ > break; > @@ -800,7 +799,7 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch, > regs[rt2]); > regs[rn] =3D pv_add_constant (regs[rn], imm); > } > - else if (decode_tb (start, insn, &op, &bit, &rn, &offset)) > + else if (decode_tb (start, insn, &is_tbnz, &bit, &rn, &offset)) > { > /* Stop analysis on branch. */ > break; > --=20 > 2.4.6 >=20