[RFC] rl78-tdep.c: Make PC a pseudo-register
Joel Brobecker
brobecker@adacore.com
Wed Aug 14 14:04:00 GMT 2013
Hi Kevin,
> I'm not tremendously fond of my patch below because I feel that there
> should be a simpler way of doing things. This patch changes PC to be
> a pseudo register in which three bytes are read and written to/from
> the corresponding raw register.
>
> Comments? Can anyone think of a better way to do this?
>
> * rl78-tdep.c (RL78_RAW_PC_REGNUM): New enum.
> (RL78_PC_REGNUM): Move to list of pseudo-register enums.
> (rl78_register_type, rl78_register_name, rl78_register_reggroup_p):
> Update to account for fact that PC is now a pseudo-register.
> (rl78_pseudo_register_write, rl78_pseudo_register_read): Add
> cases for RL78_PC_REGNUM.
I am not an expert by any means, but I thought that this type
of situation is usually handled the way you just did with this
patch. For instance, 32bit mode with 64bit registers...
>
> Index: gdb/rl78-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/rl78-tdep.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rl78-tdep.c
> --- gdb/rl78-tdep.c 4 May 2013 06:14:53 -0000 1.9
> +++ gdb/rl78-tdep.c 8 Aug 2013 00:02:28 -0000
> @@ -94,7 +94,7 @@ enum
> RL78_PSW_REGNUM, /* 8 bits */
> RL78_ES_REGNUM, /* 8 bits */
> RL78_CS_REGNUM, /* 8 bits */
> - RL78_PC_REGNUM, /* 20 bits; we'll use 32 bits for it. */
> + RL78_RAW_PC_REGNUM, /* 20 bits; we'll use 32 bits for it. */
>
> /* Fixed address SFRs (some of those above are SFRs too.) */
> RL78_SPL_REGNUM, /* 8 bits; lower half of SP */
> @@ -105,7 +105,8 @@ enum
> RL78_NUM_REGS,
>
> /* Pseudo registers. */
> - RL78_SP_REGNUM = RL78_NUM_REGS,
> + RL78_PC_REGNUM = RL78_NUM_REGS,
> + RL78_SP_REGNUM,
Out of curiosity, why not include RL78_SP_REGNUM in RL78_NUM_REGS?
> if ((regnum < RL78_NUM_REGS
> && regnum != RL78_SPL_REGNUM
> - && regnum != RL78_SPH_REGNUM)
> - || regnum == RL78_SP_REGNUM)
> + && regnum != RL78_SPH_REGNUM
> + && regnum != RL78_RAW_PC_REGNUM)
> + || regnum == RL78_SP_REGNUM
> + || regnum == RL78_PC_REGNUM)
FYI, there is an inconsistent use of tabs vs spaces that made the review
of this change a little harder...
> + else if (reg == RL78_PC_REGNUM)
> + {
> + gdb_byte rawbuf[4];
> + status = regcache_raw_read (regcache, RL78_RAW_PC_REGNUM, rawbuf);
> + memcpy (buffer, rawbuf, 3);
> + }
> else if (RL78_X_REGNUM <= reg && reg <= RL78_H_REGNUM)
> {
> ULONGEST psw;
> @@ -527,6 +540,13 @@ rl78_pseudo_register_write (struct gdbar
> regcache_raw_write (regcache, RL78_SPL_REGNUM, buffer);
> regcache_raw_write (regcache, RL78_SPH_REGNUM, buffer + 1);
> }
> + else if (reg == RL78_PC_REGNUM)
> + {
> + gdb_byte rawbuf[4];
> + memcpy (rawbuf, buffer, 3);
> + rawbuf[3] = 0;
> + regcache_raw_write (regcache, RL78_RAW_PC_REGNUM, rawbuf);
> + }
In both hunks, you're missing an empty line after the rawbuf
variable declaration (one of the many coding rules of the GDB
project).
Other than that, I didn't see anything obviously wrong with the patch.
--
Joel
More information about the Gdb-patches
mailing list