From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72753 invoked by alias); 18 Nov 2018 06:35:56 -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 72735 invoked by uid 89); 18 Nov 2018 06:35:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=gdbs, indented, GDB's, gdb's X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 18 Nov 2018 06:35:53 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6CAD9308330B; Sun, 18 Nov 2018 06:35:52 +0000 (UTC) Received: from pinnacle.lan (ovpn-116-78.phx2.redhat.com [10.3.116.78]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 47E195B68B; Sun, 18 Nov 2018 06:35:52 +0000 (UTC) Date: Sun, 18 Nov 2018 06:35:00 -0000 From: Kevin Buettner To: gdb-patches@sourceware.org Cc: John Darrington Subject: Re: [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value Message-ID: <20181117233550.09353eeb@pinnacle.lan> In-Reply-To: <20181112091721.28040-2-john@darrington.wattle.id.au> References: <20181112091721.28040-1-john@darrington.wattle.id.au> <20181112091721.28040-2-john@darrington.wattle.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00273.txt.bz2 Hi John, See my comments inline with your patch, below. On Mon, 12 Nov 2018 10:17:21 +0100 John Darrington wrote: > Make gdb aware of the return values of functions which > return in registers. >=20 > 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. > diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c > index bd0bd7c001..3da88523f7 100644 > --- a/gdb/s12z-tdep.c > +++ b/gdb/s12z-tdep.c > @@ -37,7 +37,9 @@ > #define N_PHYSICAL_REGISTERS (S12Z_N_REGISTERS - 2) >=20=20 >=20=20 > -/* A permutation of all the physical registers. */ > +/* A permutation of all the physical registers. Indexing this array > + with an integer from gdb's internal representation will return the > + register enum. */ > static const int reg_perm[N_PHYSICAL_REGISTERS] =3D > { > REG_D0, > @@ -55,6 +57,16 @@ static const int reg_perm[N_PHYSICAL_REGISTERS] =3D > REG_CCW > }; >=20=20 > +/* The inverse of the above permutation. Indexing this > + array with a register enum (e.g. REG_D2) will return the register > + number in gdb's internal representation. */ > +static const int inv_reg_perm[N_PHYSICAL_REGISTERS] =3D > + { > + 2, 3, 4, 5, /* d2, d3, d4, d5 */ > + 0, 1, /* d0, d1 */ > + 6, 7, /* d6, d7 */ > + 8, 9, 10, 11, 12 /* x, y, s, p, ccw */ > + }; 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.=20 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. 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. > /* Return the name of the register REGNUM. */ > static const char * > @@ -467,11 +479,59 @@ s12z_print_registers_info (struct gdbarch *gdbarch, >=20=20 > =0C >=20=20 > + > +static void > +s12z_extract_return_value (struct type *type, struct regcache *regcache, > + void *valbuf) > +{ > + int reg =3D -1; > + > + gdb_byte buf[4]; > + > + switch (TYPE_LENGTH (type)) > + { > + case 0: /* Nothing to do */ > + return; > + > + case 1: > + reg =3D REG_D0; > + break; > + > + case 2: > + reg =3D REG_D2; > + break; > + > + case 3: > + reg =3D REG_X; > + break; > + > + case 4: > + reg =3D 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. Kevin