[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation fr
From: |
Lluís Vilanova |
Subject: |
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework |
Date: |
Sun, 18 Jun 2017 18:47:07 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Lluís 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 during
>>> > + * 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;
>>>
>>> - @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
> rename a
> few common variables I'm changing to something more readable.
> But if you feel very strongly about keeping the original names (and minimizing
> 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 saw
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 variable.
You comments still apply to "is_jmp" vs "jmp_type" though. Unless you or anybody
else feels strongly against it, I'll keep "jmp_type", since I'm already changing
all lines that reference "is_jmp" to use DisasContextBase (instead of
DisasContext).
Thanks,
Lluis
- [Qemu-devel] [PATCH] translator mega-patch, (continued)
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework, Emilio G. Cota, 2017/06/15
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework, Lluís Vilanova, 2017/06/18
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework,
Lluís Vilanova <=
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework, Lluís Vilanova, 2017/06/18
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework, Emilio G. Cota, 2017/06/19
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework, Lluís Vilanova, 2017/06/19
Re: [Qemu-devel] [PATCH v6 3/6] target: [tcg] Add generic translation framework, Lluís Vilanova, 2017/06/18
[Qemu-devel] [PATCH v6 4/6] target: [tcg] Redefine DISAS_* onto the generic translation framework (DJ_*), Lluís Vilanova, 2017/06/12
[Qemu-devel] [PATCH v6 6/6] target: [tcg, arm] Port to generic translation framework, Lluís Vilanova, 2017/06/12
[Qemu-devel] [PATCH v6 1/6] Pass generic CPUState to gen_intermediate_code(), Lluís Vilanova, 2017/06/12