Re: [PATCH 17/22] target/i386: extract gen_far_call/jmp, reordering temporaries

[Richard Henderson <richard.henderson@linaro.org>]

On 12/23/23 05:15, Paolo Bonzini wrote:
> Extract the code into new functions, and swap T0/T1 so that T0 corresponds
> to the first immediate in the instruction stream.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/translate.c | 90 ++++++++++++++++++++-----------------
>   1 file changed, 50 insertions(+), 40 deletions(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index e5f71170967..edbad0ad746 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -2525,12 +2525,12 @@ static inline void gen_op_movl_T0_seg(DisasContext *s, X86Seg seg_reg)
>                        offsetof(CPUX86State,segs[seg_reg].selector));
>   }
>   
> -static inline void gen_op_movl_seg_T0_vm(DisasContext *s, X86Seg seg_reg)
> +static inline void gen_op_movl_seg_real(DisasContext *s, X86Seg seg_reg, TCGv seg)

In general, you probably want to drop inline markers as you come across them, and just let 
the compiler inline as it chooses.

>   {
> -    tcg_gen_ext16u_tl(s->T0, s->T0);
> -    tcg_gen_st32_tl(s->T0, tcg_env,
> +    tcg_gen_ext16u_tl(seg, seg);

While cleaning, maybe better to not silently modify the input?

> +static void gen_far_call(DisasContext *s)
> +{
> +    if (PE(s) && !VM86(s)) {
> +        tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T1);
> +        gen_helper_lcall_protected(tcg_env, s->tmp2_i32, s->T0,
> +                                   tcg_constant_i32(s->dflag - 1),
> +                                   eip_next_tl(s));
> +    } else {
> +        tcg_gen_trunc_tl_i32(s->tmp3_i32, s->T1);
> +        tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);

New temps?

> +        gen_helper_lcall_real(tcg_env, s->tmp3_i32, s->tmp2_i32,
> +                              tcg_constant_i32(s->dflag - 1),
> +                              eip_next_i32(s));
> +    }
> +    s->base.is_jmp = DISAS_JUMP;
> +}
> +
> +static void gen_far_jmp(DisasContext *s)
> +{
> +    if (PE(s) && !VM86(s)) {
> +        tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T1);

Likewise.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~