From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21728 invoked by alias); 29 Jul 2015 10:44:26 -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 21719 invoked by uid 89); 29 Jul 2015 10:44:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Jul 2015 10:44:24 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-29-Vsrt6AtjTQO9T6xDruAHKQ-1; Wed, 29 Jul 2015 11:44:19 +0100 Received: from [10.2.207.36] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 29 Jul 2015 11:44:19 +0100 Message-ID: <55B8AE83.6040407@arm.com> Date: Wed, 29 Jul 2015 10:44:00 -0000 From: Pierre Langlois User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: "pinskia@gmail.com" CC: pierre.langlois@arm.com, "gdb-patches@sourceware.org" Subject: Re: [PATCH] [AArch64] Rename boolean arguments in decoding functions References: <1438162390-30381-1-git-send-email-pierre.langlois@arm.com> In-Reply-To: X-MC-Unique: Vsrt6AtjTQO9T6xDruAHKQ-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00860.txt.bz2 On 29/07/15 10:42, pinskia@gmail.com wrote: >=20 >=20 >=20 >=20 >> On Jul 29, 2015, at 2:33 AM, Pierre Langlois w= rote: >> >> Hi all, >> >> 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. >> >> The instructions affected are BR/BLR, B/BL, CBZ/CBNZ and TBZ/TBNZ. >> >> 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'. >=20 > Why not use the bool type instead? Since that seems like a better option = and might even be better optimized.=20 Hi Andrew, I agree it'd be better, but I saw that using in GDB was removed here because of issues building on Solaris and AiX: https://sourceware.org/ml/gdb-patches/2015-02/msg00804.html Thanks, Pierre >=20 > Thanks, > Andrew >=20 >> >> Thanks, >> Pierre >> >> gdb/ChangeLog: >> >> * 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(-) >> >> 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, unsign= ed *rd) >> >> 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. >> >> Return 1 if the opcodes matches and is decoded, otherwise 0. */ >> >> static int >> -decode_b (CORE_ADDR addr, uint32_t insn, unsigned *link, int32_t *offse= t) >> +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; >> >> 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)); >> >> return 1; >> @@ -358,27 +358,27 @@ decode_bcond (CORE_ADDR addr, uint32_t insn, unsig= ned *cond, int32_t *offset) >> >> 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. >> >> Return 1 if the opcodes matches and is decoded, otherwise 0. */ >> >> 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; >> >> 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); >> >> 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. >> >> Return 1 if the opcodes matches and is decoded, otherwise 0. */ >> >> 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, >> >> *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; >> >> 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 *is6= 4, unsigned *rt, >> >> 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 *is6= 4, unsigned *rt, >> Return 1 if the opcodes matches and is decoded, otherwise 0. */ >> >> 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 */ >> >> *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; >> >> @@ -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; >> >> @@ -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