Re: [PATCH v2 12/23] target/i386: Remove cur_eip, next_eip arguments to gen_repz*

2022-09-21 Thread Paolo Bonzini
On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson
 wrote:
>
> All callers pass s->base.pc_next and s->pc, which we can just
> as well compute within the functions.  Pull out common helpers
> and reduce the amount of code under macros.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Paolo Bonzini 


> ---
>  target/i386/tcg/translate.c | 116 ++--
>  1 file changed, 57 insertions(+), 59 deletions(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 393a1c1075..f3c26a9956 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -736,7 +736,7 @@ static bool gen_check_io(DisasContext *s, MemOp ot, 
> TCGv_i32 port,
>  #endif
>  }
>
> -static inline void gen_movs(DisasContext *s, MemOp ot)
> +static void gen_movs(DisasContext *s, MemOp ot)
>  {
>  gen_string_movl_A0_ESI(s);
>  gen_op_ld_v(s, ot, s->T0, s->A0);
> @@ -1156,18 +1156,18 @@ static inline void gen_jcc1(DisasContext *s, int b, 
> TCGLabel *l1)
>
>  /* XXX: does not work with gdbstub "ice" single step - not a
> serious problem */
> -static TCGLabel *gen_jz_ecx_string(DisasContext *s, target_ulong next_eip)
> +static TCGLabel *gen_jz_ecx_string(DisasContext *s)
>  {
>  TCGLabel *l1 = gen_new_label();
>  TCGLabel *l2 = gen_new_label();
>  gen_op_jnz_ecx(s, s->aflag, l1);
>  gen_set_label(l2);
> -gen_jmp_tb(s, next_eip, 1);
> +gen_jmp_tb(s, s->pc - s->cs_base, 1);
>  gen_set_label(l1);
>  return l2;
>  }
>
> -static inline void gen_stos(DisasContext *s, MemOp ot)
> +static void gen_stos(DisasContext *s, MemOp ot)
>  {
>  gen_op_mov_v_reg(s, MO_32, s->T0, R_EAX);
>  gen_string_movl_A0_EDI(s);
> @@ -1176,7 +1176,7 @@ static inline void gen_stos(DisasContext *s, MemOp ot)
>  gen_op_add_reg_T0(s, s->aflag, R_EDI);
>  }
>
> -static inline void gen_lods(DisasContext *s, MemOp ot)
> +static void gen_lods(DisasContext *s, MemOp ot)
>  {
>  gen_string_movl_A0_ESI(s);
>  gen_op_ld_v(s, ot, s->T0, s->A0);
> @@ -1185,7 +1185,7 @@ static inline void gen_lods(DisasContext *s, MemOp ot)
>  gen_op_add_reg_T0(s, s->aflag, R_ESI);
>  }
>
> -static inline void gen_scas(DisasContext *s, MemOp ot)
> +static void gen_scas(DisasContext *s, MemOp ot)
>  {
>  gen_string_movl_A0_EDI(s);
>  gen_op_ld_v(s, ot, s->T1, s->A0);
> @@ -1194,7 +1194,7 @@ static inline void gen_scas(DisasContext *s, MemOp ot)
>  gen_op_add_reg_T0(s, s->aflag, R_EDI);
>  }
>
> -static inline void gen_cmps(DisasContext *s, MemOp ot)
> +static void gen_cmps(DisasContext *s, MemOp ot)
>  {
>  gen_string_movl_A0_EDI(s);
>  gen_op_ld_v(s, ot, s->T1, s->A0);
> @@ -1222,7 +1222,7 @@ static void gen_bpt_io(DisasContext *s, TCGv_i32 
> t_port, int ot)
>  }
>  }
>
> -static inline void gen_ins(DisasContext *s, MemOp ot)
> +static void gen_ins(DisasContext *s, MemOp ot)
>  {
>  gen_string_movl_A0_EDI(s);
>  /* Note: we must do this dummy write first to be restartable in
> @@ -1238,7 +1238,7 @@ static inline void gen_ins(DisasContext *s, MemOp ot)
>  gen_bpt_io(s, s->tmp2_i32, ot);
>  }
>
> -static inline void gen_outs(DisasContext *s, MemOp ot)
> +static void gen_outs(DisasContext *s, MemOp ot)
>  {
>  gen_string_movl_A0_ESI(s);
>  gen_op_ld_v(s, ot, s->T0, s->A0);
> @@ -1252,42 +1252,49 @@ static inline void gen_outs(DisasContext *s, MemOp ot)
>  gen_bpt_io(s, s->tmp2_i32, ot);
>  }
>
> -/* same method as Valgrind : we generate jumps to current or next
> -   instruction */
> -#define GEN_REPZ(op) 
>  \
> -static inline void gen_repz_ ## op(DisasContext *s, MemOp ot,  \
> - target_ulong cur_eip, target_ulong 
> next_eip) \
> -{
>  \
> -TCGLabel *l2;
>  \
> -gen_update_cc_op(s); 
>  \
> -l2 = gen_jz_ecx_string(s, next_eip); 
>  \
> -gen_ ## op(s, ot);   
>  \
> -gen_op_add_reg_im(s, s->aflag, R_ECX, -1);   
>  \
> -/* a loop would cause two single step exceptions if ECX = 1  
>  \
> -   before rep string_insn */ 
>  \
> -if (s->repz_opt) 
>  \
> -gen_op_jz_ecx(s, s->aflag, l2);  
>  \
> -gen_jmp(s, cur_eip); 
>  \
> +/* Generate jumps to current or next instruction */
> +static void gen_repz(DisasContext *s, MemOp ot,
> + void (*fn)(DisasContext *s, MemOp ot))
> +{
> +TCGLabel *l2;
> +gen_update_cc_op(s);
> +l2 = gen_jz_ecx_string(s);
> +

[PATCH v2 12/23] target/i386: Remove cur_eip, next_eip arguments to gen_repz*

2022-09-06 Thread Richard Henderson
All callers pass s->base.pc_next and s->pc, which we can just
as well compute within the functions.  Pull out common helpers
and reduce the amount of code under macros.

Signed-off-by: Richard Henderson 
---
 target/i386/tcg/translate.c | 116 ++--
 1 file changed, 57 insertions(+), 59 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 393a1c1075..f3c26a9956 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -736,7 +736,7 @@ static bool gen_check_io(DisasContext *s, MemOp ot, 
TCGv_i32 port,
 #endif
 }
 
-static inline void gen_movs(DisasContext *s, MemOp ot)
+static void gen_movs(DisasContext *s, MemOp ot)
 {
 gen_string_movl_A0_ESI(s);
 gen_op_ld_v(s, ot, s->T0, s->A0);
@@ -1156,18 +1156,18 @@ static inline void gen_jcc1(DisasContext *s, int b, 
TCGLabel *l1)
 
 /* XXX: does not work with gdbstub "ice" single step - not a
serious problem */
-static TCGLabel *gen_jz_ecx_string(DisasContext *s, target_ulong next_eip)
+static TCGLabel *gen_jz_ecx_string(DisasContext *s)
 {
 TCGLabel *l1 = gen_new_label();
 TCGLabel *l2 = gen_new_label();
 gen_op_jnz_ecx(s, s->aflag, l1);
 gen_set_label(l2);
-gen_jmp_tb(s, next_eip, 1);
+gen_jmp_tb(s, s->pc - s->cs_base, 1);
 gen_set_label(l1);
 return l2;
 }
 
-static inline void gen_stos(DisasContext *s, MemOp ot)
+static void gen_stos(DisasContext *s, MemOp ot)
 {
 gen_op_mov_v_reg(s, MO_32, s->T0, R_EAX);
 gen_string_movl_A0_EDI(s);
@@ -1176,7 +1176,7 @@ static inline void gen_stos(DisasContext *s, MemOp ot)
 gen_op_add_reg_T0(s, s->aflag, R_EDI);
 }
 
-static inline void gen_lods(DisasContext *s, MemOp ot)
+static void gen_lods(DisasContext *s, MemOp ot)
 {
 gen_string_movl_A0_ESI(s);
 gen_op_ld_v(s, ot, s->T0, s->A0);
@@ -1185,7 +1185,7 @@ static inline void gen_lods(DisasContext *s, MemOp ot)
 gen_op_add_reg_T0(s, s->aflag, R_ESI);
 }
 
-static inline void gen_scas(DisasContext *s, MemOp ot)
+static void gen_scas(DisasContext *s, MemOp ot)
 {
 gen_string_movl_A0_EDI(s);
 gen_op_ld_v(s, ot, s->T1, s->A0);
@@ -1194,7 +1194,7 @@ static inline void gen_scas(DisasContext *s, MemOp ot)
 gen_op_add_reg_T0(s, s->aflag, R_EDI);
 }
 
-static inline void gen_cmps(DisasContext *s, MemOp ot)
+static void gen_cmps(DisasContext *s, MemOp ot)
 {
 gen_string_movl_A0_EDI(s);
 gen_op_ld_v(s, ot, s->T1, s->A0);
@@ -1222,7 +1222,7 @@ static void gen_bpt_io(DisasContext *s, TCGv_i32 t_port, 
int ot)
 }
 }
 
-static inline void gen_ins(DisasContext *s, MemOp ot)
+static void gen_ins(DisasContext *s, MemOp ot)
 {
 gen_string_movl_A0_EDI(s);
 /* Note: we must do this dummy write first to be restartable in
@@ -1238,7 +1238,7 @@ static inline void gen_ins(DisasContext *s, MemOp ot)
 gen_bpt_io(s, s->tmp2_i32, ot);
 }
 
-static inline void gen_outs(DisasContext *s, MemOp ot)
+static void gen_outs(DisasContext *s, MemOp ot)
 {
 gen_string_movl_A0_ESI(s);
 gen_op_ld_v(s, ot, s->T0, s->A0);
@@ -1252,42 +1252,49 @@ static inline void gen_outs(DisasContext *s, MemOp ot)
 gen_bpt_io(s, s->tmp2_i32, ot);
 }
 
-/* same method as Valgrind : we generate jumps to current or next
-   instruction */
-#define GEN_REPZ(op)  \
-static inline void gen_repz_ ## op(DisasContext *s, MemOp ot,  \
- target_ulong cur_eip, target_ulong next_eip) \
-{ \
-TCGLabel *l2; \
-gen_update_cc_op(s);  \
-l2 = gen_jz_ecx_string(s, next_eip);  \
-gen_ ## op(s, ot);\
-gen_op_add_reg_im(s, s->aflag, R_ECX, -1);\
-/* a loop would cause two single step exceptions if ECX = 1   \
-   before rep string_insn */  \
-if (s->repz_opt)  \
-gen_op_jz_ecx(s, s->aflag, l2);   \
-gen_jmp(s, cur_eip);  \
+/* Generate jumps to current or next instruction */
+static void gen_repz(DisasContext *s, MemOp ot,
+ void (*fn)(DisasContext *s, MemOp ot))
+{
+TCGLabel *l2;
+gen_update_cc_op(s);
+l2 = gen_jz_ecx_string(s);
+fn(s, ot);
+gen_op_add_reg_im(s, s->aflag, R_ECX, -1);
+/*
+ * A loop would cause two single step exceptions if ECX = 1
+ * before rep string_insn
+ */
+if (s->repz_opt) {
+gen_op_jz_ecx(s, s->aflag, l2);
+}
+gen_jmp(s, s->base.pc_next - s->cs_base);
 }
 
-#define GEN_REPZ2(op)