Re: [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value

John Darrington
2018-11-18T09:13:54Z

Hi Kevin,

Thanks for the review.

On Sat, Nov 17, 2018 at 11:35:50PM -0700, Kevin Buettner wrote:
>
> gdb/ChangeLog:
> * s12z-tdep.c (s12z_extract_return_value): New function.
> (inv_reg_perm) New array.
> (s12z_return_value): Populate readbuf if non-null.

Make sure that this is indented correctly when it eventually goes
in the ChangeLog file.

Is it documented anywhere what "correctly" means?

My two cents on all of the above...

I think you'll have a lot less grief with this architecture port if
you don't try to use the numbering defined in include/opcode/s12z.h.
Create a new numbering with new constants for GDB's purposes ordered
as shown in the reg_perm array. Then use these constants in place of
the various REG_ constants that are currently in s12z-tdep.c. If you
still want to be able to access registers[], it may make sense to
have an array which maps GDB's constants to those in include/opcode.

I'm beginning to think that you're right here. I may change it in the
way you propose in a future patch.

Also, if you want CCH and CCL to be show in "info registers" and/or
allow the user to display and set them, these can be implemented via
the use of pseudo-registers.

A the moment, I don't think it's worth bothering about. CCH and CCL are
merely the high and low bytes of a 16bit register CCW. The names CCH
and CCL only exist because of two (rarely used) instructions in the ISA.

> /* Return the name of the register REGNUM. */
> static const char *
> @@ -467,11 +479,59 @@ s12z_print_registers_info (struct gdbarch *gdbarch,
>
> \f
>
> +
> +static void
> +s12z_extract_return_value (struct type *type, struct regcache *regcache,
> + void *valbuf)
> +{
> + int reg = -1;
> +
> + gdb_byte buf[4];
> +
> + switch (TYPE_LENGTH (type))
> + {
> + case 0: /* Nothing to do */
> + return;
> +
> + case 1:
> + reg = REG_D0;
> + break;
> +
> + case 2:
> + reg = REG_D2;
> + break;
> +
> + case 3:
> + reg = REG_X;
> + break;
> +
> + case 4:
> + reg = REG_D6;
> + break;
> +
> + default:
> + error (_("bad size for return value"));
> + return;
> + }
> +
> + regcache->cooked_read (inv_reg_perm[reg], buf);
> + memcpy (valbuf, buf, TYPE_LENGTH (type));

Is there any reason not to just pass valbuf in place of buf to
cooked_read? Doing so will get rid of the memcpy.

Probably not. Thanks for noticing this.

J'

--
Avoid eavesdropping. Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.