On Tue, Sep 6, 2022 at 12:10 PM Richard Henderson <richard.hender...@linaro.org> wrote: > static void gen_update_eip_cur(DisasContext *s) > { > gen_jmp_im(s, s->base.pc_next - s->cs_base); > + s->pc_save = s->base.pc_next;
s->pc_save is not valid after all gen_jmp_im() calls. Is it worth noting after each call to gen_jmp_im() why this is not a problem? > } > > static void gen_update_eip_next(DisasContext *s) > { > gen_jmp_im(s, s->pc - s->cs_base); > + s->pc_save = s->pc; > +} > + > +static TCGv gen_eip_cur(DisasContext *s) > +{ > + if (TARGET_TB_PCREL) { > + gen_update_eip_cur(s); > + return cpu_eip; > + } else { > + return tcg_constant_tl(s->base.pc_next - s->cs_base); > + } Ok, now I see why you called it gen_eip_cur(), but it's still a bit disconcerting to see the difference in behavior between the TARGET_TB_PCREL and !TARGET_TB_PCREL cases, one of them updating cpu_eip and other not. Perhaps gen_jmp_im() and gen_update_eip_cur() could be rewritten to return the destination instead: static TCGv gen_jmp_im(DisasContext *s, target_ulong eip) { if (TARGET_TB_PCREL) { target_ulong eip_save = s->pc_save - s->cs_base; tcg_gen_addi_tl(cpu_eip, cpu_eip, eip - eip_save); return cpu_eip; } else { TCGv dest = tcg_constant_tl(eip); tcg_gen_mov_tl(cpu_eip, dest); return dest; } } static TCGv gen_update_eip_cur(DisasContext *s) { TCGv dest = gen_jmp_im(s, s->base.pc_next - s->cs_base); s->pc_save = s->base.pc_next; return dest; } and the "if (update_ip)" case would use the return value? This change would basically replace the previous patch, with just the "if (TARGET_TB_PCREL)" added here. Paolo