1. Patchew
  2. QEMU
  3. [PATCH 00/13] target/i386: optimize string operations
  4. View patch

[PATCH 04/13] target/i386: unify choice between single and repeated string instructions

Paolo Bonzini posted 13 patches 3 months, 3 weeks ago
[PATCH 04/13] target/i386: unify choice between single and repeated string instructions
Posted by Paolo Bonzini 3 months, 3 weeks ago 
The same "if" is present in all generator functions for string instructions.
Push it inside gen_repz() and gen_repz_nz() instead.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/translate.c | 12 +++++++++--
 target/i386/tcg/emit.c.inc  | 42 +++++++------------------------------
 2 files changed, 17 insertions(+), 37 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 8bf6aa1fcf6..63a39d9f15a 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1340,13 +1340,21 @@ static void gen_repz(DisasContext *s, MemOp ot,
                      void (*fn)(DisasContext *s, MemOp ot))
 
 {
-    do_gen_rep(s, ot, fn, false);
+    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
+        do_gen_rep(s, ot, fn, false);
+    } else {
+        fn(s, ot);
+    }
 }
 
 static void gen_repz_nz(DisasContext *s, MemOp ot,
                         void (*fn)(DisasContext *s, MemOp ot))
 {
-    do_gen_rep(s, ot, fn, true);
+    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
+        do_gen_rep(s, ot, fn, true);
+    } else {
+        fn(s, ot);
+    }
 }
 
 static void gen_helper_fp_arith_ST0_FT0(int op)
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index a05ba019026..c346bf53c5f 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -1722,11 +1722,7 @@ static void gen_CMPccXADD(DisasContext *s, X86DecodedInsn *decode)
 static void gen_CMPS(DisasContext *s, X86DecodedInsn *decode)
 {
     MemOp ot = decode->op[2].ot;
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz_nz(s, ot, gen_cmps);
-    } else {
-        gen_cmps(s, ot);
-    }
+    gen_repz_nz(s, ot, gen_cmps);
 }
 
 static void gen_CMPXCHG(DisasContext *s, X86DecodedInsn *decode)
@@ -2217,11 +2213,7 @@ static void gen_INS(DisasContext *s, X86DecodedInsn *decode)
     }
 
     translator_io_start(&s->base);
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz(s, ot, gen_ins);
-    } else {
-        gen_ins(s, ot);
-    }
+    gen_repz(s, ot, gen_ins);
 }
 
 static void gen_INSERTQ_i(DisasContext *s, X86DecodedInsn *decode)
@@ -2406,11 +2398,7 @@ static void gen_LGS(DisasContext *s, X86DecodedInsn *decode)
 static void gen_LODS(DisasContext *s, X86DecodedInsn *decode)
 {
     MemOp ot = decode->op[1].ot;
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz(s, ot, gen_lods);
-    } else {
-        gen_lods(s, ot);
-    }
+    gen_repz(s, ot, gen_lods);
 }
 
 static void gen_LOOP(DisasContext *s, X86DecodedInsn *decode)
@@ -2608,11 +2596,7 @@ static void gen_MOVq_dq(DisasContext *s, X86DecodedInsn *decode)
 static void gen_MOVS(DisasContext *s, X86DecodedInsn *decode)
 {
     MemOp ot = decode->op[2].ot;
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz(s, ot, gen_movs);
-    } else {
-        gen_movs(s, ot);
-    }
+    gen_repz(s, ot, gen_movs);
 }
 
 static void gen_MUL(DisasContext *s, X86DecodedInsn *decode)
@@ -2774,11 +2758,7 @@ static void gen_OUTS(DisasContext *s, X86DecodedInsn *decode)
     }
 
     translator_io_start(&s->base);
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz(s, ot, gen_outs);
-    } else {
-        gen_outs(s, ot);
-    }
+    gen_repz(s, ot, gen_outs);
 }
 
 static void gen_PALIGNR(DisasContext *s, X86DecodedInsn *decode)
@@ -3859,11 +3839,7 @@ static void gen_SBB(DisasContext *s, X86DecodedInsn *decode)
 static void gen_SCAS(DisasContext *s, X86DecodedInsn *decode)
 {
     MemOp ot = decode->op[2].ot;
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz_nz(s, ot, gen_scas);
-    } else {
-        gen_scas(s, ot);
-    }
+    gen_repz_nz(s, ot, gen_scas);
 }
 
 static void gen_SETcc(DisasContext *s, X86DecodedInsn *decode)
@@ -4069,11 +4045,7 @@ static void gen_STMXCSR(DisasContext *s, X86DecodedInsn *decode)
 static void gen_STOS(DisasContext *s, X86DecodedInsn *decode)
 {
     MemOp ot = decode->op[1].ot;
-    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
-        gen_repz(s, ot, gen_stos);
-    } else {
-        gen_stos(s, ot);
-    }
+    gen_repz(s, ot, gen_stos);
 }
 
 static void gen_SUB(DisasContext *s, X86DecodedInsn *decode)
-- 
2.47.1
Re: [PATCH 04/13] target/i386: unify choice between single and repeated string instructions
Posted by Richard Henderson 3 months, 3 weeks ago 
On 12/15/24 03:06, Paolo Bonzini wrote:
> The same "if" is present in all generator functions for string instructions.
> Push it inside gen_repz() and gen_repz_nz() instead.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/translate.c | 12 +++++++++--
>   target/i386/tcg/emit.c.inc  | 42 +++++++------------------------------
>   2 files changed, 17 insertions(+), 37 deletions(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 8bf6aa1fcf6..63a39d9f15a 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -1340,13 +1340,21 @@ static void gen_repz(DisasContext *s, MemOp ot,
>                        void (*fn)(DisasContext *s, MemOp ot))
>   
>   {
> -    do_gen_rep(s, ot, fn, false);
> +    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
> +        do_gen_rep(s, ot, fn, false);
> +    } else {
> +        fn(s, ot);
> +    }
>   }
>   
>   static void gen_repz_nz(DisasContext *s, MemOp ot,
>                           void (*fn)(DisasContext *s, MemOp ot))
>   {
> -    do_gen_rep(s, ot, fn, true);
> +    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
> +        do_gen_rep(s, ot, fn, true);
> +    } else {
> +        fn(s, ot);
> +    }
>   }

Why not push it into do_gen_rep?


r~
Re: [PATCH 04/13] target/i386: unify choice between single and repeated string instructions
Posted by Paolo Bonzini 3 months, 3 weeks ago 
Il dom 15 dic 2024, 15:10 Richard Henderson <richard.henderson@linaro.org>
ha scritto:

> On 12/15/24 03:06, Paolo Bonzini wrote:
> > The same "if" is present in all generator functions for string
> instructions.
> > Push it inside gen_repz() and gen_repz_nz() instead.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   target/i386/tcg/translate.c | 12 +++++++++--
> >   target/i386/tcg/emit.c.inc  | 42 +++++++------------------------------
> >   2 files changed, 17 insertions(+), 37 deletions(-)
> >
> > diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> > index 8bf6aa1fcf6..63a39d9f15a 100644
> > --- a/target/i386/tcg/translate.c
> > +++ b/target/i386/tcg/translate.c
> > @@ -1340,13 +1340,21 @@ static void gen_repz(DisasContext *s, MemOp ot,
> >                        void (*fn)(DisasContext *s, MemOp ot))
> >
> >   {
> > -    do_gen_rep(s, ot, fn, false);
> > +    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
> > +        do_gen_rep(s, ot, fn, false);
> > +    } else {
> > +        fn(s, ot);
> > +    }
> >   }
> >
> >   static void gen_repz_nz(DisasContext *s, MemOp ot,
> >                           void (*fn)(DisasContext *s, MemOp ot))
> >   {
> > -    do_gen_rep(s, ot, fn, true);
> > +    if (s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) {
> > +        do_gen_rep(s, ot, fn, true);
> > +    } else {
> > +        fn(s, ot);
> > +    }
> >   }
>
> Why not push it into do_gen_rep?
>

Just because do_gen_rep is already complicated enough by the end of the
series.

Paolo


> r~
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.