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

[Philippe Mathieu-Daudé via]

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.hender...@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;
  }