[Qemu-devel] [PATCH v1 00/10] POWER9 TCG enablements - part15

[Qemu-devel] [PATCH v1 00/10] POWER9 TCG enablements - part15

[Nikunj A Dadhania]

This series contains implentation of CA32 and OV32 bits added to the 
ISA 3.0. Various fixed-point arithmetic instructions are updated to take
care of the newer flags. 

Finally the last patch adds new instruction mcrxrx, that helps reading 
the carry (CA and CA32) and the overflow (OV and OV32) flags

Nikunj A Dadhania (10):
  target/ppc: support for 32-bit carry and overflow
  target/ppc: Update ca32 in arithmetic add
  target/ppc: move subf logic block
  target/ppc: compute ca32 for arithmetic substract
  target/ppc: update overflow flags for add/sub
  target/ppc: use tcg ops for neg instruction
  target/ppc: update ov/ov32 for nego
  target/ppc: add ov32 flag for multiply low insns
  target/ppc: add ov32 flag in divide operations
  target/ppc: add mcrxrx instruction

 target/ppc/cpu.h        |  30 +++++++++++
 target/ppc/int_helper.c |  49 ++++++------------
 target/ppc/translate.c  | 134 +++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 155 insertions(+), 58 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH v1 01/10] target/ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

POWER ISA 3.0 adds CA32 and OV32 status in 64-bit mode. Add the flags
and corresponding defines.

Moreover, CA32 is updated when CA is updated and OV32 is updated when OV
is updated.

Arithmetic instructions:
    * Addition and Substractions:

        addic, addic., subfic, addc, subfc, adde, subfe, addme, subfme,
        addze, and subfze always updates CA and CA32.

        => CA reflects the carry out of bit 0 in 64-bit mode and out of
           bit 32 in 32-bit mode.
        => CA32 reflects the carry out of bit 32 independent of the
           mode.

        => SO and OV reflects overflow of the 64-bit result in 64-bit
           mode and overflow of the low-order 32-bit result in 32-bit
           mode
        => OV32 reflects overflow of the low-order 32-bit independent of
           the mode

    * Multiply Low and Divide:

        For mulld, divd, divde, divdu and divdeu: SO, OV, and OV32 bits
        reflects overflow of the 64-bit result

        For mullw, divw, divwe, divwu and divweu: SO, OV, and OV32 bits
        reflects overflow of the 32-bit result

     * Negate with OE=1 (nego)

       For 64-bit mode if the register RA contains
       0x8000_0000_0000_0000, OV and OV32 are set to 1.

       For 32-bit mode if the register RA contains 0x8000_0000, OV and
       OV32 are set to 1.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/cpu.h       | 30 ++++++++++++++++++++++++++++++
 target/ppc/translate.c | 17 ++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 425e79d..ef392f0 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -965,6 +965,8 @@ struct CPUPPCState {
     target_ulong so;
     target_ulong ov;
     target_ulong ca;
+    target_ulong ov32;
+    target_ulong ca32;
     /* Reservation address */
     target_ulong reserve_addr;
     /* Reservation value */
@@ -1372,11 +1374,15 @@ int ppc_compat_max_threads(PowerPCCPU *cpu);
 #define XER_SO  31
 #define XER_OV  30
 #define XER_CA  29
+#define XER_OV32  19
+#define XER_CA32  18
 #define XER_CMP  8
 #define XER_BC   0
 #define xer_so  (env->so)
 #define xer_ov  (env->ov)
 #define xer_ca  (env->ca)
+#define xer_ov32  (env->ov)
+#define xer_ca32  (env->ca)
 #define xer_cmp ((env->xer >> XER_CMP) & 0xFF)
 #define xer_bc  ((env->xer >> XER_BC)  & 0x7F)
 
@@ -2343,11 +2349,21 @@ enum {
 
 /*****************************************************************************/
 
+#ifndef TARGET_PPC64
 static inline target_ulong cpu_read_xer(CPUPPCState *env)
 {
     return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
 }
+#else
+static inline target_ulong cpu_read_xer(CPUPPCState *env)
+{
+    return env->xer | (env->so << XER_SO) |
+        (env->ov << XER_OV) | (env->ca << XER_CA) |
+        (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
+}
+#endif
 
+#ifndef TARGET_PPC64
 static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
 {
     env->so = (xer >> XER_SO) & 1;
@@ -2355,6 +2371,20 @@ static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
     env->ca = (xer >> XER_CA) & 1;
     env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
 }
+#else
+static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
+{
+    env->so = (xer >> XER_SO) & 1;
+    env->ov = (xer >> XER_OV) & 1;
+    env->ca = (xer >> XER_CA) & 1;
+    env->ov32 = (xer >> XER_OV32) & 1;
+    env->ca32 = (xer >> XER_CA32) & 1;
+    env->xer = xer & ~((1ul << XER_SO) |
+                       (1ul << XER_OV) | (1ul << XER_CA) |
+                       (1ul << XER_OV32) | (1ul << XER_CA32));
+}
+#endif
+
 
 static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
                                         target_ulong *cs_base, uint32_t *flags)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 3ba2616..498b095 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -71,7 +71,7 @@ static TCGv cpu_lr;
 #if defined(TARGET_PPC64)
 static TCGv cpu_cfar;
 #endif
-static TCGv cpu_xer, cpu_so, cpu_ov, cpu_ca;
+static TCGv cpu_xer, cpu_so, cpu_ov, cpu_ca, cpu_ov32, cpu_ca32;
 static TCGv cpu_reserve;
 static TCGv cpu_fpscr;
 static TCGv_i32 cpu_access_type;
@@ -173,6 +173,10 @@ void ppc_translate_init(void)
                                 offsetof(CPUPPCState, ov), "OV");
     cpu_ca = tcg_global_mem_new(cpu_env,
                                 offsetof(CPUPPCState, ca), "CA");
+    cpu_ov32 = tcg_global_mem_new(cpu_env,
+                                  offsetof(CPUPPCState, ov32), "OV32");
+    cpu_ca32 = tcg_global_mem_new(cpu_env,
+                                  offsetof(CPUPPCState, ca32), "CA32");
 
     cpu_reserve = tcg_global_mem_new(cpu_env,
                                      offsetof(CPUPPCState, reserve_addr),
@@ -3715,6 +3719,12 @@ static void gen_read_xer(TCGv dst)
     tcg_gen_or_tl(t0, t0, t1);
     tcg_gen_or_tl(dst, dst, t2);
     tcg_gen_or_tl(dst, dst, t0);
+#ifdef TARGET_PPC64
+    tcg_gen_shli_tl(t0, cpu_ov32, XER_OV32);
+    tcg_gen_or_tl(dst, dst, t0);
+    tcg_gen_shli_tl(t0, cpu_ca32, XER_CA32);
+    tcg_gen_or_tl(dst, dst, t0);
+#endif
     tcg_temp_free(t0);
     tcg_temp_free(t1);
     tcg_temp_free(t2);
@@ -3727,9 +3737,14 @@ static void gen_write_xer(TCGv src)
     tcg_gen_shri_tl(cpu_so, src, XER_SO);
     tcg_gen_shri_tl(cpu_ov, src, XER_OV);
     tcg_gen_shri_tl(cpu_ca, src, XER_CA);
+    tcg_gen_shri_tl(cpu_ov32, src, XER_OV32);
+    tcg_gen_shri_tl(cpu_ca32, src, XER_CA32);
     tcg_gen_andi_tl(cpu_so, cpu_so, 1);
     tcg_gen_andi_tl(cpu_ov, cpu_ov, 1);
     tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
+    tcg_gen_andi_tl(cpu_ov32, cpu_ov32, 1);
+    tcg_gen_andi_tl(cpu_ca32, cpu_ca32, 1);
+
 }
 
 /* mcrxr */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 01/10] target/ppc: support for 32-bit carry and overflow

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
> +#ifndef TARGET_PPC64
>  static inline target_ulong cpu_read_xer(CPUPPCState *env)
>  {
>      return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
>  }
> +#else
> +static inline target_ulong cpu_read_xer(CPUPPCState *env)
> +{
> +    return env->xer | (env->so << XER_SO) |
> +        (env->ov << XER_OV) | (env->ca << XER_CA) |
> +        (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
> +}
> +#endif
>
> +#ifndef TARGET_PPC64
>  static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>  {
>      env->so = (xer >> XER_SO) & 1;
> @@ -2355,6 +2371,20 @@ static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>      env->ca = (xer >> XER_CA) & 1;
>      env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
>  }
> +#else
> +static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
> +{
> +    env->so = (xer >> XER_SO) & 1;
> +    env->ov = (xer >> XER_OV) & 1;
> +    env->ca = (xer >> XER_CA) & 1;
> +    env->ov32 = (xer >> XER_OV32) & 1;
> +    env->ca32 = (xer >> XER_CA32) & 1;
> +    env->xer = xer & ~((1ul << XER_SO) |
> +                       (1ul << XER_OV) | (1ul << XER_CA) |
> +                       (1ul << XER_OV32) | (1ul << XER_CA32));
> +}
> +#endif

You should probably move both of these out of line now (perhaps cpu.c).

You probably don't want to set ov32/ca32 unless the cpu is power9.  I assume 
that if you attempt to set these bits for power8 they are 
read-as-zero/write-ignore?

> @@ -3715,6 +3719,12 @@ static void gen_read_xer(TCGv dst)
>      tcg_gen_or_tl(t0, t0, t1);
>      tcg_gen_or_tl(dst, dst, t2);
>      tcg_gen_or_tl(dst, dst, t0);
> +#ifdef TARGET_PPC64
> +    tcg_gen_shli_tl(t0, cpu_ov32, XER_OV32);
> +    tcg_gen_or_tl(dst, dst, t0);
> +    tcg_gen_shli_tl(t0, cpu_ca32, XER_CA32);
> +    tcg_gen_or_tl(dst, dst, t0);
> +#endif
>      tcg_temp_free(t0);
>      tcg_temp_free(t1);
>      tcg_temp_free(t2);
> @@ -3727,9 +3737,14 @@ static void gen_write_xer(TCGv src)
>      tcg_gen_shri_tl(cpu_so, src, XER_SO);
>      tcg_gen_shri_tl(cpu_ov, src, XER_OV);
>      tcg_gen_shri_tl(cpu_ca, src, XER_CA);
> +    tcg_gen_shri_tl(cpu_ov32, src, XER_OV32);
> +    tcg_gen_shri_tl(cpu_ca32, src, XER_CA32);
>      tcg_gen_andi_tl(cpu_so, cpu_so, 1);
>      tcg_gen_andi_tl(cpu_ov, cpu_ov, 1);
>      tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
> +    tcg_gen_andi_tl(cpu_ov32, cpu_ov32, 1);
> +    tcg_gen_andi_tl(cpu_ca32, cpu_ca32, 1);
> +
>  }

Watch the blank lines.  No ifdef here on the write side?


r~

Re: [Qemu-devel] [PATCH v1 01/10] target/ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>> +#ifndef TARGET_PPC64
>>  static inline target_ulong cpu_read_xer(CPUPPCState *env)
>>  {
>>      return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
>>  }
>> +#else
>> +static inline target_ulong cpu_read_xer(CPUPPCState *env)
>> +{
>> +    return env->xer | (env->so << XER_SO) |
>> +        (env->ov << XER_OV) | (env->ca << XER_CA) |
>> +        (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
>> +}
>> +#endif
>>
>> +#ifndef TARGET_PPC64
>>  static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>>  {
>>      env->so = (xer >> XER_SO) & 1;
>> @@ -2355,6 +2371,20 @@ static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>>      env->ca = (xer >> XER_CA) & 1;
>>      env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
>>  }
>> +#else
>> +static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>> +{
>> +    env->so = (xer >> XER_SO) & 1;
>> +    env->ov = (xer >> XER_OV) & 1;
>> +    env->ca = (xer >> XER_CA) & 1;
>> +    env->ov32 = (xer >> XER_OV32) & 1;
>> +    env->ca32 = (xer >> XER_CA32) & 1;
>> +    env->xer = xer & ~((1ul << XER_SO) |
>> +                       (1ul << XER_OV) | (1ul << XER_CA) |
>> +                       (1ul << XER_OV32) | (1ul << XER_CA32));
>> +}
>> +#endif
>
> You should probably move both of these out of line now (perhaps cpu.c).

Sure.


> You probably don't want to set ov32/ca32 unless the cpu is power9.  I assume 
> that if you attempt to set these bits for power8 they are 
> read-as-zero/write-ignore?

Sure, will make it CPU specific.

>> @@ -3715,6 +3719,12 @@ static void gen_read_xer(TCGv dst)
>>      tcg_gen_or_tl(t0, t0, t1);
>>      tcg_gen_or_tl(dst, dst, t2);
>>      tcg_gen_or_tl(dst, dst, t0);
>> +#ifdef TARGET_PPC64
>> +    tcg_gen_shli_tl(t0, cpu_ov32, XER_OV32);
>> +    tcg_gen_or_tl(dst, dst, t0);
>> +    tcg_gen_shli_tl(t0, cpu_ca32, XER_CA32);
>> +    tcg_gen_or_tl(dst, dst, t0);
>> +#endif
>>      tcg_temp_free(t0);
>>      tcg_temp_free(t1);
>>      tcg_temp_free(t2);
>> @@ -3727,9 +3737,14 @@ static void gen_write_xer(TCGv src)
>>      tcg_gen_shri_tl(cpu_so, src, XER_SO);
>>      tcg_gen_shri_tl(cpu_ov, src, XER_OV);
>>      tcg_gen_shri_tl(cpu_ca, src, XER_CA);
>> +    tcg_gen_shri_tl(cpu_ov32, src, XER_OV32);
>> +    tcg_gen_shri_tl(cpu_ca32, src, XER_CA32);
>>      tcg_gen_andi_tl(cpu_so, cpu_so, 1);
>>      tcg_gen_andi_tl(cpu_ov, cpu_ov, 1);
>>      tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
>> +    tcg_gen_andi_tl(cpu_ov32, cpu_ov32, 1);
>> +    tcg_gen_andi_tl(cpu_ca32, cpu_ca32, 1);
>> +
>>  }
>
> Watch the blank lines.  No ifdef here on the write side?

Right, will add.

Regards
Nikunj

Re: [Qemu-devel] [PATCH v1 01/10] target/ppc: support for 32-bit carry and overflow

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3462 bytes --]

On Tue, Feb 21, 2017 at 10:15:46AM +0530, Nikunj A Dadhania wrote:
> Richard Henderson <rth@twiddle.net> writes:
> 
> > On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
> >> +#ifndef TARGET_PPC64
> >>  static inline target_ulong cpu_read_xer(CPUPPCState *env)
> >>  {
> >>      return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
> >>  }
> >> +#else
> >> +static inline target_ulong cpu_read_xer(CPUPPCState *env)
> >> +{
> >> +    return env->xer | (env->so << XER_SO) |
> >> +        (env->ov << XER_OV) | (env->ca << XER_CA) |
> >> +        (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
> >> +}
> >> +#endif
> >>
> >> +#ifndef TARGET_PPC64
> >>  static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
> >>  {
> >>      env->so = (xer >> XER_SO) & 1;
> >> @@ -2355,6 +2371,20 @@ static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
> >>      env->ca = (xer >> XER_CA) & 1;
> >>      env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
> >>  }
> >> +#else
> >> +static inline void cpu_write_xer(CPUPPCState *env, target_ulong xer)
> >> +{
> >> +    env->so = (xer >> XER_SO) & 1;
> >> +    env->ov = (xer >> XER_OV) & 1;
> >> +    env->ca = (xer >> XER_CA) & 1;
> >> +    env->ov32 = (xer >> XER_OV32) & 1;
> >> +    env->ca32 = (xer >> XER_CA32) & 1;
> >> +    env->xer = xer & ~((1ul << XER_SO) |
> >> +                       (1ul << XER_OV) | (1ul << XER_CA) |
> >> +                       (1ul << XER_OV32) | (1ul << XER_CA32));
> >> +}
> >> +#endif
> >
> > You should probably move both of these out of line now (perhaps cpu.c).
> 
> Sure.
> 
> 
> > You probably don't want to set ov32/ca32 unless the cpu is power9.  I assume 
> > that if you attempt to set these bits for power8 they are 
> > read-as-zero/write-ignore?
> 
> Sure, will make it CPU specific.

Right, and given you need a CPU model check anyway, I don't see that
there's any benefit to splitting out the 32-bit build version versus
the 64-bit build version.

> >> @@ -3715,6 +3719,12 @@ static void gen_read_xer(TCGv dst)
> >>      tcg_gen_or_tl(t0, t0, t1);
> >>      tcg_gen_or_tl(dst, dst, t2);
> >>      tcg_gen_or_tl(dst, dst, t0);
> >> +#ifdef TARGET_PPC64
> >> +    tcg_gen_shli_tl(t0, cpu_ov32, XER_OV32);
> >> +    tcg_gen_or_tl(dst, dst, t0);
> >> +    tcg_gen_shli_tl(t0, cpu_ca32, XER_CA32);
> >> +    tcg_gen_or_tl(dst, dst, t0);
> >> +#endif
> >>      tcg_temp_free(t0);
> >>      tcg_temp_free(t1);
> >>      tcg_temp_free(t2);
> >> @@ -3727,9 +3737,14 @@ static void gen_write_xer(TCGv src)
> >>      tcg_gen_shri_tl(cpu_so, src, XER_SO);
> >>      tcg_gen_shri_tl(cpu_ov, src, XER_OV);
> >>      tcg_gen_shri_tl(cpu_ca, src, XER_CA);
> >> +    tcg_gen_shri_tl(cpu_ov32, src, XER_OV32);
> >> +    tcg_gen_shri_tl(cpu_ca32, src, XER_CA32);
> >>      tcg_gen_andi_tl(cpu_so, cpu_so, 1);
> >>      tcg_gen_andi_tl(cpu_ov, cpu_ov, 1);
> >>      tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
> >> +    tcg_gen_andi_tl(cpu_ov32, cpu_ov32, 1);
> >> +    tcg_gen_andi_tl(cpu_ca32, cpu_ca32, 1);
> >> +
> >>  }
> >
> > Watch the blank lines.  No ifdef here on the write side?
> 
> Right, will add.
> 
> Regards
> Nikunj
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [PATCH v1 02/10] target/ppc: Update ca32 in arithmetic add

[Nikunj A Dadhania]

Adds routine to compute ca32 - gen_op_arith_compute_ca32

For 64-bit mode use the compute ca32 routine. While for 32-bit mode, CA
and CA32 will have same value.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 498b095..2a2d071 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -816,6 +816,36 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
     tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
 }
 
+static inline void gen_op_arith_compute_ca32(DisasContext *ctx, TCGv arg0,
+                                             TCGv arg1, bool add_ca, int sub)
+{
+    TCGv t0 = tcg_temp_new();
+    TCGv t1 = tcg_temp_new();
+    TCGv inv0 = tcg_temp_new();
+
+    tcg_gen_extract_tl(t0, arg0, 0, 32);
+    tcg_gen_extract_tl(t1, arg1, 0, 32);
+    if (sub) {
+        tcg_gen_not_tl(inv0, t0);
+        if (add_ca) {
+            tcg_gen_add_tl(t1, t1, cpu_ca32);
+        } else {
+            tcg_gen_addi_tl(t1, t1, 1);
+        }
+        tcg_gen_add_tl(t0, t1, inv0);
+        tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
+    } else {
+        tcg_gen_add_tl(t0, t0, t1);
+        if (add_ca) {
+            tcg_gen_add_tl(t0, t0, cpu_ca32);
+        }
+        tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
+    }
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+
 /* Common add function */
 static inline void gen_op_arith_add(DisasContext *ctx, TCGv ret, TCGv arg1,
                                     TCGv arg2, bool add_ca, bool compute_ca,
@@ -842,6 +872,7 @@ static inline void gen_op_arith_add(DisasContext *ctx, TCGv ret, TCGv arg1,
             tcg_temp_free(t1);
             tcg_gen_shri_tl(cpu_ca, cpu_ca, 32);   /* extract bit 32 */
             tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
+            tcg_gen_mov_tl(cpu_ca32, cpu_ca);
         } else {
             TCGv zero = tcg_const_tl(0);
             if (add_ca) {
@@ -850,6 +881,7 @@ static inline void gen_op_arith_add(DisasContext *ctx, TCGv ret, TCGv arg1,
             } else {
                 tcg_gen_add2_tl(t0, cpu_ca, arg1, zero, arg2, zero);
             }
+            gen_op_arith_compute_ca32(ctx, arg1, arg2, add_ca, 0);
             tcg_temp_free(zero);
         }
     } else {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 02/10] target/ppc: Update ca32 in arithmetic add

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
> Adds routine to compute ca32 - gen_op_arith_compute_ca32
>
> For 64-bit mode use the compute ca32 routine. While for 32-bit mode, CA
> and CA32 will have same value.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 498b095..2a2d071 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -816,6 +816,36 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
>      tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>  }
>
> +static inline void gen_op_arith_compute_ca32(DisasContext *ctx, TCGv arg0,
> +                                             TCGv arg1, bool add_ca, int sub)
> +{
> +    TCGv t0 = tcg_temp_new();
> +    TCGv t1 = tcg_temp_new();
> +    TCGv inv0 = tcg_temp_new();
> +
> +    tcg_gen_extract_tl(t0, arg0, 0, 32);
> +    tcg_gen_extract_tl(t1, arg1, 0, 32);
> +    if (sub) {
> +        tcg_gen_not_tl(inv0, t0);
> +        if (add_ca) {
> +            tcg_gen_add_tl(t1, t1, cpu_ca32);
> +        } else {
> +            tcg_gen_addi_tl(t1, t1, 1);
> +        }
> +        tcg_gen_add_tl(t0, t1, inv0);
> +        tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
> +    } else {
> +        tcg_gen_add_tl(t0, t0, t1);
> +        if (add_ca) {
> +            tcg_gen_add_tl(t0, t0, cpu_ca32);
> +        }
> +        tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
> +    }
> +    tcg_temp_free(t0);
> +    tcg_temp_free(t1);
> +}

This is incorrect, since you're not considering the carry-in bits.  It's also 
inefficient since you should only need a single extraction.

Compute ca32 via

   t0 = result ^ in0 ^ in1
   extract_tl(ca32, t0, 32, 1)


r~

[Qemu-devel] [PATCH v1 03/10] target/ppc: move subf logic block

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 2a2d071..77045be 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1389,17 +1389,19 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
             tcg_temp_free(t1);
             tcg_gen_shri_tl(cpu_ca, cpu_ca, 32);    /* extract bit 32 */
             tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
-        } else if (add_ca) {
-            TCGv zero, inv1 = tcg_temp_new();
-            tcg_gen_not_tl(inv1, arg1);
-            zero = tcg_const_tl(0);
-            tcg_gen_add2_tl(t0, cpu_ca, arg2, zero, cpu_ca, zero);
-            tcg_gen_add2_tl(t0, cpu_ca, t0, cpu_ca, inv1, zero);
-            tcg_temp_free(zero);
-            tcg_temp_free(inv1);
         } else {
-            tcg_gen_setcond_tl(TCG_COND_GEU, cpu_ca, arg2, arg1);
-            tcg_gen_sub_tl(t0, arg2, arg1);
+            if (add_ca) {
+                TCGv zero, inv1 = tcg_temp_new();
+                tcg_gen_not_tl(inv1, arg1);
+                zero = tcg_const_tl(0);
+                tcg_gen_add2_tl(t0, cpu_ca, arg2, zero, cpu_ca, zero);
+                tcg_gen_add2_tl(t0, cpu_ca, t0, cpu_ca, inv1, zero);
+                tcg_temp_free(zero);
+                tcg_temp_free(inv1);
+            } else {
+                tcg_gen_setcond_tl(TCG_COND_GEU, cpu_ca, arg2, arg1);
+                tcg_gen_sub_tl(t0, arg2, arg1);
+            }
         }
     } else if (add_ca) {
         /* Since we're ignoring carry-out, we can simplify the
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 03/10] target/ppc: move subf logic block

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>              tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
> -        } else if (add_ca) {
> -            TCGv zero, inv1 = tcg_temp_new();
> -            tcg_gen_not_tl(inv1, arg1);
> -            zero = tcg_const_tl(0);
> -            tcg_gen_add2_tl(t0, cpu_ca, arg2, zero, cpu_ca, zero);
> -            tcg_gen_add2_tl(t0, cpu_ca, t0, cpu_ca, inv1, zero);
> -            tcg_temp_free(zero);
> -            tcg_temp_free(inv1);
>          } else {
> -            tcg_gen_setcond_tl(TCG_COND_GEU, cpu_ca, arg2, arg1);
> -            tcg_gen_sub_tl(t0, arg2, arg1);
> +            if (add_ca) {
> +                TCGv zero, inv1 = tcg_temp_new();
> +                tcg_gen_not_tl(inv1, arg1);
> +                zero = tcg_const_tl(0);
> +                tcg_gen_add2_tl(t0, cpu_ca, arg2, zero, cpu_ca, zero);
> +                tcg_gen_add2_tl(t0, cpu_ca, t0, cpu_ca, inv1, zero);
> +                tcg_temp_free(zero);
> +                tcg_temp_free(inv1);
> +            } else {
> +                tcg_gen_setcond_tl(TCG_COND_GEU, cpu_ca, arg2, arg1);
> +                tcg_gen_sub_tl(t0, arg2, arg1);
> +            }
>          }

Why would you want to increase the indentation for no reason?


r~

[Qemu-devel] [PATCH v1 04/10] target/ppc: compute ca32 for arithmetic substract

[Nikunj A Dadhania]

For 64-bit mode use the compute ca32 routine. While for 32-bit mode, CA
and CA32 will have same value.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 77045be..dd413de 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1389,6 +1389,7 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
             tcg_temp_free(t1);
             tcg_gen_shri_tl(cpu_ca, cpu_ca, 32);    /* extract bit 32 */
             tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
+            tcg_gen_mov_tl(cpu_ca32, cpu_ca);
         } else {
             if (add_ca) {
                 TCGv zero, inv1 = tcg_temp_new();
@@ -1402,6 +1403,7 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
                 tcg_gen_setcond_tl(TCG_COND_GEU, cpu_ca, arg2, arg1);
                 tcg_gen_sub_tl(t0, arg2, arg1);
             }
+            gen_op_arith_compute_ca32(ctx, arg1, arg2, add_ca, 1);
         }
     } else if (add_ca) {
         /* Since we're ignoring carry-out, we can simplify the
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 04/10] target/ppc: compute ca32 for arithmetic substract

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
> For 64-bit mode use the compute ca32 routine. While for 32-bit mode, CA
> and CA32 will have same value.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 77045be..dd413de 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1389,6 +1389,7 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
>              tcg_temp_free(t1);
>              tcg_gen_shri_tl(cpu_ca, cpu_ca, 32);    /* extract bit 32 */
>              tcg_gen_andi_tl(cpu_ca, cpu_ca, 1);
> +            tcg_gen_mov_tl(cpu_ca32, cpu_ca);
>          } else {
>              if (add_ca) {
>                  TCGv zero, inv1 = tcg_temp_new();
> @@ -1402,6 +1403,7 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv arg1,
>                  tcg_gen_setcond_tl(TCG_COND_GEU, cpu_ca, arg2, arg1);
>                  tcg_gen_sub_tl(t0, arg2, arg1);
>              }
> +            gen_op_arith_compute_ca32(ctx, arg1, arg2, add_ca, 1);
>          }

Ah, I see what you wanted with the previous patch.  However, you won't want to 
put this here when you fix ca32 computation as I described, because for the 
add_ca case you'll want to pass in inv1 as arg1 so that you don't have to 
re-invert it.


r~

[Qemu-devel] [PATCH v1 05/10] target/ppc: update overflow flags for add/sub

[Nikunj A Dadhania]

* SO and OV reflects overflow of the 64-bit result in 64-bit mode and
  overflow of the low-order 32-bit result in 32-bit mode

* OV32 reflects overflow of the low-order 32-bit independent of the mode

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index dd413de..5d8d109 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -809,10 +809,11 @@ static inline void gen_op_arith_compute_ov(DisasContext *ctx, TCGv arg0,
         tcg_gen_andc_tl(cpu_ov, cpu_ov, t0);
     }
     tcg_temp_free(t0);
+    tcg_gen_extract_tl(cpu_ov32, cpu_ov, 31, 1);
+    tcg_gen_extract_tl(cpu_ov, cpu_ov, 63, 1);
     if (NARROW_MODE(ctx)) {
-        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
+        tcg_gen_mov_tl(cpu_ov, cpu_ov32);
     }
-    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
     tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 05/10] target/ppc: update overflow flags for add/sub

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>      tcg_temp_free(t0);
> +    tcg_gen_extract_tl(cpu_ov32, cpu_ov, 31, 1);
> +    tcg_gen_extract_tl(cpu_ov, cpu_ov, 63, 1);
>      if (NARROW_MODE(ctx)) {
> -        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
> +        tcg_gen_mov_tl(cpu_ov, cpu_ov32);
>      }
> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);

Don't compute ov32 only to overwrite it again.  Move the ov32 extraction into 
an else of NARROW_MODE.


r~

Re: [Qemu-devel] [PATCH v1 05/10] target/ppc: update overflow flags for add/sub

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>>      tcg_temp_free(t0);
>> +    tcg_gen_extract_tl(cpu_ov32, cpu_ov, 31, 1);
>> +    tcg_gen_extract_tl(cpu_ov, cpu_ov, 63, 1);
>>      if (NARROW_MODE(ctx)) {
>> -        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
>> +        tcg_gen_mov_tl(cpu_ov, cpu_ov32);
>>      }
>> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>
> Don't compute ov32 only to overwrite it again.  Move the ov32 extraction into 
> an else of NARROW_MODE.

Sure.

Regards
Nikunj

[Qemu-devel] [PATCH v1 06/10] target/ppc: use tcg ops for neg instruction

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 5d8d109..9fa3b5a 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1483,7 +1483,7 @@ static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov)
 
 static void gen_neg(DisasContext *ctx)
 {
-    gen_op_arith_neg(ctx, 0);
+    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
 }
 
 static void gen_nego(DisasContext *ctx)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 06/10] target/ppc: use tcg ops for neg instruction

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>  static void gen_neg(DisasContext *ctx)
>  {
> -    gen_op_arith_neg(ctx, 0);
> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>  }
>

NACK.  You're forgetting "neg.".


r~

Re: [Qemu-devel] [PATCH v1 06/10] target/ppc: use tcg ops for neg instruction

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>>  static void gen_neg(DisasContext *ctx)
>>  {
>> -    gen_op_arith_neg(ctx, 0);
>> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>>  }
>>
>
> NACK.  You're forgetting "neg.".

Oops, i totally missed that. :(

Regards
Nikunj

[Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Nikunj A Dadhania]

For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
and OV32 are set to 1.

For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
set to 1.

Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
nego was the last user.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 9fa3b5a..0168e1c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1473,14 +1473,6 @@ static void gen_subfic(DisasContext *ctx)
 }
 
 /* neg neg. nego nego. */
-static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov)
-{
-    TCGv zero = tcg_const_tl(0);
-    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
-                      zero, 0, 0, compute_ov, Rc(ctx->opcode));
-    tcg_temp_free(zero);
-}
-
 static void gen_neg(DisasContext *ctx)
 {
     tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
@@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
 
 static void gen_nego(DisasContext *ctx)
 {
-    gen_op_arith_neg(ctx, 1);
+    TCGv t0 = tcg_temp_new();
+    TCGv zero = tcg_const_tl(0);
+
+    if (NARROW_MODE(ctx)) {
+        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
+    } else {
+        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
+    }
+
+    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
+    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
+    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
+    tcg_temp_free(t0);
+    tcg_temp_free(zero);
 }
 
 /***                            Integer logical                            ***/
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
> and OV32 are set to 1.
>
> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
> set to 1.
>
> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
> nego was the last user.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 9fa3b5a..0168e1c 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1473,14 +1473,6 @@ static void gen_subfic(DisasContext *ctx)
>  }
>
>  /* neg neg. nego nego. */
> -static inline void gen_op_arith_neg(DisasContext *ctx, bool compute_ov)
> -{
> -    TCGv zero = tcg_const_tl(0);
> -    gen_op_arith_subf(ctx, cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
> -                      zero, 0, 0, compute_ov, Rc(ctx->opcode));
> -    tcg_temp_free(zero);
> -}
> -
>  static void gen_neg(DisasContext *ctx)
>  {
>      tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
>
>  static void gen_nego(DisasContext *ctx)
>  {
> -    gen_op_arith_neg(ctx, 1);
> +    TCGv t0 = tcg_temp_new();
> +    TCGv zero = tcg_const_tl(0);
> +
> +    if (NARROW_MODE(ctx)) {
> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
> +    } else {
> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
> +    }
> +
> +    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
> +    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> +    tcg_temp_free(t0);
> +    tcg_temp_free(zero);
>  }

Again, you're forgetting "nego.".  Don't try to simplify from gen_op_arith_subf 
by hand.


r~

Re: [Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
>> and OV32 are set to 1.
>>
>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
>> set to 1.
>>
>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
>> nego was the last user.
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
>>
>>  static void gen_nego(DisasContext *ctx)
>>  {
>> -    gen_op_arith_neg(ctx, 1);
>> +    TCGv t0 = tcg_temp_new();
>> +    TCGv zero = tcg_const_tl(0);
>> +
>> +    if (NARROW_MODE(ctx)) {
>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
>> +    } else {
>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
>> +    }
>> +
>> +    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
>> +    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
>> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>> +    tcg_temp_free(t0);
>> +    tcg_temp_free(zero);
>>  }
>
> Again, you're forgetting "nego.".  Don't try to simplify from gen_op_arith_subf 
> by hand.

The reason of the simplification was the interpretation of ov and ov32.
I will add a code to compute the Rc.

Regards
Nikunj

Re: [Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Richard Henderson]

On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote:
> Richard Henderson <rth@twiddle.net> writes:
>
>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
>>> and OV32 are set to 1.
>>>
>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
>>> set to 1.
>>>
>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
>>> nego was the last user.
>>>
>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
>>>
>>>  static void gen_nego(DisasContext *ctx)
>>>  {
>>> -    gen_op_arith_neg(ctx, 1);
>>> +    TCGv t0 = tcg_temp_new();
>>> +    TCGv zero = tcg_const_tl(0);
>>> +
>>> +    if (NARROW_MODE(ctx)) {
>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
>>> +    } else {
>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
>>> +    }
>>> +
>>> +    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
>>> +    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
>>> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>>> +    tcg_temp_free(t0);
>>> +    tcg_temp_free(zero);
>>>  }
>>
>> Again, you're forgetting "nego.".  Don't try to simplify from gen_op_arith_subf
>> by hand.
>
> The reason of the simplification was the interpretation of ov and ov32.
> I will add a code to compute the Rc.

Why do you believe that the computation for overflow is different between neg 
and subf?


r~

Re: [Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote:
>> Richard Henderson <rth@twiddle.net> writes:
>>
>>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
>>>> and OV32 are set to 1.
>>>>
>>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
>>>> set to 1.
>>>>
>>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
>>>> nego was the last user.
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
>>>>
>>>>  static void gen_nego(DisasContext *ctx)
>>>>  {
>>>> -    gen_op_arith_neg(ctx, 1);
>>>> +    TCGv t0 = tcg_temp_new();
>>>> +    TCGv zero = tcg_const_tl(0);
>>>> +
>>>> +    if (NARROW_MODE(ctx)) {
>>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
>>>> +    } else {
>>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
>>>> +    }
>>>> +
>>>> +    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
>>>> +    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
>>>> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>>>> +    tcg_temp_free(t0);
>>>> +    tcg_temp_free(zero);
>>>>  }
>>>
>>> Again, you're forgetting "nego.".  Don't try to simplify from gen_op_arith_subf
>>> by hand.
>>
>> The reason of the simplification was the interpretation of ov and ov32.
>> I will add a code to compute the Rc.
>
> Why do you believe that the computation for overflow is different between neg 
> and subf?

For example in 64-bit mode, if we negate INT64_MIN(nego[.]), both OV and
OV32 has to be set to 1 according to ISA 3.0. If I use subf, only OV is
set to 1.

Regards
Nikunj

Re: [Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Richard Henderson]

On 02/22/2017 02:53 PM, Nikunj A Dadhania wrote:
> Richard Henderson <rth@twiddle.net> writes:
>
>> On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote:
>>> Richard Henderson <rth@twiddle.net> writes:
>>>
>>>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>>>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
>>>>> and OV32 are set to 1.
>>>>>
>>>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
>>>>> set to 1.
>>>>>
>>>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
>>>>> nego was the last user.
>>>>>
>>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
>>>>>
>>>>>  static void gen_nego(DisasContext *ctx)
>>>>>  {
>>>>> -    gen_op_arith_neg(ctx, 1);
>>>>> +    TCGv t0 = tcg_temp_new();
>>>>> +    TCGv zero = tcg_const_tl(0);
>>>>> +
>>>>> +    if (NARROW_MODE(ctx)) {
>>>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
>>>>> +    } else {
>>>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
>>>>> +    }
>>>>> +
>>>>> +    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
>>>>> +    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
>>>>> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>>>>> +    tcg_temp_free(t0);
>>>>> +    tcg_temp_free(zero);
>>>>>  }
>>>>
>>>> Again, you're forgetting "nego.".  Don't try to simplify from gen_op_arith_subf
>>>> by hand.
>>>
>>> The reason of the simplification was the interpretation of ov and ov32.
>>> I will add a code to compute the Rc.
>>
>> Why do you believe that the computation for overflow is different between neg
>> and subf?
>
> For example in 64-bit mode, if we negate INT64_MIN(nego[.]), both OV and
> OV32 has to be set to 1 according to ISA 3.0. If I use subf, only OV is
> set to 1.

What an odd corner case for OV32 wrt nego.  But you're right that's what the 
manual says.  I wonder why the hardware folk designed the chip that way.  It 
seems broken.

You might want to confirm with the hardware folk that this isn't a bug in the 
manual.


r~

Re: [Qemu-devel] [PATCH v1 07/10] target/ppc: update ov/ov32 for nego

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/22/2017 02:53 PM, Nikunj A Dadhania wrote:
>> Richard Henderson <rth@twiddle.net> writes:
>>
>>> On 02/21/2017 08:26 PM, Nikunj A Dadhania wrote:
>>>> Richard Henderson <rth@twiddle.net> writes:
>>>>
>>>>> On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
>>>>>> For 64-bit mode if the register RA contains 0x8000_0000_0000_0000, OV
>>>>>> and OV32 are set to 1.
>>>>>>
>>>>>> For 32-bit mode if the register RA contains 0x8000_0000, OV and OV32 are
>>>>>> set to 1.
>>>>>>
>>>>>> Use the tcg-ops for negation (neg_tl) and drop gen_op_arith_neg() as
>>>>>> nego was the last user.
>>>>>>
>>>>>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>>>>> @@ -1488,7 +1480,20 @@ static void gen_neg(DisasContext *ctx)
>>>>>>
>>>>>>  static void gen_nego(DisasContext *ctx)
>>>>>>  {
>>>>>> -    gen_op_arith_neg(ctx, 1);
>>>>>> +    TCGv t0 = tcg_temp_new();
>>>>>> +    TCGv zero = tcg_const_tl(0);
>>>>>> +
>>>>>> +    if (NARROW_MODE(ctx)) {
>>>>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], INT32_MIN);
>>>>>> +    } else {
>>>>>> +        tcg_gen_xori_tl(t0, cpu_gpr[rA(ctx->opcode)], (target_ulong)INT64_MIN);
>>>>>> +    }
>>>>>> +
>>>>>> +    tcg_gen_setcond_tl(TCG_COND_EQ, cpu_ov, t0, zero);
>>>>>> +    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
>>>>>> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
>>>>>> +    tcg_temp_free(t0);
>>>>>> +    tcg_temp_free(zero);
>>>>>>  }
>>>>>
>>>>> Again, you're forgetting "nego.".  Don't try to simplify from gen_op_arith_subf
>>>>> by hand.
>>>>
>>>> The reason of the simplification was the interpretation of ov and ov32.
>>>> I will add a code to compute the Rc.
>>>
>>> Why do you believe that the computation for overflow is different between neg
>>> and subf?
>>
>> For example in 64-bit mode, if we negate INT64_MIN(nego[.]), both OV and
>> OV32 has to be set to 1 according to ISA 3.0. If I use subf, only OV is
>> set to 1.
>
> What an odd corner case for OV32 wrt nego.  But you're right that's what the 
> manual says.  I wonder why the hardware folk designed the chip that way.  It 
> seems broken.
>
> You might want to confirm with the hardware folk that this isn't a bug in the 
> manual.

Sure, moreover I figured out that the hardware simulator isnt following
the ISA3.0 behaviour (setting OV and OV32 both)

Regards
Nikunj

[Qemu-devel] [PATCH v1 08/10] target/ppc: add ov32 flag for multiply low insns

[Nikunj A Dadhania]

For Multiply Word:
SO, OV, and OV32 bits reflects overflow of the 32-bit result

For Multiply DoubleWord:
SO, OV, and OV32 bits reflects overflow of the 64-bit result

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0168e1c..69ec0b2 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1286,6 +1286,7 @@ static void gen_mullwo(DisasContext *ctx)
     tcg_gen_sari_i32(t0, t0, 31);
     tcg_gen_setcond_i32(TCG_COND_NE, t0, t0, t1);
     tcg_gen_extu_i32_tl(cpu_ov, t0);
+    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
     tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
 
     tcg_temp_free_i32(t0);
@@ -1347,6 +1348,7 @@ static void gen_mulldo(DisasContext *ctx)
 
     tcg_gen_sari_i64(t0, t0, 63);
     tcg_gen_setcond_i64(TCG_COND_NE, cpu_ov, t0, t1);
+    tcg_gen_mov_tl(cpu_ov32, cpu_ov);
     tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
 
     tcg_temp_free_i64(t0);
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 08/10] target/ppc: add ov32 flag for multiply low insns

[Richard Henderson]

On 02/20/2017 09:11 PM, Nikunj A Dadhania wrote:
> For Multiply Word:
> SO, OV, and OV32 bits reflects overflow of the 32-bit result
>
> For Multiply DoubleWord:
> SO, OV, and OV32 bits reflects overflow of the 64-bit result
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 2 ++
>  1 file changed, 2 inser

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


r~

[Qemu-devel] [PATCH v1 09/10] target/ppc: add ov32 flag in divide operations

[Nikunj A Dadhania]

Add helper_div_compute_ov() in the int_helper for updating the overflow
flags.

For Divide Word:
SO, OV, and OV32 bits reflects overflow of the 32-bit result

For Divide DoubleWord:
SO, OV, and OV32 bits reflects overflow of the 64-bit result

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/int_helper.c | 49 ++++++++++++++++---------------------------------
 target/ppc/translate.c  |  6 ++++--
 2 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index dd0a892..34b54e1 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -28,6 +28,18 @@
 /*****************************************************************************/
 /* Fixed point operations helpers */
 
+static inline void helper_div_compute_ov(CPUPPCState *env, uint32_t oe,
+                                         int overflow)
+{
+    if (oe) {
+        if (unlikely(overflow)) {
+            env->so = env->ov = env->ov32 = 1;
+        } else {
+            env->ov = env->ov32 = 0;
+        }
+    }
+}
+
 target_ulong helper_divweu(CPUPPCState *env, target_ulong ra, target_ulong rb,
                            uint32_t oe)
 {
@@ -48,14 +60,7 @@ target_ulong helper_divweu(CPUPPCState *env, target_ulong ra, target_ulong rb,
         rt = 0; /* Undefined */
     }
 
-    if (oe) {
-        if (unlikely(overflow)) {
-            env->so = env->ov = 1;
-        } else {
-            env->ov = 0;
-        }
-    }
-
+    helper_div_compute_ov(env, oe, overflow);
     return (target_ulong)rt;
 }
 
@@ -80,14 +85,7 @@ target_ulong helper_divwe(CPUPPCState *env, target_ulong ra, target_ulong rb,
         rt = 0; /* Undefined */
     }
 
-    if (oe) {
-        if (unlikely(overflow)) {
-            env->so = env->ov = 1;
-        } else {
-            env->ov = 0;
-        }
-    }
-
+    helper_div_compute_ov(env, oe, overflow);
     return (target_ulong)rt;
 }
 
@@ -104,14 +102,7 @@ uint64_t helper_divdeu(CPUPPCState *env, uint64_t ra, uint64_t rb, uint32_t oe)
         rt = 0; /* Undefined */
     }
 
-    if (oe) {
-        if (unlikely(overflow)) {
-            env->so = env->ov = 1;
-        } else {
-            env->ov = 0;
-        }
-    }
-
+    helper_div_compute_ov(env, oe, overflow);
     return rt;
 }
 
@@ -126,15 +117,7 @@ uint64_t helper_divde(CPUPPCState *env, uint64_t rau, uint64_t rbu, uint32_t oe)
         rt = 0; /* Undefined */
     }
 
-    if (oe) {
-
-        if (unlikely(overflow)) {
-            env->so = env->ov = 1;
-        } else {
-            env->ov = 0;
-        }
-    }
-
+    helper_div_compute_ov(env, oe, overflow);
     return rt;
 }
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 69ec0b2..ee44205 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1022,6 +1022,7 @@ static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, TCGv arg1,
     }
     if (compute_ov) {
         tcg_gen_extu_i32_tl(cpu_ov, t2);
+        tcg_gen_extu_i32_tl(cpu_ov32, t2);
         tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
     }
     tcg_temp_free_i32(t0);
@@ -1093,6 +1094,7 @@ static inline void gen_op_arith_divd(DisasContext *ctx, TCGv ret, TCGv arg1,
     }
     if (compute_ov) {
         tcg_gen_mov_tl(cpu_ov, t2);
+        tcg_gen_mov_tl(cpu_ov32, t2);
         tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
     }
     tcg_temp_free_i64(t0);
@@ -1111,10 +1113,10 @@ static void glue(gen_, name)(DisasContext *ctx)
                       cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],     \
                       sign, compute_ov);                                      \
 }
-/* divwu  divwu.  divwuo  divwuo.   */
+/* divdu  divdu.  divduo  divduo.   */
 GEN_INT_ARITH_DIVD(divdu, 0x0E, 0, 0);
 GEN_INT_ARITH_DIVD(divduo, 0x1E, 0, 1);
-/* divw  divw.  divwo  divwo.   */
+/* divd  divd.  divdo  divdo.   */
 GEN_INT_ARITH_DIVD(divd, 0x0F, 1, 0);
 GEN_INT_ARITH_DIVD(divdo, 0x1F, 1, 1);
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 09/10] target/ppc: add ov32 flag in divide operations

[Richard Henderson]

On 02/20/2017 09:12 PM, Nikunj A Dadhania wrote:
> Add helper_div_compute_ov() in the int_helper for updating the overflow
> flags.
>
> For Divide Word:
> SO, OV, and OV32 bits reflects overflow of the 32-bit result
>
> For Divide DoubleWord:
> SO, OV, and OV32 bits reflects overflow of the 64-bit result
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/int_helper.c | 49 ++++++++++++++++---------------------------------
>  target/ppc/translate.c  |  6 ++++--
>  2 files changed, 20 insertions(+), 35 deletions(-)

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


r~

[Qemu-devel] [PATCH v1 10/10] target/ppc: add mcrxrx instruction

[Nikunj A Dadhania]

mcrxrx: Move to CR from XER Extended

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/translate.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ee44205..36d59a5 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3816,6 +3816,28 @@ static void gen_mcrxr(DisasContext *ctx)
     tcg_gen_movi_tl(cpu_ca, 0);
 }
 
+#ifdef TARGET_PPC64
+/* mcrxrx */
+static void gen_mcrxrx(DisasContext *ctx)
+{
+    TCGv t0 = tcg_temp_new();
+    TCGv t1 = tcg_temp_new();
+    TCGv_i32 dst = cpu_crf[crfD(ctx->opcode)];
+
+    /* copy OV and OV32 */
+    tcg_gen_shli_tl(t0, cpu_ov, 1);
+    tcg_gen_or_tl(t0, t0, cpu_ov32);
+    tcg_gen_shli_tl(t0, t0, 2);
+    /* copy CA and CA32 */
+    tcg_gen_shli_tl(t1, cpu_ca, 1);
+    tcg_gen_or_tl(t1, t1, cpu_ca32);
+    tcg_gen_or_tl(t0, t0, t1);
+    tcg_gen_trunc_tl_i32(dst, t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+#endif
+
 /* mfcr mfocrf */
 static void gen_mfcr(DisasContext *ctx)
 {
@@ -6485,6 +6507,7 @@ GEN_HANDLER(mtcrf, 0x1F, 0x10, 0x04, 0x00000801, PPC_MISC),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(mtmsrd, 0x1F, 0x12, 0x05, 0x001EF801, PPC_64B),
 GEN_HANDLER_E(setb, 0x1F, 0x00, 0x04, 0x0003F801, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(mcrxrx, 0x1F, 0x00, 0x12, 0x007FF801, PPC_NONE, PPC2_ISA300),
 #endif
 GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001EF801, PPC_MISC),
 GEN_HANDLER(mtspr, 0x1F, 0x13, 0x0E, 0x00000000, PPC_MISC),
-- 
2.7.4

Re: [Qemu-devel] [PATCH v1 10/10] target/ppc: add mcrxrx instruction

[Richard Henderson]

On 02/20/2017 09:12 PM, Nikunj A Dadhania wrote:
> mcrxrx: Move to CR from XER Extended
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

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


r~