Re: [PATCH v2 19/23] target/i386: Use gen_jmp_rel for gen_jcc

[Paolo Bonzini]

On Tue, Sep 6, 2022 at 12:09 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
> -static inline void gen_jcc(DisasContext *s, int b,
> -                           target_ulong val, target_ulong next_eip)
> +static void gen_jcc(DisasContext *s, MemOp ot, int b, int diff)
>  {
> -    TCGLabel *l1, *l2;
> +    TCGLabel *l1 = gen_new_label();
>
> -    if (s->jmp_opt) {
> -        l1 = gen_new_label();
> -        gen_jcc1(s, b, l1);
> -
> -        gen_goto_tb(s, 0, next_eip);
> -
> -        gen_set_label(l1);
> -        gen_goto_tb(s, 1, val);
> -    } else {
> -        l1 = gen_new_label();
> -        l2 = gen_new_label();
> -        gen_jcc1(s, b, l1);
> -
> -        gen_jmp_im(s, next_eip);
> -        tcg_gen_br(l2);
> -
> -        gen_set_label(l1);
> -        gen_jmp_im(s, val);
> -        gen_set_label(l2);
> -        gen_eob(s);
> -    }
> +    gen_jcc1(s, b, l1);
> +    gen_jmp_rel(s, ot, 0, 1);
> +    gen_set_label(l1);
> +    gen_jmp_rel(s, ot, diff, 0);

Might be worth a comment that jumps with 16-bit operand size truncate
EIP even if the jump is not taken.

Otherwise,

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

Paolo

>  }
>
>  static void gen_cmovcc1(CPUX86State *env, DisasContext *s, MemOp ot, int b,
> @@ -4721,7 +4703,6 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>      int shift;
>      MemOp ot, aflag, dflag;
>      int modrm, reg, rm, mod, op, opreg, val;
> -    target_ulong next_eip, tval;
>      bool orig_cc_op_dirty = s->cc_op_dirty;
>      CCOp orig_cc_op = s->cc_op;
>
> @@ -6881,22 +6862,20 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
>          }
>          break;
>      case 0x70 ... 0x7f: /* jcc Jb */
> -        tval = (int8_t)insn_get(env, s, MO_8);
> -        goto do_jcc;
> +        {
> +            int diff = (int8_t)insn_get(env, s, MO_8);
> +            gen_bnd_jmp(s);
> +            gen_jcc(s, dflag, b, diff);
> +        }
> +        break;
>      case 0x180 ... 0x18f: /* jcc Jv */
> -        if (dflag != MO_16) {
> -            tval = (int32_t)insn_get(env, s, MO_32);
> -        } else {
> -            tval = (int16_t)insn_get(env, s, MO_16);
> +        {
> +            int diff = (dflag != MO_16
> +                        ? (int32_t)insn_get(env, s, MO_32)
> +                        : (int16_t)insn_get(env, s, MO_16));
> +            gen_bnd_jmp(s);
> +            gen_jcc(s, dflag, b, diff);
>          }
> -    do_jcc:
> -        next_eip = s->pc - s->cs_base;
> -        tval += next_eip;
> -        if (dflag == MO_16) {
> -            tval &= 0xffff;
> -        }
> -        gen_bnd_jmp(s);
> -        gen_jcc(s, b, tval, next_eip);
>          break;
>
>      case 0x190 ... 0x19f: /* setcc Gv */
> --
> 2.34.1
>