[PATCH 15/16] target/i386: cpu_load_eflags already sets cc_op

Paolo Bonzini posted 16 patches 6 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
[PATCH 15/16] target/i386: cpu_load_eflags already sets cc_op
Posted by Paolo Bonzini 6 months ago
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
Re: [PATCH 15/16] target/i386: cpu_load_eflags already sets cc_op
Posted by Richard Henderson 6 months ago
On 5/24/24 01:10, Paolo Bonzini wrote:
> 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(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~