Explicitly generate a TSTEQ branch (which is optimized to NE x,0 if possible).
This does not make much sense yet, but later we will add more checks and some
will use a temporary to check on the decremented value of CX/ECX/RCX; it will
be clearer for all checks to share the same logic using TSTEQ(reg, cx_mask).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 4b53a47000e..8ef938372d5 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1336,6 +1336,7 @@ static void do_gen_rep(DisasContext *s, MemOp ot,
                        bool is_repz_nz)
 {
     TCGLabel *done = gen_new_label();
+    target_ulong cx_mask = MAKE_64BIT_MASK(0, 8 << s->aflag);
     bool had_rf = s->flags & HF_RF_MASK;
 
     /*
@@ -1358,7 +1359,7 @@ static void do_gen_rep(DisasContext *s, MemOp ot,
     tcg_set_insn_start_param(s->base.insn_start, 1, CC_OP_DYNAMIC);
 
     /* Any iteration at all?  */
-    gen_op_jz_ecx(s, done);
+    tcg_gen_brcondi_tl(TCG_COND_TSTEQ, cpu_regs[R_ECX], cx_mask, done);
 
     fn(s, ot);
     gen_op_add_reg_im(s, s->aflag, R_ECX, -1);
-- 
2.47.1

On 12/15/24 03:06, Paolo Bonzini wrote:
> Explicitly generate a TSTEQ branch (which is optimized to NE x,0 if possible).
> This does not make much sense yet, but later we will add more checks and some
> will use a temporary to check on the decremented value of CX/ECX/RCX; it will
> be clearer for all checks to share the same logic using TSTEQ(reg, cx_mask).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/tcg/translate.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 4b53a47000e..8ef938372d5 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -1336,6 +1336,7 @@ static void do_gen_rep(DisasContext *s, MemOp ot,
>                          bool is_repz_nz)
>   {
>       TCGLabel *done = gen_new_label();
> +    target_ulong cx_mask = MAKE_64BIT_MASK(0, 8 << s->aflag);
>       bool had_rf = s->flags & HF_RF_MASK;
>   
>       /*
> @@ -1358,7 +1359,7 @@ static void do_gen_rep(DisasContext *s, MemOp ot,
>       tcg_set_insn_start_param(s->base.insn_start, 1, CC_OP_DYNAMIC);
>   
>       /* Any iteration at all?  */
> -    gen_op_jz_ecx(s, done);
> +    tcg_gen_brcondi_tl(TCG_COND_TSTEQ, cpu_regs[R_ECX], cx_mask, done);
>   
>       fn(s, ot);
>       gen_op_add_reg_im(s, s->aflag, R_ECX, -1);

gen_op_jz_ecx should become unused now.

And maybe gen_op_jnz_ecx went unused a few patches ago, when gen_jz_ecx_string got merged?
Anyway, I think there are some dead inlines which clang would Werror about.

With those fixed,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Il dom 15 dic 2024, 15:50 Richard Henderson <richard.henderson@linaro.org>
ha scritto:

> gen_op_jz_ecx should become unused now.
>
> And maybe gen_op_jnz_ecx went unused a few patches ago, when
> gen_jz_ecx_string got merged?
>

IIRC LOOP{,Z,NZ} use both of them. I can inline them as a follow-up.

Paolo

Anyway, I think there are some dead inlines which clang would Werror about.
>
> With those fixed,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~
>
>