Re: [PATCH v2 15/23] target/i386: Create eip_next_*

[Paolo Bonzini]

On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create helpers for loading the address of the next insn.
> Use tcg_constant_* in adjacent code where convenient.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

> ---
>  target/i386/tcg/translate.c | 44 +++++++++++++++++++------------------
>  1 file changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 427ee72442..527fb79895 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -541,6 +541,16 @@ static TCGv_i32 cur_insn_len_i32(DisasContext *s)
>      return tcg_constant_i32(cur_insn_len(s));
>  }
>
> +static TCGv_i32 eip_next_i32(DisasContext *s)
> +{
> +    return tcg_constant_i32(s->pc - s->cs_base);

Perhaps this should return 0xdeadbeef or 0xffffffff in 64-bit mode, so
that code goes in the weeds very quickly if it tries to use it?

Alternatively, just make iret_protected take a target_ulong for
next_eip, without too much worries about 64-on-32 emulation.

Paolo

> +}
> +
> +static TCGv eip_next_tl(DisasContext *s)
> +{
> +    return tcg_constant_tl(s->pc - s->cs_base);
> +}
> +
>  /* Compute SEG:REG into A0.  SEG is selected from the override segment
>     (OVR_SEG) and the default segment (DEF_SEG).  OVR_SEG may be -1 to
>     indicate no override.  */
> @@ -1213,12 +1223,9 @@ static void gen_bpt_io(DisasContext *s, TCGv_i32 
> t_port, int ot)
>          /* user-mode cpu should not be in IOBPT mode */
>          g_assert_not_reached();
>  #else
> -        TCGv_i32 t_size = tcg_const_i32(1 << ot);
> -        TCGv t_next = tcg_const_tl(s->pc - s->cs_base);
> -
> +        TCGv_i32 t_size = tcg_constant_i32(1 << ot);
> +        TCGv t_next = eip_next_tl(s);
>          gen_helper_bpt_io(cpu_env, t_port, t_size, t_next);
> -        tcg_temp_free_i32(t_size);
> -        tcg_temp_free(t_next);
>  #endif /* CONFIG_USER_ONLY */
>      }
>  }
> @@ -5280,9 +5287,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>              if (dflag == MO_16) {
>                  tcg_gen_ext16u_tl(s->T0, s->T0);
>              }
> -            next_eip = s->pc - s->cs_base;
> -            tcg_gen_movi_tl(s->T1, next_eip);
> -            gen_push_v(s, s->T1);
> +            gen_push_v(s, eip_next_tl(s));
>              gen_op_jmp_v(s->T0);
>              gen_bnd_jmp(s);
>              s->base.is_jmp = DISAS_JUMP;
> @@ -5298,14 +5303,14 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>              if (PE(s) && !VM86(s)) {
>                  tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
>                  gen_helper_lcall_protected(cpu_env, s->tmp2_i32, s->T1,
> -                                           tcg_const_i32(dflag - 1),
> -                                           tcg_const_tl(s->pc - s->cs_base));
> +                                           tcg_constant_i32(dflag - 1),
> +                                           eip_next_tl(s));
>              } else {
>                  tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
>                  tcg_gen_trunc_tl_i32(s->tmp3_i32, s->T1);
>                  gen_helper_lcall_real(cpu_env, s->tmp2_i32, s->tmp3_i32,
> -                                      tcg_const_i32(dflag - 1),
> -                                      tcg_const_i32(s->pc - s->cs_base));
> +                                      tcg_constant_i32(dflag - 1),
> +                                      eip_next_i32(s));
>              }
>              s->base.is_jmp = DISAS_JUMP;
>              break;
> @@ -5328,7 +5333,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>              if (PE(s) && !VM86(s)) {
>                  tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
>                  gen_helper_ljmp_protected(cpu_env, s->tmp2_i32, s->T1,
> -                                          tcg_const_tl(s->pc - s->cs_base));
> +                                          eip_next_tl(s));
>              } else {
>                  gen_op_movl_seg_T0_vm(s, R_CS);
>                  gen_op_jmp_v(s->T1);
> @@ -6819,8 +6824,8 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>              }
>              gen_helper_iret_real(cpu_env, tcg_const_i32(dflag - 1));
>          } else {
> -            gen_helper_iret_protected(cpu_env, tcg_const_i32(dflag - 1),
> -                                      tcg_const_i32(s->pc - s->cs_base));
> +            gen_helper_iret_protected(cpu_env, tcg_constant_i32(dflag - 1),
> +                                      eip_next_i32(s));
>          }
>          set_cc_op(s, CC_OP_EFLAGS);
>          s->base.is_jmp = DISAS_EOB_ONLY;
> @@ -6832,15 +6837,13 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>              } else {
>                  tval = (int16_t)insn_get(env, s, MO_16);
>              }
> -            next_eip = s->pc - s->cs_base;
> -            tval += next_eip;
> +            tval += s->pc - s->cs_base;
>              if (dflag == MO_16) {
>                  tval &= 0xffff;
>              } else if (!CODE64(s)) {
>                  tval &= 0xffffffff;
>              }
> -            tcg_gen_movi_tl(s->T0, next_eip);
> -            gen_push_v(s, s->T0);
> +            gen_push_v(s, eip_next_tl(s));
>              gen_bnd_jmp(s);
>              gen_jmp(s, tval);
>          }
> @@ -7374,8 +7377,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>              TCGLabel *l1, *l2, *l3;
>
>              tval = (int8_t)insn_get(env, s, MO_8);
> -            next_eip = s->pc - s->cs_base;
> -            tval += next_eip;
> +            tval += s->pc - s->cs_base;
>              if (dflag == MO_16) {
>                  tval &= 0xffff;
>              }
> --
> 2.34.1
>