[Qemu-devel] [PATCH v2 00/11] POWER9 TCG enablements - part15

[Qemu-devel] [PATCH v2 00/11] 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

Changelog:
v1: 
* Use these ISA 3.0 flag to enable CA32 and OV32
* Re-write ca32 compute routine
* Add setting of flags for "neg." and "nego."

Nikunj A Dadhania (11):
  target/ppc: move cpu_[read, write]_xer to cpu.c
  target/ppc: optimize gen_write_xer()
  target/ppc: support for 32-bit carry and overflow
  target/ppc: update ca32 in arithmetic add
  target/ppc: update ca32 in 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/Makefile.objs    |   1 +
 target/ppc/cpu.c            |  51 ++++++++++++++++
 target/ppc/cpu.h            |  21 +++----
 target/ppc/int_helper.c     |  49 +++++----------
 target/ppc/translate.c      | 143 ++++++++++++++++++++++++++++++++++++--------
 target/ppc/translate_init.c |   4 +-
 6 files changed, 196 insertions(+), 73 deletions(-)
 create mode 100644 target/ppc/cpu.c

-- 
2.7.4

[Qemu-devel] [PATCH v2 01/11] target/ppc: move cpu_[read, write]_xer to cpu.c

[Nikunj A Dadhania]

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target/ppc/Makefile.objs |  1 +
 target/ppc/cpu.c         | 36 ++++++++++++++++++++++++++++++++++++
 target/ppc/cpu.h         | 14 ++------------
 3 files changed, 39 insertions(+), 12 deletions(-)
 create mode 100644 target/ppc/cpu.c

diff --git a/target/ppc/Makefile.objs b/target/ppc/Makefile.objs
index a8c7a30..4f4168f 100644
--- a/target/ppc/Makefile.objs
+++ b/target/ppc/Makefile.objs
@@ -1,4 +1,5 @@
 obj-y += cpu-models.o
+obj-y += cpu.o
 obj-y += translate.o
 ifeq ($(CONFIG_SOFTMMU),y)
 obj-y += machine.o mmu_helper.o mmu-hash32.o monitor.o
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
new file mode 100644
index 0000000..de3004b
--- /dev/null
+++ b/target/ppc/cpu.c
@@ -0,0 +1,36 @@
+/*
+ *  PowerPC CPU routines for qemu.
+ *
+ * Copyright (c) 2017 Nikunj A Dadhania, IBM Corporation.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "cpu-models.h"
+
+target_ulong cpu_read_xer(CPUPPCState *env)
+{
+    return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
+        (env->ca << XER_CA);
+}
+
+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->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
+}
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 425e79d..b559b67 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2343,18 +2343,8 @@ enum {
 
 /*****************************************************************************/
 
-static inline target_ulong cpu_read_xer(CPUPPCState *env)
-{
-    return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) | (env->ca << XER_CA);
-}
-
-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->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
-}
+target_ulong cpu_read_xer(CPUPPCState *env);
+void cpu_write_xer(CPUPPCState *env, target_ulong xer);
 
 static inline void cpu_get_tb_cpu_state(CPUPPCState *env, target_ulong *pc,
                                         target_ulong *cs_base, uint32_t *flags)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 01/11] target/ppc: move cpu_[read, write]_xer to cpu.c

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/Makefile.objs |  1 +
>  target/ppc/cpu.c         | 36 ++++++++++++++++++++++++++++++++++++
>  target/ppc/cpu.h         | 14 ++------------
>  3 files changed, 39 insertions(+), 12 deletions(-)
>  create mode 100644 target/ppc/cpu.c

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


r~

[Qemu-devel] [PATCH v2 02/11] target/ppc: optimize gen_write_xer()

[Nikunj A Dadhania]

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

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 3ba2616..b09e16f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3724,12 +3724,9 @@ static void gen_write_xer(TCGv src)
 {
     tcg_gen_andi_tl(cpu_xer, src,
                     ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA)));
-    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_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_extract_tl(cpu_so, src, XER_SO, 1);
+    tcg_gen_extract_tl(cpu_ov, src, XER_OV, 1);
+    tcg_gen_extract_tl(cpu_ca, src, XER_CA, 1);
 }
 
 /* mcrxr */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 02/11] target/ppc: optimize gen_write_xer()

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

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


r~

[Qemu-devel] [PATCH v2 03/11] 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.c            | 19 +++++++++++++++++--
 target/ppc/cpu.h            |  7 +++++++
 target/ppc/translate.c      | 29 ++++++++++++++++++++++++-----
 target/ppc/translate_init.c |  4 ++--
 4 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index de3004b..89c1ccb 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -23,8 +23,15 @@
 
 target_ulong cpu_read_xer(CPUPPCState *env)
 {
-    return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
+    target_ulong xer;
+
+    xer = env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
         (env->ca << XER_CA);
+
+    if (is_isa300(env)) {
+        xer |= (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
+    }
+    return xer;
 }
 
 void cpu_write_xer(CPUPPCState *env, target_ulong xer)
@@ -32,5 +39,13 @@ 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->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
+    if (is_isa300(env)) {
+        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));
+    } else {
+        env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
+    }
 }
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b559b67..ee2eb45 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,6 +2349,7 @@ enum {
 
 /*****************************************************************************/
 
+#define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
 target_ulong cpu_read_xer(CPUPPCState *env);
 void cpu_write_xer(CPUPPCState *env, target_ulong xer);
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b09e16f..c9f6768 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),
@@ -3703,7 +3707,7 @@ static void gen_tdi(DisasContext *ctx)
 
 /***                          Processor control                            ***/
 
-static void gen_read_xer(TCGv dst)
+static void gen_read_xer(DisasContext *ctx, TCGv dst)
 {
     TCGv t0 = tcg_temp_new();
     TCGv t1 = tcg_temp_new();
@@ -3715,15 +3719,30 @@ 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);
+    if (is_isa300(ctx)) {
+        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);
+    }
     tcg_temp_free(t0);
     tcg_temp_free(t1);
     tcg_temp_free(t2);
 }
 
-static void gen_write_xer(TCGv src)
+static void gen_write_xer(DisasContext *ctx, TCGv src)
 {
-    tcg_gen_andi_tl(cpu_xer, src,
-                    ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA)));
+    if (is_isa300(ctx)) {
+        tcg_gen_andi_tl(cpu_xer, src,
+                        ~((1u << XER_SO) |
+                          (1u << XER_OV) | (1u << XER_OV32) |
+                          (1u << XER_CA) | (1u << XER_CA32)));
+        tcg_gen_extract_tl(cpu_ov32, src, XER_OV32, 1);
+        tcg_gen_extract_tl(cpu_ca32, src, XER_CA32, 1);
+    } else {
+        tcg_gen_andi_tl(cpu_xer, src,
+                        ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA)));
+    }
     tcg_gen_extract_tl(cpu_so, src, XER_SO, 1);
     tcg_gen_extract_tl(cpu_ov, src, XER_OV, 1);
     tcg_gen_extract_tl(cpu_ca, src, XER_CA, 1);
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index be35cbd..eb667bb 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -107,12 +107,12 @@ static void spr_access_nop(DisasContext *ctx, int sprn, int gprn)
 /* XER */
 static void spr_read_xer (DisasContext *ctx, int gprn, int sprn)
 {
-    gen_read_xer(cpu_gpr[gprn]);
+    gen_read_xer(ctx, cpu_gpr[gprn]);
 }
 
 static void spr_write_xer (DisasContext *ctx, int sprn, int gprn)
 {
-    gen_write_xer(cpu_gpr[gprn]);
+    gen_write_xer(ctx, cpu_gpr[gprn]);
 }
 
 /* LR */
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 03/11] target/ppc: support for 32-bit carry and overflow

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
>  target_ulong cpu_read_xer(CPUPPCState *env)
>  {
> -    return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
> +    target_ulong xer;
> +
> +    xer = env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
>          (env->ca << XER_CA);
> +
> +    if (is_isa300(env)) {
> +        xer |= (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
> +    }
> +    return xer;
>  }
>
>  void cpu_write_xer(CPUPPCState *env, target_ulong xer)
> @@ -32,5 +39,13 @@ 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->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
> +    if (is_isa300(env)) {
> +        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));
> +    } else {
> +        env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
> +    }
>  }

Do cpus before power9 really save all of the bits you write to it?
I.e. if you write -1 to XER, do you read -1 back?

If so, then I suppose I'll have to revise my previous advice; there's no point 
in NOT storing bit 19 in env->ov32, because we can always read it back out.


r~

Re: [Qemu-devel] [PATCH v2 03/11] target/ppc: support for 32-bit carry and overflow

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
>>  target_ulong cpu_read_xer(CPUPPCState *env)
>>  {
>> -    return env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
>> +    target_ulong xer;
>> +
>> +    xer = env->xer | (env->so << XER_SO) | (env->ov << XER_OV) |
>>          (env->ca << XER_CA);
>> +
>> +    if (is_isa300(env)) {
>> +        xer |= (env->ov32 << XER_OV32) | (env->ca32 << XER_CA32);
>> +    }
>> +    return xer;
>>  }
>>
>>  void cpu_write_xer(CPUPPCState *env, target_ulong xer)
>> @@ -32,5 +39,13 @@ 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->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
>> +    if (is_isa300(env)) {
>> +        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));
>> +    } else {
>> +        env->xer = xer & ~((1u << XER_SO) | (1u << XER_OV) | (1u << XER_CA));
>> +    }
>>  }
>
> Do cpus before power9 really save all of the bits you write to it?
> I.e. if you write -1 to XER, do you read -1 back?

I did a quick test on power8, and it doesn't read back -1 after a write(-1).

int main(void)
{
  long res;
  asm volatile ("mtxer %1; mfxer %0;" : "=r"(res) : "r"(-1));
  printf("xer %lx\n", res);
}

$ ./a.out 
xer e00fffff
$

> If so, then I suppose I'll have to revise my previous advice; there's no point 
> in NOT storing bit 19 in env->ov32, because we can always read it back out.

Regards
Nikunj

[Qemu-devel] [PATCH v2 04/11] 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 | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c9f6768..b589d09 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -816,6 +816,23 @@ 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 res, TCGv arg0, TCGv arg1,
+                                             int add_ca, int sub)
+{
+    TCGv t0;
+
+    if (!is_isa300(ctx)) {
+        return;
+    }
+
+    t0 = tcg_temp_new();
+    tcg_gen_xor_tl(t0, arg0, arg1);
+    tcg_gen_xor_tl(t0, t0, res);
+    tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
+    tcg_temp_free(t0);
+}
+
 /* 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 +859,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 +868,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, t0, arg1, arg2, add_ca, 0);
             tcg_temp_free(zero);
         }
     } else {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 04/11] target/ppc: update ca32 in arithmetic add

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
> +            tcg_gen_mov_tl(cpu_ca32, cpu_ca);

Should also be conditional on isa300.


r~

Re: [Qemu-devel] [PATCH v2 04/11] target/ppc: update ca32 in arithmetic add

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
>> +            tcg_gen_mov_tl(cpu_ca32, cpu_ca);
>
> Should also be conditional on isa300.

Ah yes, will fix it.

Regards
Nikunj

[Qemu-devel] [PATCH v2 05/11] target/ppc: update ca32 in arithmetic substract

[Nikunj A Dadhania]

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

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b589d09..184d10f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -827,7 +827,15 @@ static inline void gen_op_arith_compute_ca32(DisasContext *ctx,
     }
 
     t0 = tcg_temp_new();
-    tcg_gen_xor_tl(t0, arg0, arg1);
+    if (!add_ca && sub) {
+        /* Invert arg0 before xor as in the !add_ca case,
+         * we do not get inverse of arg0
+         */
+        tcg_gen_not_tl(t0, arg0);
+        tcg_gen_xor_tl(t0, t0, arg1);
+    } else {
+        tcg_gen_xor_tl(t0, arg0, arg1);
+    }
     tcg_gen_xor_tl(t0, t0, res);
     tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
     tcg_temp_free(t0);
@@ -1382,11 +1390,13 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv 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);
+            gen_op_arith_compute_ca32(ctx, t0, inv1, arg2, add_ca, 1);
             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);
+            gen_op_arith_compute_ca32(ctx, t0, 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 v2 05/11] target/ppc: update ca32 in arithmetic substract

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b589d09..184d10f 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -827,7 +827,15 @@ static inline void gen_op_arith_compute_ca32(DisasContext *ctx,
>      }
>
>      t0 = tcg_temp_new();
> -    tcg_gen_xor_tl(t0, arg0, arg1);
> +    if (!add_ca && sub) {

I think checking add_ca is wrong.  And note that it is otherwise unused.

> +        /* Invert arg0 before xor as in the !add_ca case,
> +         * we do not get inverse of arg0
> +         */
> +        tcg_gen_not_tl(t0, arg0);
> +        tcg_gen_xor_tl(t0, t0, arg1);
> +    } else {
> +        tcg_gen_xor_tl(t0, arg0, arg1);
> +    }
>      tcg_gen_xor_tl(t0, t0, res);
>      tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
>      tcg_temp_free(t0);
> @@ -1382,11 +1390,13 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv 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);
> +            gen_op_arith_compute_ca32(ctx, t0, inv1, arg2, add_ca, 1);

The last parameter here should be 0.

>              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);
> +            gen_op_arith_compute_ca32(ctx, t0, arg1, arg2, add_ca, 1);
>          }
>      } else if (add_ca) {
>          /* Since we're ignoring carry-out, we can simplify the
>

r~

Re: [Qemu-devel] [PATCH v2 05/11] target/ppc: update ca32 in arithmetic substract

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  target/ppc/translate.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index b589d09..184d10f 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -827,7 +827,15 @@ static inline void gen_op_arith_compute_ca32(DisasContext *ctx,
>>      }
>>
>>      t0 = tcg_temp_new();
>> -    tcg_gen_xor_tl(t0, arg0, arg1);
>> +    if (!add_ca && sub) {
>
> I think checking add_ca is wrong.  And note that it is otherwise unused.

Yes, as suggested below (making last parameter as 0), I can just check
on "sub"

>> +        /* Invert arg0 before xor as in the !add_ca case,
>> +         * we do not get inverse of arg0
>> +         */
>> +        tcg_gen_not_tl(t0, arg0);
>> +        tcg_gen_xor_tl(t0, t0, arg1);
>> +    } else {
>> +        tcg_gen_xor_tl(t0, arg0, arg1);
>> +    }
>>      tcg_gen_xor_tl(t0, t0, res);
>>      tcg_gen_extract_tl(cpu_ca32, t0, 32, 1);
>>      tcg_temp_free(t0);
>> @@ -1382,11 +1390,13 @@ static inline void gen_op_arith_subf(DisasContext *ctx, TCGv ret, TCGv 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);
>> +            gen_op_arith_compute_ca32(ctx, t0, inv1, arg2, add_ca, 1);
>
> The last parameter here should be 0.

Sure.

Regards
Nikunj

[Qemu-devel] [PATCH v2 06/11] 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 | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 184d10f..74185ba 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -809,10 +809,18 @@ 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);
-    if (NARROW_MODE(ctx)) {
-        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
+    if (is_isa300(ctx)) {
+        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_mov_tl(cpu_ov, cpu_ov32);
+        }
+    } else {
+        if (NARROW_MODE(ctx)) {
+            tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
+        }
+        tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
     }
-    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 v2 06/11] target/ppc: update overflow flags for add/sub

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
> * 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 | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 184d10f..74185ba 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -809,10 +809,18 @@ 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);
> -    if (NARROW_MODE(ctx)) {
> -        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
> +    if (is_isa300(ctx)) {
> +        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_mov_tl(cpu_ov, cpu_ov32);
> +        }

Again, you're computing cpu_ov twice.

> +    } else {
> +        if (NARROW_MODE(ctx)) {
> +            tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
> +        }
> +        tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>      }
> -    tcg_gen_shri_tl(cpu_ov, cpu_ov, TARGET_LONG_BITS - 1);
>      tcg_gen_or_tl(cpu_so, cpu_so, cpu_ov);
>  }
>
>

Re: [Qemu-devel] [PATCH v2 06/11] target/ppc: update overflow flags for add/sub

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
>> * 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 | 14 +++++++++++---
>>  1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 184d10f..74185ba 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -809,10 +809,18 @@ 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);
>> -    if (NARROW_MODE(ctx)) {
>> -        tcg_gen_ext32s_tl(cpu_ov, cpu_ov);
>> +    if (is_isa300(ctx)) {
>> +        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_mov_tl(cpu_ov, cpu_ov32);
>> +        }
>
> Again, you're computing cpu_ov twice.

I lost the corrected code in patch juggling :(

Regards,
Nikunj

[Qemu-devel] [PATCH v2 07/11] 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 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 74185ba..eecdfe9 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1483,7 +1483,10 @@ 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)]);
+    if (unlikely(Rc(ctx->opcode))) {
+        gen_set_Rc0(ctx, cpu_gpr[rD(ctx->opcode)]);
+    }
 }
 
 static void gen_nego(DisasContext *ctx)
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 07/11] target/ppc: use tcg ops for neg instruction

[Richard Henderson]

On 02/22/2017 08:29 PM, Nikunj A Dadhania wrote:
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target/ppc/translate.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

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


r~

[Qemu-devel] [PATCH v2 08/11] 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 | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index eecdfe9..2a9f508 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)]);
@@ -1491,7 +1483,23 @@ 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)]);
+    if (unlikely(Rc(ctx->opcode))) {
+        gen_set_Rc0(ctx, cpu_gpr[rD(ctx->opcode)]);
+    }
+    tcg_temp_free(t0);
+    tcg_temp_free(zero);
 }
 
 /***                            Integer logical                            ***/
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 08/11] target/ppc: update ov/ov32 for nego

[Richard Henderson]

On 02/22/2017 08:29 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 | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index eecdfe9..2a9f508 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)]);
> @@ -1491,7 +1483,23 @@ 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);

I think we just now covered this is wrong in the v1 thread.


> +    tcg_gen_neg_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> +    if (unlikely(Rc(ctx->opcode))) {
> +        gen_set_Rc0(ctx, cpu_gpr[rD(ctx->opcode)]);
> +    }
> +    tcg_temp_free(t0);
> +    tcg_temp_free(zero);
>  }
>
>  /***                            Integer logical                            ***/
>

Re: [Qemu-devel] [PATCH v2 08/11] target/ppc: update ov/ov32 for nego

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 02/22/2017 08:29 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 | 26 +++++++++++++++++---------
>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index eecdfe9..2a9f508 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)]);
>> @@ -1491,7 +1483,23 @@ 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);
>
> I think we just now covered this is wrong in the v1 thread.

With respect to the simulator, right?

I will restore the same neg/nego behaviour using subf, as OV/OV32 will
be updated as per the simulator.

Regards
Nikunj

Re: [Qemu-devel] [PATCH v2 08/11] target/ppc: update ov/ov32 for nego

[Richard Henderson]

On 02/22/2017 10:03 PM, Nikunj A Dadhania wrote:
> Richard Henderson <rth@twiddle.net> writes:
>
>> On 02/22/2017 08:29 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 | 26 +++++++++++++++++---------
>>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>>> index eecdfe9..2a9f508 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)]);
>>> @@ -1491,7 +1483,23 @@ 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);
>>
>> I think we just now covered this is wrong in the v1 thread.
>
> With respect to the simulator, right?
>
> I will restore the same neg/nego behaviour using subf, as OV/OV32 will
> be updated as per the simulator.

Yes.  It's the only solution that makes sense to me.


r~

[Qemu-devel] [PATCH v2 09/11] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 target/ppc/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 2a9f508..d676863 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1288,6 +1288,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);
@@ -1349,6 +1350,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 v2 09/11] target/ppc: add ov32 flag for multiply low insns

[Richard Henderson]

On 02/22/2017 08:29 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>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> ---
>  target/ppc/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 2a9f508..d676863 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1288,6 +1288,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);
> @@ -1349,6 +1350,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);

isa300 checks?


r~

[Qemu-devel] [PATCH v2 10/11] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 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 d676863..1423ebd 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -1024,6 +1024,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);
@@ -1095,6 +1096,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);
@@ -1113,10 +1115,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 v2 10/11] target/ppc: add ov32 flag in divide operations

[Richard Henderson]

On 02/22/2017 08:29 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>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> ---
>  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 d676863..1423ebd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -1024,6 +1024,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);
> @@ -1095,6 +1096,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);

isa300 checks?


r~

>      }
>      tcg_temp_free_i64(t0);
> @@ -1113,10 +1115,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);
>
>

[Qemu-devel] [PATCH v2 11/11] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 target/ppc/translate.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 1423ebd..34bc704 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3823,6 +3823,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)
 {
@@ -6492,6 +6514,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