[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: |
Mon, 19 Jun 2017 00:54:05 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Lluís Vilanova writes:
> 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).
Aha, just checked your proposed patches more closely and it totally makes sense
to keep "is_jmp" to simplify the diffs, so I'll go for that one.
Thanks!
Lluis
- Re: [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, 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, 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