This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] [AArch64] Rename boolean arguments in decoding functions
- From: pinskia at gmail dot com
- To: Pierre Langlois <pierre dot langlois at arm dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Wed, 29 Jul 2015 02:42:28 -0700
- Subject: Re: [PATCH] [AArch64] Rename boolean arguments in decoding functions
- Authentication-results: sourceware.org; auth=none
- References: <1438162390-30381-1-git-send-email-pierre dot langlois at arm dot com>
> On Jul 29, 2015, at 2:33 AM, Pierre Langlois <pierre.langlois@arm.com> wrote:
>
> 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'.
Why not use the bool type instead? Since that seems like a better option and might even be better optimized.
Thanks,
Andrew
>
> 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, unsigned *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 *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 = insn >> 31;
> + *is_bl = (insn >> 31) & 0x1;
> *offset = 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, unsigned *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 = (insn >> 21) & 1;
> + *is_blr = (insn >> 21) & 1;
> *rn = (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 = (insn >> 0) & 0x1f;
> *is64 = (insn >> 31) & 0x1;
> - *op = (insn >> 24) & 0x1;
> + *is_cbnz = (insn >> 24) & 0x1;
> *offset = 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 *is64, 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 *is64, 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 = (insn >> 0) & 0x1f;
> - *op = insn & (1 << 24);
> + *is_tbnz = (insn >> 24) & 0x1;
> *bit = ((insn >> (31 - 4)) & 0x20) | ((insn >> 19) & 0x1f);
> *imm = 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] = 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;
> --
> 2.4.6
>