[PATCH 02/10] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL

Paolo Bonzini posted 10 patches 6 months, 1 week ago
[PATCH 02/10] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL
Posted by Paolo Bonzini 6 months, 1 week ago
Handle it like the other arithmetic cc_ops.  This simplifies a
bit the implementation of bit test instructions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.h           | 13 +++++++++++--
 target/i386/tcg/translate.c |  3 +--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f54cd93b3f9..8504a7998fd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1275,6 +1275,7 @@ typedef enum {
     CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest.  */
     CC_OP_ADOX, /* CC_SRC2 = O, CC_SRC = rest.  */
     CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest.  */
+    CC_OP_CLR, /* Z and P set, all other flags clear.  */
 
     CC_OP_MULB, /* modify all flags, C, O = (CC_SRC != 0) */
     CC_OP_MULW,
@@ -1331,8 +1332,16 @@ typedef enum {
     CC_OP_BMILGL,
     CC_OP_BMILGQ,
 
-    CC_OP_CLR, /* Z set, all other flags clear.  */
-    CC_OP_POPCNT, /* Z via CC_DST, all other flags clear.  */
+    /*
+     * Note that only CC_OP_POPCNT (i.e. the one with MO_TL size)
+     * is used or implemented, because the translation needs
+     * to zero-extend CC_DST anyway.
+     */
+    CC_OP_POPCNTB__, /* Z via CC_DST, all other flags clear.  */
+    CC_OP_POPCNTW__,
+    CC_OP_POPCNTL__,
+    CC_OP_POPCNTQ__,
+    CC_OP_POPCNT = sizeof(target_ulong) == 8 ? CC_OP_POPCNTQ__ : CC_OP_POPCNTL__,
 
     CC_OP_NB,
 } CCOp;
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index f32cda4e169..934c514e64f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1019,8 +1019,6 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg)
                              .imm = CC_Z };
     case CC_OP_CLR:
         return (CCPrepare) { .cond = TCG_COND_ALWAYS };
-    case CC_OP_POPCNT:
-        return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };
     default:
         {
             MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
@@ -3177,6 +3175,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b)
         case CC_OP_SHLB ... CC_OP_SHLQ:
         case CC_OP_SARB ... CC_OP_SARQ:
         case CC_OP_BMILGB ... CC_OP_BMILGQ:
+        case CC_OP_POPCNT:
             /* Z was going to be computed from the non-zero status of CC_DST.
                We can get that same Z value (and the new C value) by leaving
                CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the
-- 
2.45.2
Re: [PATCH 02/10] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL
Posted by Richard Henderson 6 months, 1 week ago
On 6/20/24 02:54, Paolo Bonzini wrote:
> Handle it like the other arithmetic cc_ops.  This simplifies a
> bit the implementation of bit test instructions.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/cpu.h           | 13 +++++++++++--
>   target/i386/tcg/translate.c |  3 +--
>   2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index f54cd93b3f9..8504a7998fd 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1275,6 +1275,7 @@ typedef enum {
>       CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest.  */
>       CC_OP_ADOX, /* CC_SRC2 = O, CC_SRC = rest.  */
>       CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest.  */
> +    CC_OP_CLR, /* Z and P set, all other flags clear.  */
>   
>       CC_OP_MULB, /* modify all flags, C, O = (CC_SRC != 0) */
>       CC_OP_MULW,
> @@ -1331,8 +1332,16 @@ typedef enum {
>       CC_OP_BMILGL,
>       CC_OP_BMILGQ,
>   
> -    CC_OP_CLR, /* Z set, all other flags clear.  */
> -    CC_OP_POPCNT, /* Z via CC_DST, all other flags clear.  */
> +    /*
> +     * Note that only CC_OP_POPCNT (i.e. the one with MO_TL size)
> +     * is used or implemented, because the translation needs
> +     * to zero-extend CC_DST anyway.
> +     */
> +    CC_OP_POPCNTB__, /* Z via CC_DST, all other flags clear.  */
> +    CC_OP_POPCNTW__,
> +    CC_OP_POPCNTL__,
> +    CC_OP_POPCNTQ__,
> +    CC_OP_POPCNT = sizeof(target_ulong) == 8 ? CC_OP_POPCNTQ__ : CC_OP_POPCNTL__,
>   
>       CC_OP_NB,
>   } CCOp;
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index f32cda4e169..934c514e64f 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -1019,8 +1019,6 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg)
>                                .imm = CC_Z };
>       case CC_OP_CLR:
>           return (CCPrepare) { .cond = TCG_COND_ALWAYS };
> -    case CC_OP_POPCNT:
> -        return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };

The previous patch needs to have changed this to dst.

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


r~