* [PATCH 01/16] target/i386: remove unnecessary gen_update_cc_op before gen_eob*
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:11 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 02/16] target/i386: cleanup eob handling of RSM Paolo Bonzini
` (14 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
This is already handled in gen_eob(). Before adding another DISAS_*
case, remove the double calls.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 76be7425800..f44edb3c29c 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -4776,14 +4776,12 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_jmp_rel_csize(dc, 0, 0);
break;
case DISAS_EOB_NEXT:
- gen_update_cc_op(dc);
gen_update_eip_cur(dc);
/* fall through */
case DISAS_EOB_ONLY:
gen_eob(dc);
break;
case DISAS_EOB_INHIBIT_IRQ:
- gen_update_cc_op(dc);
gen_update_eip_cur(dc);
gen_eob_inhibit_irq(dc);
break;
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 02/16] target/i386: cleanup eob handling of RSM
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
2024-05-24 8:10 ` [PATCH 01/16] target/i386: remove unnecessary gen_update_cc_op before gen_eob* Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:14 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 03/16] target/i386: document and group DISAS_* constants Paolo Bonzini
` (13 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
gen_helper_rsm cannot generate an exception, and reloads the flags.
So there's no need to spill cc_op and update cpu_eip, but on the
other hand cc_op must be reset to CC_OP_EFLAGS before returning.
It all works by chance, because by spilling cc_op before the call
to the helper, it becomes non-dirty and gen_eob will not overwrite
the CC_OP_EFLAGS value that is placed there by the helper. But
let's clean it up.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index f44edb3c29c..3c7d8d72144 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -4488,9 +4488,8 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
/* we should not be in SMM mode */
g_assert_not_reached();
#else
- gen_update_cc_op(s);
- gen_update_eip_next(s);
gen_helper_rsm(tcg_env);
+ set_cc_op(s, CC_OP_EFLAGS);
#endif /* CONFIG_USER_ONLY */
s->base.is_jmp = DISAS_EOB_ONLY;
break;
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 02/16] target/i386: cleanup eob handling of RSM
2024-05-24 8:10 ` [PATCH 02/16] target/i386: cleanup eob handling of RSM Paolo Bonzini
@ 2024-05-24 14:14 ` Richard Henderson
0 siblings, 0 replies; 38+ messages in thread
From: Richard Henderson @ 2024-05-24 14:14 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 5/24/24 01:10, Paolo Bonzini wrote:
> gen_helper_rsm cannot generate an exception, and reloads the flags.
> So there's no need to spill cc_op and update cpu_eip, but on the
> other hand cc_op must be reset to CC_OP_EFLAGS before returning.
>
> It all works by chance, because by spilling cc_op before the call
> to the helper, it becomes non-dirty and gen_eob will not overwrite
> the CC_OP_EFLAGS value that is placed there by the helper. But
> let's clean it up.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
> target/i386/tcg/translate.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
2024-05-24 8:10 ` [PATCH 01/16] target/i386: remove unnecessary gen_update_cc_op before gen_eob* Paolo Bonzini
2024-05-24 8:10 ` [PATCH 02/16] target/i386: cleanup eob handling of RSM Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:20 ` Richard Henderson
2024-05-24 14:23 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 04/16] target/i386: avoid calling gen_eob_syscall before tb_stop Paolo Bonzini
` (12 subsequent siblings)
15 siblings, 2 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
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 <pbonzini@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
+
+/*
+ * EIP has already been updated. For jumps that wish to use
+ * lookup_and_goto_ptr()
+ */
#define DISAS_JUMP DISAS_TARGET_3
/* The environment in which user-only runs is constrained. */
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 8:10 ` [PATCH 03/16] target/i386: document and group DISAS_* constants Paolo Bonzini
@ 2024-05-24 14:20 ` Richard Henderson
2024-05-24 14:23 ` Richard Henderson
1 sibling, 0 replies; 38+ messages in thread
From: Richard Henderson @ 2024-05-24 14:20 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
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<pbonzini@redhat.com>
> ---
> target/i386/tcg/translate.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 8:10 ` [PATCH 03/16] target/i386: document and group DISAS_* constants Paolo Bonzini
2024-05-24 14:20 ` Richard Henderson
@ 2024-05-24 14:23 ` Richard Henderson
2024-05-24 15:02 ` Paolo Bonzini
1 sibling, 1 reply; 38+ messages in thread
From: Richard Henderson @ 2024-05-24 14:23 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
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 <pbonzini@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).
Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 14:23 ` Richard Henderson
@ 2024-05-24 15:02 ` Paolo Bonzini
2024-05-24 15:04 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 15:02 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, May 24, 2024 at 4:23 PM Richard Henderson
<richard.henderson@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 <pbonzini@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
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 15:02 ` Paolo Bonzini
@ 2024-05-24 15:04 ` Paolo Bonzini
2024-05-24 15:13 ` Richard Henderson
0 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 15:04 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, May 24, 2024 at 5:02 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Fri, May 24, 2024 at 4:23 PM Richard Henderson
> <richard.henderson@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 <pbonzini@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.
... and nope, it's the other way round - DISAS_NORETURN is a bug
waiting to happen for x86 translation because it doesn't process any
of HF_INHIBIT_IRQ_MASK, HF_RF_MASK or HF_TF_MASK.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 15:04 ` Paolo Bonzini
@ 2024-05-24 15:13 ` Richard Henderson
2024-05-24 15:18 ` Paolo Bonzini
0 siblings, 1 reply; 38+ messages in thread
From: Richard Henderson @ 2024-05-24 15:13 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On 5/24/24 08:04, Paolo Bonzini wrote:
> ... and nope, it's the other way round - DISAS_NORETURN is a bug
> waiting to happen for x86 translation because it doesn't process any
> of HF_INHIBIT_IRQ_MASK, HF_RF_MASK or HF_TF_MASK.
Do you need to suppress use_goto_tb in these cases?
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 03/16] target/i386: document and group DISAS_* constants
2024-05-24 15:13 ` Richard Henderson
@ 2024-05-24 15:18 ` Paolo Bonzini
0 siblings, 0 replies; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 15:18 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, May 24, 2024 at 5:13 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/24/24 08:04, Paolo Bonzini wrote:
> > ... and nope, it's the other way round - DISAS_NORETURN is a bug
> > waiting to happen for x86 translation because it doesn't process any
> > of HF_INHIBIT_IRQ_MASK, HF_RF_MASK or HF_TF_MASK.
>
> Do you need to suppress use_goto_tb in these cases?
HF_INHIBIT_IRQ_MASK and HF_TF_MASK already do, HF_RF_MASK is missing.
Nice catch.
Paolo
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 04/16] target/i386: avoid calling gen_eob_syscall before tb_stop
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (2 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 03/16] target/i386: document and group DISAS_* constants Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:27 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 05/16] target/i386: avoid calling gen_eob_inhibit_irq " Paolo Bonzini
` (11 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
syscall and sysret only have one exit, so they do not need to
generate the end-of-translation code inline. It can be
deferred to tb_stop.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 52d758a224b..24e83c1af84 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -168,6 +168,12 @@ typedef struct DisasContext {
*/
#define DISAS_JUMP DISAS_TARGET_3
+/*
+ * EIP has already been updated. Use updated value of
+ * EFLAGS.TF to determine singlestep trap (SYSCALL/SYSRET).
+ */
+#define DISAS_EOB_RECHECK_TF DISAS_TARGET_4
+
/* The environment in which user-only runs is constrained. */
#ifdef CONFIG_USER_ONLY
#define PE(S) true
@@ -3576,7 +3582,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
/* TF handling for the syscall insn is different. The TF bit is checked
after the syscall insn completes. This allows #DB to not be
generated after one has entered CPL0 if TF is set in FMASK. */
- gen_eob_syscall(s);
+ s->base.is_jmp = DISAS_EOB_RECHECK_TF;
break;
case 0x107: /* sysret */
/* For Intel SYSRET is only valid in long mode */
@@ -3595,7 +3601,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
checked after the sysret insn completes. This allows #DB to be
generated "as if" the syscall insn in userspace has just
completed. */
- gen_eob_syscall(s);
+ s->base.is_jmp = DISAS_EOB_RECHECK_TF;
}
break;
case 0x1a2: /* cpuid */
@@ -4799,6 +4805,9 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
case DISAS_EOB_ONLY:
gen_eob(dc);
break;
+ case DISAS_EOB_RECHECK_TF:
+ gen_eob_syscall(dc);
+ break;
case DISAS_EOB_INHIBIT_IRQ:
gen_update_eip_cur(dc);
gen_eob_inhibit_irq(dc);
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 05/16] target/i386: avoid calling gen_eob_inhibit_irq before tb_stop
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (3 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 04/16] target/i386: avoid calling gen_eob_syscall before tb_stop Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:29 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 06/16] target/i386: assert that gen_update_eip_cur and gen_update_eip_next are the same in tb_stop Paolo Bonzini
` (10 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
sti only has one exit, so it does not need to generate the
end-of-translation code inline. It can be deferred to tb_stop.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 13 -------------
target/i386/tcg/emit.c.inc | 4 +---
2 files changed, 1 insertion(+), 16 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 24e83c1af84..5dae890d2b6 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -557,19 +557,6 @@ static void gen_update_eip_cur(DisasContext *s)
s->pc_save = s->base.pc_next;
}
-static void gen_update_eip_next(DisasContext *s)
-{
- assert(s->pc_save != -1);
- if (tb_cflags(s->base.tb) & CF_PCREL) {
- tcg_gen_addi_tl(cpu_eip, cpu_eip, s->pc - s->pc_save);
- } else if (CODE64(s)) {
- tcg_gen_movi_tl(cpu_eip, s->pc);
- } else {
- tcg_gen_movi_tl(cpu_eip, (uint32_t)(s->pc - s->cs_base));
- }
- s->pc_save = s->pc;
-}
-
static int cur_insn_len(DisasContext *s)
{
return s->pc - s->base.pc_next;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index c78e35b1e28..8e311b6d213 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -3475,9 +3475,7 @@ static void gen_STD(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
static void gen_STI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
gen_set_eflags(s, IF_MASK);
- /* interruptions are enabled only the first insn after sti */
- gen_update_eip_next(s);
- gen_eob_inhibit_irq(s);
+ s->base.is_jmp = DISAS_EOB_INHIBIT_IRQ;
}
static void gen_VAESKEYGEN(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 06/16] target/i386: assert that gen_update_eip_cur and gen_update_eip_next are the same in tb_stop
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (4 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 05/16] target/i386: avoid calling gen_eob_inhibit_irq " Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:30 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 07/16] target/i386: raze the gen_eob* jungle Paolo Bonzini
` (9 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
This is an invariant, since these cases of tb_stop() should only
be reached through the "instruction decoding completed" path of
i386_tr_translate_insn().
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 5dae890d2b6..2c7917d239f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -4787,6 +4787,7 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_jmp_rel_csize(dc, 0, 0);
break;
case DISAS_EOB_NEXT:
+ assert(dc->base.pc_next == dc->pc);
gen_update_eip_cur(dc);
/* fall through */
case DISAS_EOB_ONLY:
@@ -4796,6 +4797,7 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_eob_syscall(dc);
break;
case DISAS_EOB_INHIBIT_IRQ:
+ assert(dc->base.pc_next == dc->pc);
gen_update_eip_cur(dc);
gen_eob_inhibit_irq(dc);
break;
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 07/16] target/i386: raze the gen_eob* jungle
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (5 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 06/16] target/i386: assert that gen_update_eip_cur and gen_update_eip_next are the same in tb_stop Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:32 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 08/16] target/i386: reg in gen_ldst_modrm is always OR_TMP0 Paolo Bonzini
` (8 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
Make gen_eob take the DISAS_* constant as an argument, so that
it is not necessary to have wrappers around it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 60 +++++++++----------------------------
1 file changed, 14 insertions(+), 46 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 2c7917d239f..c46385be060 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -260,8 +260,6 @@ STUB_HELPER(write_crN, TCGv_env env, TCGv_i32 reg, TCGv val)
STUB_HELPER(wrmsr, TCGv_env env)
#endif
-static void gen_eob(DisasContext *s);
-static void gen_jr(DisasContext *s);
static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num);
static void gen_jmp_rel_csize(DisasContext *s, int diff, int tb_num);
static void gen_exception_gpf(DisasContext *s);
@@ -2259,12 +2257,13 @@ static void gen_bnd_jmp(DisasContext *s)
}
}
-/* Generate an end of block. Trace exception is also generated if needed.
- If INHIBIT, set HF_INHIBIT_IRQ_MASK if it isn't already set.
- If RECHECK_TF, emit a rechecking helper for #DB, ignoring the state of
- S->TF. This is used by the syscall/sysret insns. */
+/*
+ * Generate an end of block, including common tasks such as generating
+ * single step traps, resetting the RF flag, and handling the interrupt
+ * shadow.
+ */
static void
-gen_eob_worker(DisasContext *s, bool inhibit, bool recheck_tf, bool jr)
+gen_eob(DisasContext *s, int mode)
{
bool inhibit_reset;
@@ -2275,52 +2274,29 @@ gen_eob_worker(DisasContext *s, bool inhibit, bool recheck_tf, bool jr)
if (s->flags & HF_INHIBIT_IRQ_MASK) {
gen_reset_hflag(s, HF_INHIBIT_IRQ_MASK);
inhibit_reset = true;
- } else if (inhibit) {
+ } else if (mode == DISAS_EOB_INHIBIT_IRQ) {
gen_set_hflag(s, HF_INHIBIT_IRQ_MASK);
}
if (s->base.tb->flags & HF_RF_MASK) {
gen_reset_eflags(s, RF_MASK);
}
- if (recheck_tf) {
+ if (mode == DISAS_EOB_RECHECK_TF) {
gen_helper_rechecking_single_step(tcg_env);
tcg_gen_exit_tb(NULL, 0);
} else if (s->flags & HF_TF_MASK) {
gen_helper_single_step(tcg_env);
- } else if (jr &&
+ } else if (mode == DISAS_JUMP &&
/* give irqs a chance to happen */
!inhibit_reset) {
tcg_gen_lookup_and_goto_ptr();
} else {
tcg_gen_exit_tb(NULL, 0);
}
+
s->base.is_jmp = DISAS_NORETURN;
}
-static inline void
-gen_eob_syscall(DisasContext *s)
-{
- gen_eob_worker(s, false, true, false);
-}
-
-/* End of block. Set HF_INHIBIT_IRQ_MASK if it isn't already set. */
-static void gen_eob_inhibit_irq(DisasContext *s)
-{
- gen_eob_worker(s, true, false, false);
-}
-
-/* End of block, resetting the inhibit irq flag. */
-static void gen_eob(DisasContext *s)
-{
- gen_eob_worker(s, false, false, false);
-}
-
-/* Jump to register */
-static void gen_jr(DisasContext *s)
-{
- gen_eob_worker(s, false, false, true);
-}
-
/* Jump to eip+diff, truncating the result to OT. */
static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num)
{
@@ -2372,9 +2348,9 @@ static void gen_jmp_rel(DisasContext *s, MemOp ot, int diff, int tb_num)
tcg_gen_movi_tl(cpu_eip, new_eip);
}
if (s->jmp_opt) {
- gen_jr(s); /* jump to another page */
+ gen_eob(s, DISAS_JUMP); /* jump to another page */
} else {
- gen_eob(s); /* exit to main loop */
+ gen_eob(s, DISAS_EOB_ONLY); /* exit to main loop */
}
}
}
@@ -4787,22 +4763,14 @@ static void i386_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
gen_jmp_rel_csize(dc, 0, 0);
break;
case DISAS_EOB_NEXT:
+ case DISAS_EOB_INHIBIT_IRQ:
assert(dc->base.pc_next == dc->pc);
gen_update_eip_cur(dc);
/* fall through */
case DISAS_EOB_ONLY:
- gen_eob(dc);
- break;
case DISAS_EOB_RECHECK_TF:
- gen_eob_syscall(dc);
- break;
- case DISAS_EOB_INHIBIT_IRQ:
- assert(dc->base.pc_next == dc->pc);
- gen_update_eip_cur(dc);
- gen_eob_inhibit_irq(dc);
- break;
case DISAS_JUMP:
- gen_jr(dc);
+ gen_eob(dc, dc->base.is_jmp);
break;
default:
g_assert_not_reached();
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 08/16] target/i386: reg in gen_ldst_modrm is always OR_TMP0
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (6 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 07/16] target/i386: raze the gen_eob* jungle Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:33 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 09/16] target/i386: split gen_ldst_modrm for load and store Paolo Bonzini
` (7 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
Values other than OR_TMP0 were only ever used by MOV and MOVNTI
opcodes. Now that these have been converted to the new decoder,
remove the argument.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index c46385be060..b75d61a9141 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1821,10 +1821,9 @@ static void gen_add_A0_ds_seg(DisasContext *s)
gen_lea_v_seg(s, s->aflag, s->A0, R_DS, s->override);
}
-/* generate modrm memory load or store of 'reg'. TMP0 is used if reg ==
- OR_TMP0 */
+/* generate modrm memory load or store of 'reg'. */
static void gen_ldst_modrm(CPUX86State *env, DisasContext *s, int modrm,
- MemOp ot, int reg, int is_store)
+ MemOp ot, int is_store)
{
int mod, rm;
@@ -1832,24 +1831,16 @@ static void gen_ldst_modrm(CPUX86State *env, DisasContext *s, int modrm,
rm = (modrm & 7) | REX_B(s);
if (mod == 3) {
if (is_store) {
- if (reg != OR_TMP0)
- gen_op_mov_v_reg(s, ot, s->T0, reg);
gen_op_mov_reg_v(s, ot, rm, s->T0);
} else {
gen_op_mov_v_reg(s, ot, s->T0, rm);
- if (reg != OR_TMP0)
- gen_op_mov_reg_v(s, ot, reg, s->T0);
}
} else {
gen_lea_modrm(env, s, modrm);
if (is_store) {
- if (reg != OR_TMP0)
- gen_op_mov_v_reg(s, ot, s->T0, reg);
gen_op_st_v(s, ot, s->T0, s->A0);
} else {
gen_op_ld_v(s, ot, s->T0, s->A0);
- if (reg != OR_TMP0)
- gen_op_mov_reg_v(s, ot, reg, s->T0);
}
}
}
@@ -3440,7 +3431,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
ot = dflag;
modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | REX_R(s);
- gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, ot, 0);
gen_extu(ot, s->T0);
/* Note that lzcnt and tzcnt are in different extensions. */
@@ -3587,14 +3578,14 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
tcg_gen_ld32u_tl(s->T0, tcg_env,
offsetof(CPUX86State, ldt.selector));
ot = mod == 3 ? dflag : MO_16;
- gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 1);
+ gen_ldst_modrm(env, s, modrm, ot, 1);
break;
case 2: /* lldt */
if (!PE(s) || VM86(s))
goto illegal_op;
if (check_cpl0(s)) {
gen_svm_check_intercept(s, SVM_EXIT_LDTR_WRITE);
- gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, MO_16, 0);
tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
gen_helper_lldt(tcg_env, s->tmp2_i32);
}
@@ -3609,14 +3600,14 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
tcg_gen_ld32u_tl(s->T0, tcg_env,
offsetof(CPUX86State, tr.selector));
ot = mod == 3 ? dflag : MO_16;
- gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 1);
+ gen_ldst_modrm(env, s, modrm, ot, 1);
break;
case 3: /* ltr */
if (!PE(s) || VM86(s))
goto illegal_op;
if (check_cpl0(s)) {
gen_svm_check_intercept(s, SVM_EXIT_TR_WRITE);
- gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, MO_16, 0);
tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
gen_helper_ltr(tcg_env, s->tmp2_i32);
}
@@ -3625,7 +3616,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
case 5: /* verw */
if (!PE(s) || VM86(s))
goto illegal_op;
- gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, MO_16, 0);
gen_update_cc_op(s);
if (op == 4) {
gen_helper_verr(tcg_env, s->T0);
@@ -3889,7 +3880,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
*/
mod = (modrm >> 6) & 3;
ot = (mod != 3 ? MO_16 : s->dflag);
- gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 1);
+ gen_ldst_modrm(env, s, modrm, ot, 1);
break;
case 0xee: /* rdpkru */
if (s->prefix & (PREFIX_LOCK | PREFIX_DATA
@@ -3916,7 +3907,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
break;
}
gen_svm_check_intercept(s, SVM_EXIT_WRITE_CR0);
- gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, MO_16, 0);
/*
* Only the 4 lower bits of CR0 are modified.
* PE cannot be set to zero if already set to one.
@@ -3988,7 +3979,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
ot = dflag != MO_16 ? MO_32 : MO_16;
modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | REX_R(s);
- gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, MO_16, 0);
t0 = tcg_temp_new();
gen_update_cc_op(s);
if (b == 0x102) {
@@ -4492,7 +4483,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
reg = ((modrm >> 3) & 7) | REX_R(s);
ot = dflag;
- gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
+ gen_ldst_modrm(env, s, modrm, ot, 0);
gen_extu(ot, s->T0);
tcg_gen_mov_tl(cpu_cc_src, s->T0);
tcg_gen_ctpop_tl(s->T0, s->T0);
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 09/16] target/i386: split gen_ldst_modrm for load and store
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (7 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 08/16] target/i386: reg in gen_ldst_modrm is always OR_TMP0 Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:34 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 10/16] target/i386: inline gen_add_A0_ds_seg Paolo Bonzini
` (6 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
The is_store argument of gen_ldst_modrm has only ever been passed
a constant. Just split the function in two.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 52 +++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index b75d61a9141..d32b5b63f5c 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1821,27 +1821,33 @@ static void gen_add_A0_ds_seg(DisasContext *s)
gen_lea_v_seg(s, s->aflag, s->A0, R_DS, s->override);
}
-/* generate modrm memory load or store of 'reg'. */
-static void gen_ldst_modrm(CPUX86State *env, DisasContext *s, int modrm,
- MemOp ot, int is_store)
+/* generate modrm load of memory or register. */
+static void gen_ld_modrm(CPUX86State *env, DisasContext *s, int modrm, MemOp ot)
{
int mod, rm;
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
if (mod == 3) {
- if (is_store) {
- gen_op_mov_reg_v(s, ot, rm, s->T0);
- } else {
- gen_op_mov_v_reg(s, ot, s->T0, rm);
- }
+ gen_op_mov_v_reg(s, ot, s->T0, rm);
} else {
gen_lea_modrm(env, s, modrm);
- if (is_store) {
- gen_op_st_v(s, ot, s->T0, s->A0);
- } else {
- gen_op_ld_v(s, ot, s->T0, s->A0);
- }
+ gen_op_ld_v(s, ot, s->T0, s->A0);
+ }
+}
+
+/* generate modrm store of memory or register. */
+static void gen_st_modrm(CPUX86State *env, DisasContext *s, int modrm, MemOp ot)
+{
+ int mod, rm;
+
+ mod = (modrm >> 6) & 3;
+ rm = (modrm & 7) | REX_B(s);
+ if (mod == 3) {
+ gen_op_mov_reg_v(s, ot, rm, s->T0);
+ } else {
+ gen_lea_modrm(env, s, modrm);
+ gen_op_st_v(s, ot, s->T0, s->A0);
}
}
@@ -3431,7 +3437,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
ot = dflag;
modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | REX_R(s);
- gen_ldst_modrm(env, s, modrm, ot, 0);
+ gen_ld_modrm(env, s, modrm, ot);
gen_extu(ot, s->T0);
/* Note that lzcnt and tzcnt are in different extensions. */
@@ -3578,14 +3584,14 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
tcg_gen_ld32u_tl(s->T0, tcg_env,
offsetof(CPUX86State, ldt.selector));
ot = mod == 3 ? dflag : MO_16;
- gen_ldst_modrm(env, s, modrm, ot, 1);
+ gen_st_modrm(env, s, modrm, ot);
break;
case 2: /* lldt */
if (!PE(s) || VM86(s))
goto illegal_op;
if (check_cpl0(s)) {
gen_svm_check_intercept(s, SVM_EXIT_LDTR_WRITE);
- gen_ldst_modrm(env, s, modrm, MO_16, 0);
+ gen_ld_modrm(env, s, modrm, MO_16);
tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
gen_helper_lldt(tcg_env, s->tmp2_i32);
}
@@ -3600,14 +3606,14 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
tcg_gen_ld32u_tl(s->T0, tcg_env,
offsetof(CPUX86State, tr.selector));
ot = mod == 3 ? dflag : MO_16;
- gen_ldst_modrm(env, s, modrm, ot, 1);
+ gen_st_modrm(env, s, modrm, ot);
break;
case 3: /* ltr */
if (!PE(s) || VM86(s))
goto illegal_op;
if (check_cpl0(s)) {
gen_svm_check_intercept(s, SVM_EXIT_TR_WRITE);
- gen_ldst_modrm(env, s, modrm, MO_16, 0);
+ gen_ld_modrm(env, s, modrm, MO_16);
tcg_gen_trunc_tl_i32(s->tmp2_i32, s->T0);
gen_helper_ltr(tcg_env, s->tmp2_i32);
}
@@ -3616,7 +3622,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
case 5: /* verw */
if (!PE(s) || VM86(s))
goto illegal_op;
- gen_ldst_modrm(env, s, modrm, MO_16, 0);
+ gen_ld_modrm(env, s, modrm, MO_16);
gen_update_cc_op(s);
if (op == 4) {
gen_helper_verr(tcg_env, s->T0);
@@ -3880,7 +3886,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
*/
mod = (modrm >> 6) & 3;
ot = (mod != 3 ? MO_16 : s->dflag);
- gen_ldst_modrm(env, s, modrm, ot, 1);
+ gen_st_modrm(env, s, modrm, ot);
break;
case 0xee: /* rdpkru */
if (s->prefix & (PREFIX_LOCK | PREFIX_DATA
@@ -3907,7 +3913,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
break;
}
gen_svm_check_intercept(s, SVM_EXIT_WRITE_CR0);
- gen_ldst_modrm(env, s, modrm, MO_16, 0);
+ gen_ld_modrm(env, s, modrm, MO_16);
/*
* Only the 4 lower bits of CR0 are modified.
* PE cannot be set to zero if already set to one.
@@ -3979,7 +3985,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
ot = dflag != MO_16 ? MO_32 : MO_16;
modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | REX_R(s);
- gen_ldst_modrm(env, s, modrm, MO_16, 0);
+ gen_ld_modrm(env, s, modrm, MO_16);
t0 = tcg_temp_new();
gen_update_cc_op(s);
if (b == 0x102) {
@@ -4483,7 +4489,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
reg = ((modrm >> 3) & 7) | REX_R(s);
ot = dflag;
- gen_ldst_modrm(env, s, modrm, ot, 0);
+ gen_ld_modrm(env, s, modrm, ot);
gen_extu(ot, s->T0);
tcg_gen_mov_tl(cpu_cc_src, s->T0);
tcg_gen_ctpop_tl(s->T0, s->T0);
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 10/16] target/i386: inline gen_add_A0_ds_seg
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (8 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 09/16] target/i386: split gen_ldst_modrm for load and store Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:35 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 11/16] target/i386: use mo_stacksize more Paolo Bonzini
` (5 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
It is only used in MONITOR, where a direct call of gen_lea_v_seg
is simpler, and in XLAT. Inline it in the latter.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 9 +--------
target/i386/tcg/emit.c.inc | 2 +-
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index d32b5b63f5c..8138da23b3d 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1815,12 +1815,6 @@ static void gen_bndck(CPUX86State *env, DisasContext *s, int modrm,
gen_helper_bndck(tcg_env, s->tmp2_i32);
}
-/* used for LEA and MOV AX, mem */
-static void gen_add_A0_ds_seg(DisasContext *s)
-{
- gen_lea_v_seg(s, s->aflag, s->A0, R_DS, s->override);
-}
-
/* generate modrm load of memory or register. */
static void gen_ld_modrm(CPUX86State *env, DisasContext *s, int modrm, MemOp ot)
{
@@ -3663,8 +3657,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
}
gen_update_cc_op(s);
gen_update_eip_cur(s);
- tcg_gen_mov_tl(s->A0, cpu_regs[R_EAX]);
- gen_add_A0_ds_seg(s);
+ gen_lea_v_seg(s, s->aflag, cpu_regs[R_EAX], R_DS, s->override);
gen_helper_monitor(tcg_env, s->A0);
break;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 8e311b6d213..f293db01b5c 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -4043,7 +4043,7 @@ static void gen_XLAT(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
/* AL is already zero-extended into s->T0. */
tcg_gen_add_tl(s->A0, cpu_regs[R_EBX], s->T0);
- gen_add_A0_ds_seg(s);
+ gen_lea_v_seg(s, s->aflag, s->A0, R_DS, s->override);
gen_op_ld_v(s, MO_8, s->T0, s->A0);
}
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 11/16] target/i386: use mo_stacksize more
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (9 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 10/16] target/i386: inline gen_add_A0_ds_seg Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:36 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 12/16] target/i386: introduce gen_lea_ss_ofs Paolo Bonzini
` (4 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
Use mo_stacksize for all stack accesses, including when
a 64-bit code segment is impossible and the code is
therefore checking only for SS32(s).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 8138da23b3d..7b6bc486a63 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2068,12 +2068,12 @@ static inline void gen_pop_update(DisasContext *s, MemOp ot)
static inline void gen_stack_A0(DisasContext *s)
{
- gen_lea_v_seg(s, SS32(s) ? MO_32 : MO_16, cpu_regs[R_ESP], R_SS, -1);
+ gen_lea_v_seg(s, mo_stacksize(s), cpu_regs[R_ESP], R_SS, -1);
}
static void gen_pusha(DisasContext *s)
{
- MemOp s_ot = SS32(s) ? MO_32 : MO_16;
+ MemOp s_ot = mo_stacksize(s);
MemOp d_ot = s->dflag;
int size = 1 << d_ot;
int i;
@@ -2089,7 +2089,7 @@ static void gen_pusha(DisasContext *s)
static void gen_popa(DisasContext *s)
{
- MemOp s_ot = SS32(s) ? MO_32 : MO_16;
+ MemOp s_ot = mo_stacksize(s);
MemOp d_ot = s->dflag;
int size = 1 << d_ot;
int i;
@@ -2111,7 +2111,7 @@ static void gen_popa(DisasContext *s)
static void gen_enter(DisasContext *s, int esp_addend, int level)
{
MemOp d_ot = mo_pushpop(s, s->dflag);
- MemOp a_ot = CODE64(s) ? MO_64 : SS32(s) ? MO_32 : MO_16;
+ MemOp a_ot = mo_stacksize(s);
int size = 1 << d_ot;
/* Push BP; compute FrameTemp into T1. */
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 12/16] target/i386: introduce gen_lea_ss_ofs
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (10 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 11/16] target/i386: use mo_stacksize more Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:53 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 13/16] target/i386: clean up repeated string operations Paolo Bonzini
` (3 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
Generalize gen_stack_A0() to include an initial add and to use an arbitrary
destination. This is a common pattern and it is not a huge burden to
add the extra arguments to the only caller of gen_stack_A0().
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 51 +++++++++++++++----------------------
target/i386/tcg/emit.c.inc | 2 +-
2 files changed, 22 insertions(+), 31 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 7b6bc486a63..8354209b037 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2028,24 +2028,27 @@ static inline void gen_stack_update(DisasContext *s, int addend)
gen_op_add_reg_im(s, mo_stacksize(s), R_ESP, addend);
}
+static void gen_lea_ss_ofs(DisasContext *s, TCGv dest, TCGv src, target_ulong offset)
+{
+ if (offset) {
+ tcg_gen_addi_tl(dest, src, offset);
+ src = dest;
+ }
+ gen_lea_v_seg_dest(s, mo_stacksize(s), dest, src, R_SS, -1);
+}
+
/* Generate a push. It depends on ss32, addseg and dflag. */
static void gen_push_v(DisasContext *s, TCGv val)
{
MemOp d_ot = mo_pushpop(s, s->dflag);
MemOp a_ot = mo_stacksize(s);
int size = 1 << d_ot;
- TCGv new_esp = s->A0;
+ TCGv new_esp = tcg_temp_new();
- tcg_gen_subi_tl(s->A0, cpu_regs[R_ESP], size);
-
- if (!CODE64(s)) {
- if (ADDSEG(s)) {
- new_esp = tcg_temp_new();
- tcg_gen_mov_tl(new_esp, s->A0);
- }
- gen_lea_v_seg(s, a_ot, s->A0, R_SS, -1);
- }
+ tcg_gen_subi_tl(new_esp, cpu_regs[R_ESP], size);
+ /* Now reduce the value to the address size and apply SS base. */
+ gen_lea_ss_ofs(s, s->A0, new_esp, 0);
gen_op_st_v(s, d_ot, val, s->A0);
gen_op_mov_reg_v(s, a_ot, R_ESP, new_esp);
}
@@ -2055,7 +2058,7 @@ static MemOp gen_pop_T0(DisasContext *s)
{
MemOp d_ot = mo_pushpop(s, s->dflag);
- gen_lea_v_seg_dest(s, mo_stacksize(s), s->T0, cpu_regs[R_ESP], R_SS, -1);
+ gen_lea_ss_ofs(s, s->T0, cpu_regs[R_ESP], 0);
gen_op_ld_v(s, d_ot, s->T0, s->T0);
return d_ot;
@@ -2066,21 +2069,14 @@ static inline void gen_pop_update(DisasContext *s, MemOp ot)
gen_stack_update(s, 1 << ot);
}
-static inline void gen_stack_A0(DisasContext *s)
-{
- gen_lea_v_seg(s, mo_stacksize(s), cpu_regs[R_ESP], R_SS, -1);
-}
-
static void gen_pusha(DisasContext *s)
{
- MemOp s_ot = mo_stacksize(s);
MemOp d_ot = s->dflag;
int size = 1 << d_ot;
int i;
for (i = 0; i < 8; i++) {
- tcg_gen_addi_tl(s->A0, cpu_regs[R_ESP], (i - 8) * size);
- gen_lea_v_seg(s, s_ot, s->A0, R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, cpu_regs[R_ESP], (i - 8) * size);
gen_op_st_v(s, d_ot, cpu_regs[7 - i], s->A0);
}
@@ -2089,7 +2085,6 @@ static void gen_pusha(DisasContext *s)
static void gen_popa(DisasContext *s)
{
- MemOp s_ot = mo_stacksize(s);
MemOp d_ot = s->dflag;
int size = 1 << d_ot;
int i;
@@ -2099,8 +2094,7 @@ static void gen_popa(DisasContext *s)
if (7 - i == R_ESP) {
continue;
}
- tcg_gen_addi_tl(s->A0, cpu_regs[R_ESP], i * size);
- gen_lea_v_seg(s, s_ot, s->A0, R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, cpu_regs[R_ESP], i * size);
gen_op_ld_v(s, d_ot, s->T0, s->A0);
gen_op_mov_reg_v(s, d_ot, 7 - i, s->T0);
}
@@ -2116,7 +2110,7 @@ static void gen_enter(DisasContext *s, int esp_addend, int level)
/* Push BP; compute FrameTemp into T1. */
tcg_gen_subi_tl(s->T1, cpu_regs[R_ESP], size);
- gen_lea_v_seg(s, a_ot, s->T1, R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, s->T1, 0);
gen_op_st_v(s, d_ot, cpu_regs[R_EBP], s->A0);
level &= 31;
@@ -2125,18 +2119,15 @@ static void gen_enter(DisasContext *s, int esp_addend, int level)
/* Copy level-1 pointers from the previous frame. */
for (i = 1; i < level; ++i) {
- tcg_gen_subi_tl(s->A0, cpu_regs[R_EBP], size * i);
- gen_lea_v_seg(s, a_ot, s->A0, R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, cpu_regs[R_EBP], -size * i);
gen_op_ld_v(s, d_ot, s->tmp0, s->A0);
- tcg_gen_subi_tl(s->A0, s->T1, size * i);
- gen_lea_v_seg(s, a_ot, s->A0, R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, s->T1, -size * i);
gen_op_st_v(s, d_ot, s->tmp0, s->A0);
}
/* Push the current FrameTemp as the last level. */
- tcg_gen_subi_tl(s->A0, s->T1, size * level);
- gen_lea_v_seg(s, a_ot, s->A0, R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, s->T1, -size * level);
gen_op_st_v(s, d_ot, s->T1, s->A0);
}
@@ -2153,7 +2144,7 @@ static void gen_leave(DisasContext *s)
MemOp d_ot = mo_pushpop(s, s->dflag);
MemOp a_ot = mo_stacksize(s);
- gen_lea_v_seg(s, a_ot, cpu_regs[R_EBP], R_SS, -1);
+ gen_lea_ss_ofs(s, s->A0, cpu_regs[R_EBP], 0);
gen_op_ld_v(s, d_ot, s->T0, s->A0);
tcg_gen_addi_tl(s->T1, cpu_regs[R_EBP], 1 << d_ot);
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index f293db01b5c..83fa745fd8a 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -3077,7 +3077,7 @@ static void gen_RETF(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
int16_t adjust = decode->e.op2 == X86_TYPE_I ? decode->immediate : 0;
if (!PE(s) || VM86(s)) {
- gen_stack_A0(s);
+ gen_lea_ss_ofs(s, s->A0, cpu_regs[R_ESP], 0);
/* pop offset */
gen_op_ld_v(s, s->dflag, s->T0, s->A0);
/* NOTE: keeping EIP updated is not a problem in case of
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 12/16] target/i386: introduce gen_lea_ss_ofs
2024-05-24 8:10 ` [PATCH 12/16] target/i386: introduce gen_lea_ss_ofs Paolo Bonzini
@ 2024-05-24 14:53 ` Richard Henderson
0 siblings, 0 replies; 38+ messages in thread
From: Richard Henderson @ 2024-05-24 14:53 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 5/24/24 01:10, Paolo Bonzini wrote:
> Generalize gen_stack_A0() to include an initial add and to use an arbitrary
> destination. This is a common pattern and it is not a huge burden to
> add the extra arguments to the only caller of gen_stack_A0().
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
> target/i386/tcg/translate.c | 51 +++++++++++++++----------------------
> target/i386/tcg/emit.c.inc | 2 +-
> 2 files changed, 22 insertions(+), 31 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 13/16] target/i386: clean up repeated string operations
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (11 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 12/16] target/i386: introduce gen_lea_ss_ofs Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:58 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 14/16] target/i386: remove aflag argument of gen_lea_v_seg Paolo Bonzini
` (2 subsequent siblings)
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
Do not bother generating inline wrappers for gen_repz and gen_repz2;
use s->prefix to separate REPZ from REPNZ in the case of SCAS and
CMPS.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 22 ++++------------------
target/i386/tcg/emit.c.inc | 22 +++++++++-------------
2 files changed, 13 insertions(+), 31 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 8354209b037..18d8c0de674 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1320,14 +1320,12 @@ static void gen_repz(DisasContext *s, MemOp ot,
gen_jmp_rel_csize(s, -cur_insn_len(s), 0);
}
-#define GEN_REPZ(op) \
- static inline void gen_repz_ ## op(DisasContext *s, MemOp ot) \
- { gen_repz(s, ot, gen_##op); }
-
-static void gen_repz2(DisasContext *s, MemOp ot, int nz,
- void (*fn)(DisasContext *s, MemOp ot))
+static void gen_repz_nz(DisasContext *s, MemOp ot,
+ void (*fn)(DisasContext *s, MemOp ot))
{
TCGLabel *l2;
+ int nz = (s->prefix & PREFIX_REPNZ) ? 1 : 0;
+
l2 = gen_jz_ecx_string(s);
fn(s, ot);
gen_op_add_reg_im(s, s->aflag, R_ECX, -1);
@@ -1343,18 +1341,6 @@ static void gen_repz2(DisasContext *s, MemOp ot, int nz,
gen_jmp_rel_csize(s, -cur_insn_len(s), 0);
}
-#define GEN_REPZ2(op) \
- static inline void gen_repz_ ## op(DisasContext *s, MemOp ot, int nz) \
- { gen_repz2(s, ot, nz, gen_##op); }
-
-GEN_REPZ(movs)
-GEN_REPZ(stos)
-GEN_REPZ(lods)
-GEN_REPZ(ins)
-GEN_REPZ(outs)
-GEN_REPZ2(scas)
-GEN_REPZ2(cmps)
-
static void gen_helper_fp_arith_ST0_FT0(int op)
{
switch (op) {
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 83fa745fd8a..bc96735f61d 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1508,10 +1508,8 @@ static void gen_CMPccXADD(DisasContext *s, CPUX86State *env, X86DecodedInsn *dec
static void gen_CMPS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
MemOp ot = decode->op[2].ot;
- if (s->prefix & PREFIX_REPNZ) {
- gen_repz_cmps(s, ot, 1);
- } else if (s->prefix & PREFIX_REPZ) {
- gen_repz_cmps(s, ot, 0);
+ if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
+ gen_repz_nz(s, ot, gen_cmps);
} else {
gen_cmps(s, ot);
}
@@ -1834,7 +1832,7 @@ static void gen_INS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
translator_io_start(&s->base);
if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
- gen_repz_ins(s, ot);
+ gen_repz(s, ot, gen_ins);
} else {
gen_ins(s, ot);
}
@@ -1993,7 +1991,7 @@ static void gen_LODS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
MemOp ot = decode->op[2].ot;
if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
- gen_repz_lods(s, ot);
+ gen_repz(s, ot, gen_lods);
} else {
gen_lods(s, ot);
}
@@ -2155,7 +2153,7 @@ static void gen_MOVS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
MemOp ot = decode->op[2].ot;
if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
- gen_repz_movs(s, ot);
+ gen_repz(s, ot, gen_movs);
} else {
gen_movs(s, ot);
}
@@ -2321,7 +2319,7 @@ static void gen_OUTS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
translator_io_start(&s->base);
if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
- gen_repz_outs(s, ot);
+ gen_repz(s, ot, gen_outs);
} else {
gen_outs(s, ot);
}
@@ -3329,10 +3327,8 @@ static void gen_SBB(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
static void gen_SCAS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
MemOp ot = decode->op[2].ot;
- if (s->prefix & PREFIX_REPNZ) {
- gen_repz_scas(s, ot, 1);
- } else if (s->prefix & PREFIX_REPZ) {
- gen_repz_scas(s, ot, 0);
+ if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
+ gen_repz_nz(s, ot, gen_scas);
} else {
gen_scas(s, ot);
}
@@ -3495,7 +3491,7 @@ static void gen_STOS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
MemOp ot = decode->op[1].ot;
if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
- gen_repz_stos(s, ot);
+ gen_repz(s, ot, gen_stos);
} else {
gen_stos(s, ot);
}
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 14/16] target/i386: remove aflag argument of gen_lea_v_seg
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (12 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 13/16] target/i386: clean up repeated string operations Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 14:59 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 15/16] target/i386: cpu_load_eflags already sets cc_op Paolo Bonzini
2024-05-24 8:10 ` [PATCH 16/16] target/i386: set CC_OP in helpers if they want CC_OP_EFLAGS Paolo Bonzini
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
It is always s->aflag.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 20 ++++++++++----------
target/i386/tcg/emit.c.inc | 6 +++---
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 18d8c0de674..1a776e77297 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -673,20 +673,20 @@ static void gen_lea_v_seg_dest(DisasContext *s, MemOp aflag, TCGv dest, TCGv a0,
}
}
-static void gen_lea_v_seg(DisasContext *s, MemOp aflag, TCGv a0,
+static void gen_lea_v_seg(DisasContext *s, TCGv a0,
int def_seg, int ovr_seg)
{
- gen_lea_v_seg_dest(s, aflag, s->A0, a0, def_seg, ovr_seg);
+ gen_lea_v_seg_dest(s, s->aflag, s->A0, a0, def_seg, ovr_seg);
}
static inline void gen_string_movl_A0_ESI(DisasContext *s)
{
- gen_lea_v_seg(s, s->aflag, cpu_regs[R_ESI], R_DS, s->override);
+ gen_lea_v_seg(s, cpu_regs[R_ESI], R_DS, s->override);
}
static inline void gen_string_movl_A0_EDI(DisasContext *s)
{
- gen_lea_v_seg(s, s->aflag, cpu_regs[R_EDI], R_ES, -1);
+ gen_lea_v_seg(s, cpu_regs[R_EDI], R_ES, -1);
}
static inline TCGv gen_compute_Dshift(DisasContext *s, MemOp ot)
@@ -1777,7 +1777,7 @@ static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
{
AddressParts a = gen_lea_modrm_0(env, s, modrm);
TCGv ea = gen_lea_modrm_1(s, a, false);
- gen_lea_v_seg(s, s->aflag, ea, a.def_seg, s->override);
+ gen_lea_v_seg(s, ea, a.def_seg, s->override);
}
static void gen_nop_modrm(CPUX86State *env, DisasContext *s, int modrm)
@@ -2516,7 +2516,7 @@ static bool disas_insn_x87(DisasContext *s, CPUState *cpu, int b)
bool update_fdp = true;
tcg_gen_mov_tl(last_addr, ea);
- gen_lea_v_seg(s, s->aflag, ea, a.def_seg, s->override);
+ gen_lea_v_seg(s, ea, a.def_seg, s->override);
switch (op) {
case 0x00 ... 0x07: /* fxxxs */
@@ -3313,7 +3313,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
tcg_gen_sari_tl(s->tmp0, s->T1, 3 + ot);
tcg_gen_shli_tl(s->tmp0, s->tmp0, ot);
tcg_gen_add_tl(s->A0, gen_lea_modrm_1(s, a, false), s->tmp0);
- gen_lea_v_seg(s, s->aflag, s->A0, a.def_seg, s->override);
+ gen_lea_v_seg(s, s->A0, a.def_seg, s->override);
if (!(s->prefix & PREFIX_LOCK)) {
gen_op_ld_v(s, ot, s->T0, s->A0);
}
@@ -3634,7 +3634,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
}
gen_update_cc_op(s);
gen_update_eip_cur(s);
- gen_lea_v_seg(s, s->aflag, cpu_regs[R_EAX], R_DS, s->override);
+ gen_lea_v_seg(s, cpu_regs[R_EAX], R_DS, s->override);
gen_helper_monitor(tcg_env, s->A0);
break;
@@ -4040,7 +4040,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
} else {
tcg_gen_movi_tl(s->A0, 0);
}
- gen_lea_v_seg(s, s->aflag, s->A0, a.def_seg, s->override);
+ gen_lea_v_seg(s, s->A0, a.def_seg, s->override);
if (a.index >= 0) {
tcg_gen_mov_tl(s->T0, cpu_regs[a.index]);
} else {
@@ -4145,7 +4145,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
} else {
tcg_gen_movi_tl(s->A0, 0);
}
- gen_lea_v_seg(s, s->aflag, s->A0, a.def_seg, s->override);
+ gen_lea_v_seg(s, s->A0, a.def_seg, s->override);
if (a.index >= 0) {
tcg_gen_mov_tl(s->T0, cpu_regs[a.index]);
} else {
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index bc96735f61d..9eecf7ab56c 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -76,7 +76,7 @@ static void gen_NM_exception(DisasContext *s)
static void gen_load_ea(DisasContext *s, AddressParts *mem, bool is_vsib)
{
TCGv ea = gen_lea_modrm_1(s, *mem, is_vsib);
- gen_lea_v_seg(s, s->aflag, ea, mem->def_seg, s->override);
+ gen_lea_v_seg(s, ea, mem->def_seg, s->override);
}
static inline int mmx_offset(MemOp ot)
@@ -2044,7 +2044,7 @@ static void gen_MOV(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
static void gen_MASKMOV(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
- gen_lea_v_seg(s, s->aflag, cpu_regs[R_EDI], R_DS, s->override);
+ gen_lea_v_seg(s, cpu_regs[R_EDI], R_DS, s->override);
if (s->prefix & PREFIX_DATA) {
gen_helper_maskmov_xmm(tcg_env, OP_PTR1, OP_PTR2, s->A0);
@@ -4039,7 +4039,7 @@ static void gen_XLAT(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
/* AL is already zero-extended into s->T0. */
tcg_gen_add_tl(s->A0, cpu_regs[R_EBX], s->T0);
- gen_lea_v_seg(s, s->aflag, s->A0, R_DS, s->override);
+ gen_lea_v_seg(s, s->A0, R_DS, s->override);
gen_op_ld_v(s, MO_8, s->T0, s->A0);
}
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 15/16] target/i386: cpu_load_eflags already sets cc_op
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (13 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 14/16] target/i386: remove aflag argument of gen_lea_v_seg Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 15:02 ` Richard Henderson
2024-05-24 8:10 ` [PATCH 16/16] target/i386: set CC_OP in helpers if they want CC_OP_EFLAGS Paolo Bonzini
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
No need to set it again at the end of the translation block, cc_op_dirty
can be set to false.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/translate.c | 37 ++++++++++++++++++++++++-------------
target/i386/tcg/emit.c.inc | 2 +-
2 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 1a776e77297..7442a8a51b1 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -332,7 +332,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = {
[CC_OP_POPCNT] = USES_CC_SRC,
};
-static void set_cc_op(DisasContext *s, CCOp op)
+static void set_cc_op_1(DisasContext *s, CCOp op, bool dirty)
{
int dead;
@@ -355,20 +355,27 @@ static void set_cc_op(DisasContext *s, CCOp op)
tcg_gen_discard_tl(s->cc_srcT);
}
- if (op == CC_OP_DYNAMIC) {
- /* The DYNAMIC setting is translator only, and should never be
- stored. Thus we always consider it clean. */
- s->cc_op_dirty = false;
- } else {
- /* Discard any computed CC_OP value (see shifts). */
- if (s->cc_op == CC_OP_DYNAMIC) {
- tcg_gen_discard_i32(cpu_cc_op);
- }
- s->cc_op_dirty = true;
+ if (dirty && s->cc_op == CC_OP_DYNAMIC) {
+ tcg_gen_discard_i32(cpu_cc_op);
}
+ s->cc_op_dirty = dirty;
s->cc_op = op;
}
+static void set_cc_op(DisasContext *s, CCOp op)
+{
+ /*
+ * The DYNAMIC setting is translator only, everything else
+ * will be spilled later.
+ */
+ set_cc_op_1(s, op, op != CC_OP_DYNAMIC);
+}
+
+static void assume_cc_op(DisasContext *s, CCOp op)
+{
+ set_cc_op_1(s, op, false);
+}
+
static void gen_update_cc_op(DisasContext *s)
{
if (s->cc_op_dirty) {
@@ -3510,6 +3517,10 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
gen_update_cc_op(s);
gen_update_eip_cur(s);
gen_helper_syscall(tcg_env, cur_insn_len_i32(s));
+ /* condition codes are modified only in long mode */
+ if (LMA(s)) {
+ assume_cc_op(s, CC_OP_EFLAGS);
+ }
/* TF handling for the syscall insn is different. The TF bit is checked
after the syscall insn completes. This allows #DB to not be
generated after one has entered CPL0 if TF is set in FMASK. */
@@ -3526,7 +3537,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
gen_helper_sysret(tcg_env, tcg_constant_i32(dflag - 1));
/* condition codes are modified only in long mode */
if (LMA(s)) {
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
/* TF handling for the sysret insn is different. The TF bit is
checked after the sysret insn completes. This allows #DB to be
@@ -4444,7 +4455,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
g_assert_not_reached();
#else
gen_helper_rsm(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
#endif /* CONFIG_USER_ONLY */
s->base.is_jmp = DISAS_EOB_ONLY;
break;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 9eecf7ab56c..9fea395dfbf 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1881,7 +1881,7 @@ static void gen_IRET(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
gen_helper_iret_protected(tcg_env, tcg_constant_i32(s->dflag - 1),
eip_next_i32(s));
}
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
s->base.is_jmp = DISAS_EOB_ONLY;
}
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 16/16] target/i386: set CC_OP in helpers if they want CC_OP_EFLAGS
2024-05-24 8:10 [PATCH 00/16] target/i386/tcg: translation cleanups Paolo Bonzini
` (14 preceding siblings ...)
2024-05-24 8:10 ` [PATCH 15/16] target/i386: cpu_load_eflags already sets cc_op Paolo Bonzini
@ 2024-05-24 8:10 ` Paolo Bonzini
2024-05-24 15:07 ` Richard Henderson
15 siblings, 1 reply; 38+ messages in thread
From: Paolo Bonzini @ 2024-05-24 8:10 UTC (permalink / raw)
To: qemu-devel
Mark cc_op as clean and do not spill it at the end of the translation block.
Technically this is a tiny bit less efficient, but:
* it results in translations that are a tiny bit smaller
* for most of these instructions, it is not unlikely that they are close to
the end of the basic block, in which case the spilling of cc_op would be
there anyway
* even in other cases, the cost is probably dwarfed by that of computing flags.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/ops_sse.h | 8 ++++++++
target/i386/tcg/fpu_helper.c | 2 ++
target/i386/tcg/int_helper.c | 13 +++++++++----
target/i386/tcg/seg_helper.c | 16 ++++++++--------
target/i386/tcg/translate.c | 12 ++++++------
target/i386/tcg/emit.c.inc | 22 +++++++++++-----------
6 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 6a465a35fdb..f0aa1894aa2 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -1111,6 +1111,7 @@ void helper_ucomiss(CPUX86State *env, Reg *d, Reg *s)
s1 = s->ZMM_S(0);
ret = float32_compare_quiet(s0, s1, &env->sse_status);
CC_SRC = comis_eflags[ret + 1];
+ CC_OP = CC_OP_EFLAGS;
}
void helper_comiss(CPUX86State *env, Reg *d, Reg *s)
@@ -1122,6 +1123,7 @@ void helper_comiss(CPUX86State *env, Reg *d, Reg *s)
s1 = s->ZMM_S(0);
ret = float32_compare(s0, s1, &env->sse_status);
CC_SRC = comis_eflags[ret + 1];
+ CC_OP = CC_OP_EFLAGS;
}
void helper_ucomisd(CPUX86State *env, Reg *d, Reg *s)
@@ -1133,6 +1135,7 @@ void helper_ucomisd(CPUX86State *env, Reg *d, Reg *s)
d1 = s->ZMM_D(0);
ret = float64_compare_quiet(d0, d1, &env->sse_status);
CC_SRC = comis_eflags[ret + 1];
+ CC_OP = CC_OP_EFLAGS;
}
void helper_comisd(CPUX86State *env, Reg *d, Reg *s)
@@ -1144,6 +1147,7 @@ void helper_comisd(CPUX86State *env, Reg *d, Reg *s)
d1 = s->ZMM_D(0);
ret = float64_compare(d0, d1, &env->sse_status);
CC_SRC = comis_eflags[ret + 1];
+ CC_OP = CC_OP_EFLAGS;
}
#endif
@@ -1610,6 +1614,7 @@ void glue(helper_ptest, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
cf |= (s->Q(i) & ~d->Q(i));
}
CC_SRC = (zf ? 0 : CC_Z) | (cf ? 0 : CC_C);
+ CC_OP = CC_OP_EFLAGS;
}
#define FMOVSLDUP(i) s->L((i) & ~1)
@@ -1966,6 +1971,7 @@ static inline unsigned pcmpxstrx(CPUX86State *env, Reg *d, Reg *s,
validd--;
CC_SRC = (valids < upper ? CC_Z : 0) | (validd < upper ? CC_S : 0);
+ CC_OP = CC_OP_EFLAGS;
switch ((ctrl >> 2) & 3) {
case 0:
@@ -2297,6 +2303,7 @@ void glue(helper_vtestps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
cf |= (s->L(i) & ~d->L(i));
}
CC_SRC = ((zf >> 31) ? 0 : CC_Z) | ((cf >> 31) ? 0 : CC_C);
+ CC_OP = CC_OP_EFLAGS;
}
void glue(helper_vtestpd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
@@ -2309,6 +2316,7 @@ void glue(helper_vtestpd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
cf |= (s->Q(i) & ~d->Q(i));
}
CC_SRC = ((zf >> 63) ? 0 : CC_Z) | ((cf >> 63) ? 0 : CC_C);
+ CC_OP = CC_OP_EFLAGS;
}
void glue(helper_vpmaskmovd_st, SUFFIX)(CPUX86State *env,
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index ece22a3553f..8df8cae6310 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -487,6 +487,7 @@ void helper_fcomi_ST0_FT0(CPUX86State *env)
ret = floatx80_compare(ST0, FT0, &env->fp_status);
eflags = cpu_cc_compute_all(env) & ~(CC_Z | CC_P | CC_C);
CC_SRC = eflags | fcomi_ccval[ret + 1];
+ CC_OP = CC_OP_EFLAGS;
merge_exception_flags(env, old_flags);
}
@@ -499,6 +500,7 @@ void helper_fucomi_ST0_FT0(CPUX86State *env)
ret = floatx80_compare_quiet(ST0, FT0, &env->fp_status);
eflags = cpu_cc_compute_all(env) & ~(CC_Z | CC_P | CC_C);
CC_SRC = eflags | fcomi_ccval[ret + 1];
+ CC_OP = CC_OP_EFLAGS;
merge_exception_flags(env, old_flags);
}
diff --git a/target/i386/tcg/int_helper.c b/target/i386/tcg/int_helper.c
index 4cc59f15203..e1f92405282 100644
--- a/target/i386/tcg/int_helper.c
+++ b/target/i386/tcg/int_helper.c
@@ -187,6 +187,7 @@ void helper_aaa(CPUX86State *env)
}
env->regs[R_EAX] = (env->regs[R_EAX] & ~0xffff) | al | (ah << 8);
CC_SRC = eflags;
+ CC_OP = CC_OP_EFLAGS;
}
void helper_aas(CPUX86State *env)
@@ -211,6 +212,7 @@ void helper_aas(CPUX86State *env)
}
env->regs[R_EAX] = (env->regs[R_EAX] & ~0xffff) | al | (ah << 8);
CC_SRC = eflags;
+ CC_OP = CC_OP_EFLAGS;
}
void helper_daa(CPUX86State *env)
@@ -238,6 +240,7 @@ void helper_daa(CPUX86State *env)
eflags |= parity_table[al]; /* pf */
eflags |= (al & 0x80); /* sf */
CC_SRC = eflags;
+ CC_OP = CC_OP_EFLAGS;
}
void helper_das(CPUX86State *env)
@@ -269,6 +272,7 @@ void helper_das(CPUX86State *env)
eflags |= parity_table[al]; /* pf */
eflags |= (al & 0x80); /* sf */
CC_SRC = eflags;
+ CC_OP = CC_OP_EFLAGS;
}
#ifdef TARGET_X86_64
@@ -449,10 +453,11 @@ target_ulong HELPER(rdrand)(CPUX86State *env)
error_free(err);
/* Failure clears CF and all other flags, and returns 0. */
env->cc_src = 0;
- return 0;
+ ret = 0;
+ } else {
+ /* Success sets CF and clears all others. */
+ env->cc_src = CC_C;
}
-
- /* Success sets CF and clears all others. */
- env->cc_src = CC_C;
+ env->cc_op = CC_OP_EFLAGS;
return ret;
}
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 34ccabd8ce3..0301459004e 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -2326,7 +2326,7 @@ void helper_verr(CPUX86State *env, target_ulong selector1)
int rpl, dpl, cpl;
selector = selector1 & 0xffff;
- eflags = cpu_cc_compute_all(env);
+ eflags = cpu_cc_compute_all(env) | CC_Z;
if ((selector & 0xfffc) == 0) {
goto fail;
}
@@ -2351,11 +2351,11 @@ void helper_verr(CPUX86State *env, target_ulong selector1)
} else {
if (dpl < cpl || dpl < rpl) {
fail:
- CC_SRC = eflags & ~CC_Z;
- return;
+ eflags &= ~CC_Z;
}
}
- CC_SRC = eflags | CC_Z;
+ CC_SRC = eflags;
+ CC_OP = CC_OP_EFLAGS;
}
void helper_verw(CPUX86State *env, target_ulong selector1)
@@ -2364,7 +2364,7 @@ void helper_verw(CPUX86State *env, target_ulong selector1)
int rpl, dpl, cpl;
selector = selector1 & 0xffff;
- eflags = cpu_cc_compute_all(env);
+ eflags = cpu_cc_compute_all(env) | CC_Z;
if ((selector & 0xfffc) == 0) {
goto fail;
}
@@ -2385,9 +2385,9 @@ void helper_verw(CPUX86State *env, target_ulong selector1)
}
if (!(e2 & DESC_W_MASK)) {
fail:
- CC_SRC = eflags & ~CC_Z;
- return;
+ eflags &= ~CC_Z;
}
}
- CC_SRC = eflags | CC_Z;
+ CC_SRC = eflags;
+ CC_OP = CC_OP_EFLAGS;
}
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 7442a8a51b1..851ac86ff32 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2940,7 +2940,7 @@ static bool disas_insn_x87(DisasContext *s, CPUState *cpu, int b)
gen_update_cc_op(s);
gen_helper_fmov_FT0_STN(tcg_env, tcg_constant_i32(opreg));
gen_helper_fucomi_ST0_FT0(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
break;
case 0x1e: /* fcomi */
if (!(s->cpuid_features & CPUID_CMOV)) {
@@ -2949,7 +2949,7 @@ static bool disas_insn_x87(DisasContext *s, CPUState *cpu, int b)
gen_update_cc_op(s);
gen_helper_fmov_FT0_STN(tcg_env, tcg_constant_i32(opreg));
gen_helper_fcomi_ST0_FT0(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
break;
case 0x28: /* ffree sti */
gen_helper_ffree_STN(tcg_env, tcg_constant_i32(opreg));
@@ -3008,7 +3008,7 @@ static bool disas_insn_x87(DisasContext *s, CPUState *cpu, int b)
gen_helper_fmov_FT0_STN(tcg_env, tcg_constant_i32(opreg));
gen_helper_fucomi_ST0_FT0(tcg_env);
gen_helper_fpop(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
break;
case 0x3e: /* fcomip */
if (!(s->cpuid_features & CPUID_CMOV)) {
@@ -3018,7 +3018,7 @@ static bool disas_insn_x87(DisasContext *s, CPUState *cpu, int b)
gen_helper_fmov_FT0_STN(tcg_env, tcg_constant_i32(opreg));
gen_helper_fcomi_ST0_FT0(tcg_env);
gen_helper_fpop(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
break;
case 0x10 ... 0x13: /* fcmovxx */
case 0x18 ... 0x1b:
@@ -3224,7 +3224,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
gen_helper_rdrand(s->T0, tcg_env);
rm = (modrm & 7) | REX_B(s);
gen_op_mov_reg_v(s, dflag, rm, s->T0);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
break;
default:
@@ -3611,7 +3611,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
} else {
gen_helper_verw(tcg_env, s->T0);
}
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
break;
default:
goto unknown_op;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 9fea395dfbf..e990141454b 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -918,7 +918,7 @@ static void gen_##uname(DisasContext *s, CPUX86State *env, X86DecodedInsn *decod
} else { \
gen_helper_##lname##_ymm(tcg_env, OP_PTR1, OP_PTR2); \
} \
- set_cc_op(s, CC_OP_EFLAGS); \
+ assume_cc_op(s, CC_OP_EFLAGS); \
}
UNARY_CMP_SSE(VPTEST, ptest)
UNARY_CMP_SSE(VTESTPS, vtestps)
@@ -1079,7 +1079,7 @@ static void gen_AAA(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
gen_update_cc_op(s);
gen_helper_aaa(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_AAD(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
@@ -1102,7 +1102,7 @@ static void gen_AAS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
gen_update_cc_op(s);
gen_helper_aas(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_ADC(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
@@ -1564,14 +1564,14 @@ static void gen_DAA(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
gen_update_cc_op(s);
gen_helper_daa(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_DAS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
gen_update_cc_op(s);
gen_helper_das(tcg_env);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_DEC(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
@@ -2351,14 +2351,14 @@ static void gen_PCMPESTRI(DisasContext *s, CPUX86State *env, X86DecodedInsn *dec
{
TCGv_i32 imm = tcg_constant8u_i32(decode->immediate);
gen_helper_pcmpestri_xmm(tcg_env, OP_PTR1, OP_PTR2, imm);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_PCMPESTRM(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
TCGv_i32 imm = tcg_constant8u_i32(decode->immediate);
gen_helper_pcmpestrm_xmm(tcg_env, OP_PTR1, OP_PTR2, imm);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
if ((s->prefix & PREFIX_VEX) && !s->vex_l) {
tcg_gen_gvec_dup_imm(MO_64, offsetof(CPUX86State, xmm_regs[0].ZMM_X(1)),
16, 16, 0);
@@ -2369,14 +2369,14 @@ static void gen_PCMPISTRI(DisasContext *s, CPUX86State *env, X86DecodedInsn *dec
{
TCGv_i32 imm = tcg_constant8u_i32(decode->immediate);
gen_helper_pcmpistri_xmm(tcg_env, OP_PTR1, OP_PTR2, imm);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_PCMPISTRM(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
{
TCGv_i32 imm = tcg_constant8u_i32(decode->immediate);
gen_helper_pcmpistrm_xmm(tcg_env, OP_PTR1, OP_PTR2, imm);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
if ((s->prefix & PREFIX_VEX) && !s->vex_l) {
tcg_gen_gvec_dup_imm(MO_64, offsetof(CPUX86State, xmm_regs[0].ZMM_X(1)),
16, 16, 0);
@@ -3589,7 +3589,7 @@ static void gen_VCOMI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
SSEFunc_0_epp fn;
fn = s->prefix & PREFIX_DATA ? gen_helper_comisd : gen_helper_comiss;
fn(tcg_env, OP_PTR1, OP_PTR2);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_VCVTPD2PS(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
@@ -3976,7 +3976,7 @@ static void gen_VUCOMI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode
SSEFunc_0_epp fn;
fn = s->prefix & PREFIX_DATA ? gen_helper_ucomisd : gen_helper_ucomiss;
fn(tcg_env, OP_PTR1, OP_PTR2);
- set_cc_op(s, CC_OP_EFLAGS);
+ assume_cc_op(s, CC_OP_EFLAGS);
}
static void gen_VZEROALL(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
--
2.45.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 16/16] target/i386: set CC_OP in helpers if they want CC_OP_EFLAGS
2024-05-24 8:10 ` [PATCH 16/16] target/i386: set CC_OP in helpers if they want CC_OP_EFLAGS Paolo Bonzini
@ 2024-05-24 15:07 ` Richard Henderson
0 siblings, 0 replies; 38+ messages in thread
From: Richard Henderson @ 2024-05-24 15:07 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel
On 5/24/24 01:10, Paolo Bonzini wrote:
> Mark cc_op as clean and do not spill it at the end of the translation block.
> Technically this is a tiny bit less efficient, but:
>
> * it results in translations that are a tiny bit smaller
>
> * for most of these instructions, it is not unlikely that they are close to
> the end of the basic block, in which case the spilling of cc_op would be
> there anyway
>
> * even in other cases, the cost is probably dwarfed by that of computing flags.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
> target/i386/ops_sse.h | 8 ++++++++
> target/i386/tcg/fpu_helper.c | 2 ++
> target/i386/tcg/int_helper.c | 13 +++++++++----
> target/i386/tcg/seg_helper.c | 16 ++++++++--------
> target/i386/tcg/translate.c | 12 ++++++------
> target/i386/tcg/emit.c.inc | 22 +++++++++++-----------
> 6 files changed, 44 insertions(+), 29 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
A helper to set both CC_SRC and CC_OP = EFLAGS might be nice, as well as beginning to
remove the CC_* macros, and thus the implicit use of @env.
r~
^ permalink raw reply [flat|nested] 38+ messages in thread