From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35965) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dMcQ9-0001OB-Rd for qemu-devel@nongnu.org; Sun, 18 Jun 2017 11:47:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dMcQ5-0002Vr-TE for qemu-devel@nongnu.org; Sun, 18 Jun 2017 11:47:25 -0400 Received: from roura.ac.upc.edu ([147.83.33.10]:57273 helo=roura.ac.upc.es) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dMcQ5-0002VD-HF for qemu-devel@nongnu.org; Sun, 18 Jun 2017 11:47:21 -0400 From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= References: <149727922719.28532.11985025310576184920.stgit@frigg.lan> <149727924970.28532.9346819516051209538.stgit@frigg.lan> <20170615221911.GB26408@flamenco> <20170615232507.GA15332@flamenco> <87r2yhe5os.fsf@frigg.lan> Date: Sun, 18 Jun 2017 18:47:07 +0300 In-Reply-To: <87r2yhe5os.fsf@frigg.lan> (=?utf-8?Q?=22Llu=C3=ADs?= Vilanova"'s message of "Sun, 18 Jun 2017 17:22:43 +0300") Message-ID: <87a855b8n8.fsf@frigg.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Emilio G. Cota" Cc: Paolo Bonzini , Peter Crosthwaite , Alex =?utf-8?Q?Benn=C3=A9e?= , qemu-devel@nongnu.org, Richard Henderson Llu=C3=ADs Vilanova writes: > Emilio G Cota writes: >> On Thu, Jun 15, 2017 at 18:19:11 -0400, Emilio G. Cota wrote: >>> (snip) >>> > +/** >>> > + * DisasContextBase: >>> > + * @tb: Translation block for this disassembly. >>> > + * @pc_first: Address of first guest instruction in this TB. >>> > + * @pc_next: Address of next guest instruction in this TB (current d= uring >>> > + * disassembly). >>> > + * @num_insns: Number of translated instructions (including current). >>> > + * @singlestep_enabled: "Hardware" single stepping enabled. >>> > + * >>> > + * Architecture-agnostic disassembly context. >>> > + */ >>> > +typedef struct DisasContextBase { >>> > + TranslationBlock *tb; >>> > + target_ulong pc_first; >>> > + target_ulong pc_next; >>> > + DisasJumpType jmp_type; >>> > + unsigned int num_insns; >>> > + bool singlestep_enabled; >>> > +} DisasContextBase; >>>=20 >>> - @pc_next: I'd stick with @pc, it's shorter, it's everywhere already, = and >>> with the documentation it's very clear what it is for. >>> - @jmp_type: missing doc :-) >> Also, consider keeping the @is_jmp name instead of renaming it to >> @jmp_type. (@jmp would be shorter but it would be confusing though, >> e.g. cris has both dc->jmp and dc->is_jmp.) > I just figured that this series could also take the chance of trying to r= ename a > few common variables I'm changing to something more readable. > But if you feel very strongly about keeping the original names (and minim= izing > the diffs as you say later), I'll revert the name changes. Also, going through the changes to break them down into smaller pieces, I s= aw that TranslationBlock (at least in i386) already has a "pc" member, so using "pc_next" in DisasContextBase makes it even clearer it's a different variab= le. You comments still apply to "is_jmp" vs "jmp_type" though. Unless you or an= ybody else feels strongly against it, I'll keep "jmp_type", since I'm already cha= nging all lines that reference "is_jmp" to use DisasContextBase (instead of DisasContext). Thanks, Lluis