Re: [PATCH v2 02/23] target/i386: Return bool from disas_insn

[Philippe Mathieu-Daudé]

On 6/9/22 12:09, Richard Henderson wrote:
> Instead of returning the new pc, which is present in
> DisasContext, return true if an insn was translated.
> This is false when we detect a page crossing and must
> undo the insn under translation.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/i386/tcg/translate.c | 42 +++++++++++++++++++------------------
>   1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 1e24bb2985..46300ffd91 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -4665,7 +4665,7 @@ 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(DisasContext *s, CPUState *cpu)
> +static bool disas_insn(DisasContext *s, CPUState *cpu)
>   {
>       CPUX86State *env = cpu->env_ptr;
>       int b, prefixes;
> @@ -4695,12 +4695,13 @@ static target_ulong disas_insn(DisasContext *s, 
> CPUState *cpu)
>           return s->pc;

Shouldn't we return 'true' here?

>       case 2:
>           /* Restore state that may affect the next instruction. */
> +        s->pc = s->base.pc_next;
>           s->cc_op_dirty = orig_cc_op_dirty;
>           s->cc_op = orig_cc_op;
>           s->base.num_insns--;
>           tcg_remove_ops_after(s->prev_insn_end);
>           s->base.is_jmp = DISAS_TOO_MANY;
> -        return s->base.pc_next;
> +        return false;
>       default:
>           g_assert_not_reached();
>       }
> @@ -8609,13 +8610,13 @@ static target_ulong disas_insn(DisasContext *s, 
> CPUState *cpu)
>       default:
>           goto unknown_op;
>       }
> -    return s->pc;
> +    return true;
>    illegal_op:
>       gen_illegal_opcode(s);
> -    return s->pc;
> +    return true;
>    unknown_op:
>       gen_unknown_opcode(env, s);
> -    return s->pc;
> +    return true;
>   }