Re: [Qemu-devel] [PATCH v11 10/29] target/i386: [tcg] Refactor translate_insn

["Emilio G. Cota" <cota@braap.org>]

On Wed, Jun 28, 2017 at 15:57:00 +0300, Lluís Vilanova wrote:
> Incrementally paves the way towards using the generic instruction translation
> loop.
> 
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  target/i386/translate.c |   72 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 48 insertions(+), 24 deletions(-)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 3eee348de7..da4b409d97 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
> @@ -4420,15 +4420,17 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
>  
>  /* convert one instruction. s->base.is_jmp is set if the translation must
>     be stopped. Return the next pc value */
> -static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
> -                               target_ulong pc_start)
> +static target_ulong disas_insn(DisasContextBase *dcbase, CPUState *cpu)
>  {
> +    DisasContext *s = container_of(dcbase, DisasContext, base);
> +    CPUX86State *env = cpu->env_ptr;

Minor nit: you can pass dc (*s) here directly, no need for container_of

(snip)
> +static target_ulong i386_trblock_translate_insn(DisasContextBase *dcbase,
> +                                                CPUState *cpu)
> +{
> +    DisasContext *dc = container_of(dcbase, DisasContext, base);
> +    target_ulong pc_next = disas_insn(&dc->base, cpu);
> +
> +    if (dc->base.is_jmp) {
> +        return pc_next;
> +    }
> +
> +    if (dc->tf || (dc->base.tb->flags & HF_INHIBIT_IRQ_MASK)) {
> +        /* if single step mode, we generate only one instruction and
> +           generate an exception */
> +        /* if irq were inhibited with HF_INHIBIT_IRQ_MASK, we clear
> +           the flag and abort the translation to give the irqs a
> +           change to be happen */

I know you just moved lines around, but while at it, s/change to be/chance to/

Other than that,

Reviewed-by: Emilio G. Cota <cota@braap.org>

		E.