On 10/14/23 03:01, Paolo Bonzini wrote:
> The new x86 decoder wants to compute EFLAGS before writeback, which
> can be an issue for some instructions such as ARPL. Extract code
> to compute the EFLAGS without clobbering CC_SRC, in case the ARPL
> memory write causes a fault.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> target/i386/tcg/translate.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index e13bf7df591..2da7c357cdc 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -872,18 +872,20 @@ static void gen_op_update_neg_cc(DisasContext *s)
> tcg_gen_movi_tl(s->cc_srcT, 0);
> }
>
> -/* compute all eflags to cc_src */
> -static void gen_compute_eflags(DisasContext *s)
> +/* compute all eflags to reg */
> +static void gen_mov_eflags(DisasContext *s, TCGv reg)
> {
> TCGv zero, dst, src1, src2;
> int live, dead;
>
> if (s->cc_op == CC_OP_EFLAGS) {
> + if (reg != cpu_cc_src) {
> + tcg_gen_mov_tl(reg, cpu_cc_src);
> + }
tcg_gen_mov_tl already takes care of eliding the nop move.
> return;
> }
> if (s->cc_op == CC_OP_CLR) {
> - tcg_gen_movi_tl(cpu_cc_src, CC_Z | CC_P);
> - set_cc_op(s, CC_OP_EFLAGS);
> + tcg_gen_movi_tl(reg, CC_Z | CC_P);
> return;
> }
>
> @@ -909,7 +911,13 @@ static void gen_compute_eflags(DisasContext *s)
> }
>
> gen_update_cc_op(s);
> - gen_helper_cc_compute_all(cpu_cc_src, dst, src1, src2, cpu_cc_op);
> + gen_helper_cc_compute_all(reg, dst, src1, src2, cpu_cc_op);
> +}
You don't want to gen_update_cc_op.
I think you want
TCGv_i32 cc_op = cpu_cc_op;
if (s->cc_op != CC_OP_DYNAMIC)
cc_op = tcg_constant_i32(s->cc_op);
gen_helper_cc_compute_all(..., cc_op);
No write to cpu_cc_op required at all, since...
> +/* compute all eflags to cc_src */
> +static void gen_compute_eflags(DisasContext *s)
> +{
> + gen_mov_eflags(s, cpu_cc_src);
> set_cc_op(s, CC_OP_EFLAGS);
> }
... this will modify it again anyway.
r~