Patchwork [09/14] i386: do not call helper to compute ZF/SF

login
register
mail settings
Submitter Paolo Bonzini
Date Oct. 6, 2012, 12:30 p.m.
Message ID <1349526621-13939-10-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/189690/
State New
Headers show

Comments

Paolo Bonzini - Oct. 6, 2012, 12:30 p.m.
ZF, SF and PF can always be computed from CC_DST except in the
CC_OP_EFLAGS case (and CC_OP_DYNAMIC, which just resolves to CC_OP_EFLAGS
in gen_compute_eflags).  Use setcond to compute ZF and SF.

We could also use a table lookup to compute PF.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/translate.c | 28 ++++++++++++++++++++++------
 1 file modificato, 22 inserzioni(+), 6 rimozioni(-)
Blue Swirl - Oct. 7, 2012, 7:16 p.m.
On Sat, Oct 6, 2012 at 12:30 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> ZF, SF and PF can always be computed from CC_DST except in the
> CC_OP_EFLAGS case (and CC_OP_DYNAMIC, which just resolves to CC_OP_EFLAGS
> in gen_compute_eflags).  Use setcond to compute ZF and SF.
>
> We could also use a table lookup to compute PF.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Blue Swirl <blauwirbel@gmail.com>

> ---
>  target-i386/translate.c | 28 ++++++++++++++++++++++------
>  1 file modificato, 22 inserzioni(+), 6 rimozioni(-)
>
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 09512c3..daa36c1 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -859,9 +859,17 @@ static void gen_compute_eflags_p(DisasContext *s, TCGv reg)
>  /* compute eflags.S to reg */
>  static void gen_compute_eflags_s(DisasContext *s, TCGv reg)
>  {
> -    gen_compute_eflags(s);
> -    tcg_gen_shri_tl(reg, cpu_cc_src, 7);
> -    tcg_gen_andi_tl(reg, reg, 1);
> +    if (s->cc_op == CC_OP_DYNAMIC) {
> +        gen_compute_eflags(s);
> +    }
> +    if (s->cc_op == CC_OP_EFLAGS) {
> +        tcg_gen_shri_tl(reg, cpu_cc_src, 7);
> +        tcg_gen_andi_tl(reg, reg, 1);
> +    } else {

A comment would be nice here, like something extracted from commit message.

> +        int size = (s->cc_op - CC_OP_ADDB) & 3;
> +        gen_ext_tl(reg, cpu_cc_dst, size, true);
> +        tcg_gen_setcondi_tl(TCG_COND_LT, reg, reg, 0);
> +    }
>  }
>
>  /* compute eflags.O to reg */
> @@ -875,9 +883,17 @@ static void gen_compute_eflags_o(DisasContext *s, TCGv reg)
>  /* compute eflags.Z to reg */
>  static void gen_compute_eflags_z(DisasContext *s, TCGv reg)
>  {
> -    gen_compute_eflags(s);
> -    tcg_gen_shri_tl(reg, cpu_cc_src, 6);
> -    tcg_gen_andi_tl(reg, reg, 1);
> +    if (s->cc_op == CC_OP_DYNAMIC) {
> +        gen_compute_eflags(s);
> +    }
> +    if (s->cc_op == CC_OP_EFLAGS) {
> +        tcg_gen_shri_tl(reg, cpu_cc_src, 6);
> +        tcg_gen_andi_tl(reg, reg, 1);
> +    } else {

Ditto.

> +        int size = (s->cc_op - CC_OP_ADDB) & 3;
> +        gen_ext_tl(reg, cpu_cc_dst, size, false);
> +        tcg_gen_setcondi_tl(TCG_COND_EQ, reg, cpu_cc_dst, 0);
> +    }
>  }
>
>  static inline void gen_setcc_slow_T0(DisasContext *s, int jcc_op)
> --
> 1.7.12.1
>
>
>
Richard Henderson - Oct. 9, 2012, 7:15 p.m.
On 10/06/2012 05:30 AM, Paolo Bonzini wrote:
> ZF, SF and PF can always be computed from CC_DST except in the
> CC_OP_EFLAGS case (and CC_OP_DYNAMIC, which just resolves to CC_OP_EFLAGS
> in gen_compute_eflags).  Use setcond to compute ZF and SF.
> 
> We could also use a table lookup to compute PF.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~
Richard Henderson - Oct. 9, 2012, 7:16 p.m.
On 10/06/2012 05:30 AM, Paolo Bonzini wrote:
> +        int size = (s->cc_op - CC_OP_ADDB) & 3;
> +        gen_ext_tl(reg, cpu_cc_dst, size, false);
> +        tcg_gen_setcondi_tl(TCG_COND_EQ, reg, cpu_cc_dst, 0);

I take that back.  Should be (EQ, reg, reg, 0) here;
you've dropped the extension on the floor.


r~
Paolo Bonzini - Oct. 10, 2012, 6:42 a.m.
Il 09/10/2012 21:16, Richard Henderson ha scritto:
>> > +        int size = (s->cc_op - CC_OP_ADDB) & 3;
>> > +        gen_ext_tl(reg, cpu_cc_dst, size, false);
>> > +        tcg_gen_setcondi_tl(TCG_COND_EQ, reg, cpu_cc_dst, 0);
> I take that back.  Should be (EQ, reg, reg, 0) here;
> you've dropped the extension on the floor.

More precisely it should be:

        TCGv t0 = gen_ext_tl(reg, cpu_cc_dst, size, false);
        tcg_gen_setcondi_tl(TCG_COND_EQ, reg, t0, 0);

and similarly for SF.

Paolo

Patch

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 09512c3..daa36c1 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -859,9 +859,17 @@  static void gen_compute_eflags_p(DisasContext *s, TCGv reg)
 /* compute eflags.S to reg */
 static void gen_compute_eflags_s(DisasContext *s, TCGv reg)
 {
-    gen_compute_eflags(s);
-    tcg_gen_shri_tl(reg, cpu_cc_src, 7);
-    tcg_gen_andi_tl(reg, reg, 1);
+    if (s->cc_op == CC_OP_DYNAMIC) {
+        gen_compute_eflags(s);
+    }
+    if (s->cc_op == CC_OP_EFLAGS) {
+        tcg_gen_shri_tl(reg, cpu_cc_src, 7);
+        tcg_gen_andi_tl(reg, reg, 1);
+    } else {
+        int size = (s->cc_op - CC_OP_ADDB) & 3;
+        gen_ext_tl(reg, cpu_cc_dst, size, true);
+        tcg_gen_setcondi_tl(TCG_COND_LT, reg, reg, 0);
+    }
 }
 
 /* compute eflags.O to reg */
@@ -875,9 +883,17 @@  static void gen_compute_eflags_o(DisasContext *s, TCGv reg)
 /* compute eflags.Z to reg */
 static void gen_compute_eflags_z(DisasContext *s, TCGv reg)
 {
-    gen_compute_eflags(s);
-    tcg_gen_shri_tl(reg, cpu_cc_src, 6);
-    tcg_gen_andi_tl(reg, reg, 1);
+    if (s->cc_op == CC_OP_DYNAMIC) {
+        gen_compute_eflags(s);
+    }
+    if (s->cc_op == CC_OP_EFLAGS) {
+        tcg_gen_shri_tl(reg, cpu_cc_src, 6);
+        tcg_gen_andi_tl(reg, reg, 1);
+    } else {
+        int size = (s->cc_op - CC_OP_ADDB) & 3;
+        gen_ext_tl(reg, cpu_cc_dst, size, false);
+        tcg_gen_setcondi_tl(TCG_COND_EQ, reg, cpu_cc_dst, 0);
+    }
 }
 
 static inline void gen_setcc_slow_T0(DisasContext *s, int jcc_op)