diff mbox

[v11,13/29] target/i386: [tcg] Port to generic translation framework

Message ID 149865534644.17063.5754453204442221401.stgit@frigg.lan (mailing list archive)
State New, archived
Headers show

Commit Message

Lluís Vilanova June 28, 2017, 1:09 p.m. UTC
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 target/i386/translate.c |  120 +++++++----------------------------------------
 1 file changed, 18 insertions(+), 102 deletions(-)

Comments

Emilio Cota June 30, 2017, 1:11 a.m. UTC | #1
On Wed, Jun 28, 2017 at 16:09:06 +0300, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---

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

BTW have you tested icount mode?

Minor nits below.

>  target/i386/translate.c |  120 +++++++----------------------------------------
>  1 file changed, 18 insertions(+), 102 deletions(-)
> 
> diff --git a/target/i386/translate.c b/target/i386/translate.c
> index 3950fe95a4..295be26a95 100644
> --- a/target/i386/translate.c
> +++ b/target/i386/translate.c
(snip)
> @@ -8544,111 +8548,23 @@ static void i386_trblock_disas_log(const DisasContextBase *dcbase,
>  
>  }
>  
> +static const TranslatorOps i386_trblock_ops = {
> +    .init_disas_context = i386_trblock_init_disas_context,
> +    .init_globals = i386_trblock_init_globals,
> +    .tb_start = i386_trblock_tb_start,
> +    .insn_start = i386_trblock_insn_start,
> +    .breakpoint_check = i386_trblock_breakpoint_check,
> +    .translate_insn = i386_trblock_translate_insn,
> +    .tb_stop = i386_trblock_tb_stop,
> +    .disas_log = i386_trblock_disas_log,
> +};

I like hard tabs here, which make things visually easier, e.g.:

> +static const TranslatorOps i386_trblock_ops = {
> +    .init_disas_context	= i386_trblock_init_disas_context,
> +    .init_globals		= i386_trblock_init_globals,

Don't know whether checkpatch likes it, but I do, and I see some
examples in the code base.

>  /* generate intermediate code for basic block 'tb'.  */
>  void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
>  {
(snip)
> +    DisasContext dc1;
>  
> -    tb->size = dc->base.pc_next - dc->base.pc_first;
> -    tb->icount = num_insns;
> +    translate_block(&i386_trblock_ops, &dc1.base, cpu, tb);
>  }

I'd just call it dc, then.

		E.
Richard Henderson July 2, 2017, 12:52 a.m. UTC | #2
On 06/28/2017 06:09 AM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova<vilanova@ac.upc.edu>
> ---
>   target/i386/translate.c |  120 +++++++----------------------------------------
>   1 file changed, 18 insertions(+), 102 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
Lluís Vilanova July 7, 2017, 10:27 a.m. UTC | #3
Emilio G Cota writes:

> On Wed, Jun 28, 2017 at 16:09:06 +0300, Lluís Vilanova wrote:
[...]
>> diff --git a/target/i386/translate.c b/target/i386/translate.c
>> index 3950fe95a4..295be26a95 100644
>> --- a/target/i386/translate.c
>> +++ b/target/i386/translate.c
> (snip)
>> @@ -8544,111 +8548,23 @@ static void i386_trblock_disas_log(const DisasContextBase *dcbase,
>> 
>> }
>> 
>> +static const TranslatorOps i386_trblock_ops = {
>> +    .init_disas_context = i386_trblock_init_disas_context,
>> +    .init_globals = i386_trblock_init_globals,
>> +    .tb_start = i386_trblock_tb_start,
>> +    .insn_start = i386_trblock_insn_start,
>> +    .breakpoint_check = i386_trblock_breakpoint_check,
>> +    .translate_insn = i386_trblock_translate_insn,
>> +    .tb_stop = i386_trblock_tb_stop,
>> +    .disas_log = i386_trblock_disas_log,
>> +};

> I like hard tabs here, which make things visually easier, e.g.:

>> +static const TranslatorOps i386_trblock_ops = {
>> +    .init_disas_context	= i386_trblock_init_disas_context,
>> +    .init_globals		= i386_trblock_init_globals,

> Don't know whether checkpatch likes it, but I do, and I see some
> examples in the code base.

According to the guidelines, not tabs should be used (and I'm assuming
checkpatch looks for that). The best I can do is to space-align these.


Thanks,
  Lluis
Lluís Vilanova July 7, 2017, 10:29 a.m. UTC | #4
Emilio G Cota writes:

> On Wed, Jun 28, 2017 at 16:09:06 +0300, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---

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

> BTW have you tested icount mode?

Yes. Regular, -singlestep and -icount 1.

Thanks,
  Lluis
diff mbox

Patch

diff --git a/target/i386/translate.c b/target/i386/translate.c
index 3950fe95a4..295be26a95 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -8454,6 +8454,10 @@  static void i386_trblock_init_globals(DisasContextBase *dcbase, CPUState *cpu)
     cpu_cc_srcT = tcg_temp_local_new();
 }
 
+static void i386_trblock_tb_start(DisasContextBase *db, CPUState *cpu)
+{
+}
+
 static void i386_trblock_insn_start(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
@@ -8472,7 +8476,7 @@  static BreakpointCheckType i386_trblock_breakpoint_check(
         /* The address covered by the breakpoint must be included in
            [tb->pc, tb->pc + tb->size) in order to for it to be
            properly cleared -- thus we increment the PC here so that
-           the logic setting tb->size below does the right thing.  */
+           the generic logic setting tb->size later does the right thing.  */
         dc->base.pc_next += 1;
         return BC_HIT_TB;
     } else {
@@ -8544,111 +8548,23 @@  static void i386_trblock_disas_log(const DisasContextBase *dcbase,
 
 }
 
+static const TranslatorOps i386_trblock_ops = {
+    .init_disas_context = i386_trblock_init_disas_context,
+    .init_globals = i386_trblock_init_globals,
+    .tb_start = i386_trblock_tb_start,
+    .insn_start = i386_trblock_insn_start,
+    .breakpoint_check = i386_trblock_breakpoint_check,
+    .translate_insn = i386_trblock_translate_insn,
+    .tb_stop = i386_trblock_tb_stop,
+    .disas_log = i386_trblock_disas_log,
+};
+
 /* generate intermediate code for basic block 'tb'.  */
 void gen_intermediate_code(CPUState *cpu, TranslationBlock *tb)
 {
-    DisasContext dc1, *dc = &dc1;
-    int num_insns;
-    int max_insns;
-
-    /* generate intermediate code */
-    dc->base.singlestep_enabled = cpu->singlestep_enabled;
-    dc->base.tb = tb;
-    dc->base.is_jmp = DISAS_NEXT;
-    dc->base.pc_first = tb->pc;
-    dc->base.pc_next = dc->base.pc_first;
-    i386_trblock_init_disas_context(&dc->base, cpu);
-
-    i386_trblock_init_globals(&dc->base, cpu);
-
-    num_insns = 0;
-    max_insns = tb->cflags & CF_COUNT_MASK;
-    if (max_insns == 0) {
-        max_insns = CF_COUNT_MASK;
-    }
-    if (max_insns > TCG_MAX_INSNS) {
-        max_insns = TCG_MAX_INSNS;
-    }
-
-    gen_tb_start(tb);
-    for(;;) {
-        i386_trblock_insn_start(&dc->base, cpu);
-        num_insns++;
-
-        if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
-            CPUBreakpoint *bp;
-            QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
-                if (bp->pc == dc->base.pc_next) {
-                    BreakpointCheckType bp_check =
-                        i386_trblock_breakpoint_check(&dc->base, cpu, bp);
-                    switch (bp_check) {
-                    case BC_MISS:
-                        /* Target ignored this breakpoint, go to next */
-                        break;
-                    case BC_HIT_INSN:
-                        /* Hit, keep translating */
-                        /*
-                         * TODO: if we're never going to have more than one
-                         *       BP in a single address, we can simply use a
-                         *       bool here.
-                         */
-                        goto done_breakpoints;
-                    case BC_HIT_TB:
-                        /* Hit, end TB */
-                        goto done_generating;
-                    default:
-                        g_assert_not_reached();
-                    }
-                }
-            }
-        }
-    done_breakpoints:
-
-        if (num_insns == max_insns && (tb->cflags & CF_LAST_IO)) {
-            gen_io_start();
-        }
-
-        dc->base.pc_next = i386_trblock_translate_insn(&dc->base, cpu);
-        /* stop translation if indicated */
-        if (dc->base.is_jmp) {
-            break;
-        }
-        /* if single step mode, we generate only one instruction and
-           generate an exception */
-        if (dc->base.singlestep_enabled) {
-            dc->base.is_jmp = DISAS_TOO_MANY;
-            break;
-        }
-        /* if too long translation, stop generation too */
-        if (tcg_op_buf_full() ||
-            num_insns >= max_insns) {
-            dc->base.is_jmp = DISAS_TOO_MANY;
-            break;
-        }
-        if (singlestep) {
-            dc->base.is_jmp = DISAS_TOO_MANY;
-            break;
-        }
-    }
-    i386_trblock_tb_stop(&dc->base, cpu);
-    if (tb->cflags & CF_LAST_IO)
-        gen_io_end();
-done_generating:
-    gen_tb_end(tb, num_insns);
-
-#ifdef DEBUG_DISAS
-    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)
-        && qemu_log_in_addr_range(dc->base.pc_first)) {
-        qemu_log_lock();
-        qemu_log("----------------\n");
-        i386_trblock_disas_log(&dc->base, cpu);
-        qemu_log("\n");
-        qemu_log_unlock();
-    }
-#endif
+    DisasContext dc1;
 
-    tb->size = dc->base.pc_next - dc->base.pc_first;
-    tb->icount = num_insns;
+    translate_block(&i386_trblock_ops, &dc1.base, cpu, tb);
 }
 
 void restore_state_to_opc(CPUX86State *env, TranslationBlock *tb,