diff mbox series

[5/9] target/i386: Preserve DisasContextBase.insn_start across rewind

Message ID 20240406223248.502699-6-richard.henderson@linaro.org
State New, archived
Headers show
Series accel/tcg: Fix can_do_io vs 2nd page mmio | expand

Commit Message

Richard Henderson April 6, 2024, 10:32 p.m. UTC
When aborting translation of the current insn, restore the
previous value of insn_start.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/tcg/translate.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Paolo Bonzini April 8, 2024, 7:25 a.m. UTC | #1
On Mon, Apr 8, 2024 at 9:15 AM <no-reply@patchew.org> wrote:
> When aborting translation of the current insn, restore the
> previous value of insn_start.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

The TODO comment in disas_insn() in fact can be expanded a bit.
When/if the table-based decoder takes over completely, decoding can be
moved completely to insn_start, and insn_start could return a bool to
undo the beginning of the translation.

Paolo

> ---
>  target/i386/tcg/translate.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 07f642dc9e..76a42c679c 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -139,6 +139,7 @@ typedef struct DisasContext {
>      TCGv_i64 tmp1_i64;
>
>      sigjmp_buf jmpbuf;
> +    TCGOp *prev_insn_start;
>      TCGOp *prev_insn_end;
>  } DisasContext;
>
> @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>          /* END TODO */
>          s->base.num_insns--;
>          tcg_remove_ops_after(s->prev_insn_end);
> +        s->base.insn_start = s->prev_insn_start;
>          s->base.is_jmp = DISAS_TOO_MANY;
>          return false;
>      default:
> @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>      DisasContext *dc = container_of(dcbase, DisasContext, base);
>      target_ulong pc_arg = dc->base.pc_next;
>
> +    dc->prev_insn_start = dc->base.insn_start;
>      dc->prev_insn_end = tcg_last_op();
>      if (tb_cflags(dcbase->tb) & CF_PCREL) {
>          pc_arg &= ~TARGET_PAGE_MASK;
> --
> 2.34.1
>
Philippe Mathieu-Daudé April 9, 2024, 3:23 p.m. UTC | #2
On 7/4/24 00:32, Richard Henderson wrote:
> When aborting translation of the current insn, restore the
> previous value of insn_start.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/i386/tcg/translate.c | 3 +++
>   1 file changed, 3 insertions(+)


> @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>           /* END TODO */
>           s->base.num_insns--;
>           tcg_remove_ops_after(s->prev_insn_end);
> +        s->base.insn_start = s->prev_insn_start;
>           s->base.is_jmp = DISAS_TOO_MANY;
>           return false;
>       default:
> @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
>       DisasContext *dc = container_of(dcbase, DisasContext, base);
>       target_ulong pc_arg = dc->base.pc_next;
>   
> +    dc->prev_insn_start = dc->base.insn_start;
>       dc->prev_insn_end = tcg_last_op();
>       if (tb_cflags(dcbase->tb) & CF_PCREL) {
>           pc_arg &= ~TARGET_PAGE_MASK;

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Philippe Mathieu-Daudé April 9, 2024, 3:28 p.m. UTC | #3
On 9/4/24 17:23, Philippe Mathieu-Daudé wrote:
> On 7/4/24 00:32, Richard Henderson wrote:
>> When aborting translation of the current insn, restore the
>> previous value of insn_start.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/i386/tcg/translate.c | 3 +++
>>   1 file changed, 3 insertions(+)
> 
> 
>> @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState 
>> *cpu)
>>           /* END TODO */
>>           s->base.num_insns--;
>>           tcg_remove_ops_after(s->prev_insn_end);
>> +        s->base.insn_start = s->prev_insn_start;
>>           s->base.is_jmp = DISAS_TOO_MANY;
>>           return false;
>>       default:
>> @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase 
>> *dcbase, CPUState *cpu)
>>       DisasContext *dc = container_of(dcbase, DisasContext, base);
>>       target_ulong pc_arg = dc->base.pc_next;
>> +    dc->prev_insn_start = dc->base.insn_start;
>>       dc->prev_insn_end = tcg_last_op();
>>       if (tb_cflags(dcbase->tb) & CF_PCREL) {
>>           pc_arg &= ~TARGET_PAGE_MASK;
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

And:
Tested-by: Jørgen Hansen <Jorgen.Hansen@wdc.com>
(also to patches 1 & 2)

:)
diff mbox series

Patch

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 07f642dc9e..76a42c679c 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -139,6 +139,7 @@  typedef struct DisasContext {
     TCGv_i64 tmp1_i64;
 
     sigjmp_buf jmpbuf;
+    TCGOp *prev_insn_start;
     TCGOp *prev_insn_end;
 } DisasContext;
 
@@ -3123,6 +3124,7 @@  static bool disas_insn(DisasContext *s, CPUState *cpu)
         /* END TODO */
         s->base.num_insns--;
         tcg_remove_ops_after(s->prev_insn_end);
+        s->base.insn_start = s->prev_insn_start;
         s->base.is_jmp = DISAS_TOO_MANY;
         return false;
     default:
@@ -6995,6 +6997,7 @@  static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
     DisasContext *dc = container_of(dcbase, DisasContext, base);
     target_ulong pc_arg = dc->base.pc_next;
 
+    dc->prev_insn_start = dc->base.insn_start;
     dc->prev_insn_end = tcg_last_op();
     if (tb_cflags(dcbase->tb) & CF_PCREL) {
         pc_arg &= ~TARGET_PAGE_MASK;