On Fri, May 24, 2024 at 4:23 PM Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 5/24/24 01:10, Paolo Bonzini wrote:
> > Place DISAS_* constants that update cpu_eip first, and
> > the "jump" ones last. Add comments explaining the differences
> > and usage.
> >
> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> > ---
> > target/i386/tcg/translate.c | 25 ++++++++++++++++++++++---
> > 1 file changed, 22 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> > index 3c7d8d72144..52d758a224b 100644
> > --- a/target/i386/tcg/translate.c
> > +++ b/target/i386/tcg/translate.c
> > @@ -144,9 +144,28 @@ typedef struct DisasContext {
> > TCGOp *prev_insn_end;
> > } DisasContext;
> >
> > -#define DISAS_EOB_ONLY DISAS_TARGET_0
> > -#define DISAS_EOB_NEXT DISAS_TARGET_1
> > -#define DISAS_EOB_INHIBIT_IRQ DISAS_TARGET_2
> > +/*
> > + * Point EIP to next instruction before ending translation.
> > + * For instructions that can change hflags.
> > + */
> > +#define DISAS_EOB_NEXT DISAS_TARGET_0
> > +
> > +/*
> > + * Point EIP to next instruction and set HF_INHIBIT_IRQ if not
> > + * already set. For instructions that activate interrupt shadow.
> > + */
> > +#define DISAS_EOB_INHIBIT_IRQ DISAS_TARGET_1
> > +
> > +/*
> > + * EIP has already been updated. For jumps that do not use
> > + * lookup_and_goto_ptr()
> > + */
> > +#define DISAS_EOB_ONLY DISAS_TARGET_2
>
> Better as "for instructions that must return to the main loop", because pure
> jumps should
> either use goto_tb (DISAS_NORETURN) or lookup_and_goto_ptr (DISAS_JUMP).
I guess it depends on what you mean by jump... Instructions that use
DISAS_EOB_ONLY are "jumpy" - IRET, RETF, SYSENTER, SYSEXIT, RSM. They
cannot use DISAS_NORETURN because they need to call gen_update_cc_op()
before finishing the TB.
Except now that I think about it, at the end of the series they don't
set cc_op anymore, so probably there's room for some more cleanup
there and DISAS_EOB_ONLY can be removed.
Paolo
