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

[Richard Henderson]

On 9/21/22 06:09, Paolo Bonzini wrote:
> 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.

Hmm.  But is that correct?  That's not reflected by the pseudocode for Jcc.


r~