[Qemu-devel] [RFC v1 00/13] POWER9 TCG enablements - part1

[Qemu-devel] [RFC v1 00/13] POWER9 TCG enablements - part1

[Nikunj A Dadhania]

This set starts adding new instructions for POWER9 described in ISA3.0.

Patches:
  01-02: First two patches adds the required POWER9 cpu model and ISA defines.
  03-12: Adds following instructions:
             addpcis   : Add PC Immediate Shifted
             cmprb     : Compare Ranged Byte
             moduw     : Modulo Unsigned Word
             modsw     : Modulo Signed Word
             modud     : Modulo Unsigned Dword
             modsd     : Modulo Signed Dword
             cnttzd[.] : Count Trailing Zero Dword
             cnttzw[.] : Count Trailing Zero Word
             cmpeqb    : Compare Equal Byte
             setb      : Set Boolean
             maddld    : Multiply-Add Low Dword
             maddhd    : Multiply-Add High Dword
             maddhdu   : Multiply-Add High Dword Unsigned
     13: Adds support for the new Expanded Opcode (EO) added in ISA3.0

Aneesh Kumar K.V (1):
  target-ppc: Introduce Power9 family

Nikunj A Dadhania (9):
  target-ppc: Introduce POWER ISA 3.0 flag
  target-ppc: adding addpcis instruction
  target-ppc: add cmprb instruction
  target-ppc: add modulo word operations
  target-ppc: add modulo dword operations
  target-ppc: add cnttzw[.] instruction
  target-ppc: add maddld instruction
  target-ppc: add maddhd and maddhdu instruction
  target-ppc: introduce opc4 for Expanded Opcode

Sandipan Das (1):
  target-ppc: add cnttzd[.] instruction

Swapnil Bokade (1):
  target-ppc: add cmpeqb instruction

Vivek Andrew Sha (1):
  target-ppc: add setb instruction

 hw/ppc/spapr_cpu_core.c     |   5 +
 target-ppc/cpu-models.c     |   5 +
 target-ppc/cpu-models.h     |   1 +
 target-ppc/cpu-qom.h        |   7 +
 target-ppc/cpu.h            |   5 +-
 target-ppc/helper.h         |   2 +
 target-ppc/int_helper.c     |  10 ++
 target-ppc/mmu_helper.c     |   3 +-
 target-ppc/translate.c      | 369 ++++++++++++++++++++++++++++++++++++++++++--
 target-ppc/translate_init.c | 188 ++++++++++++++++++----
 10 files changed, 552 insertions(+), 43 deletions(-)

-- 
2.7.4

[Qemu-devel] [RFC v1 01/13] target-ppc: Introduce Power9 family

[Nikunj A Dadhania]

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
[ rebased and added POWER9 alias and POWER9 SPAPR core ]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 hw/ppc/spapr_cpu_core.c     |  5 +++
 target-ppc/cpu-models.c     |  5 +++
 target-ppc/cpu-models.h     |  1 +
 target-ppc/cpu-qom.h        |  7 ++++
 target-ppc/mmu_helper.c     |  3 +-
 target-ppc/translate_init.c | 85 ++++++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 9347f07..766c515 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -351,6 +351,7 @@ SPAPR_CPU_CORE_INITFN(POWER7+_v2.1, POWER7plus);
 SPAPR_CPU_CORE_INITFN(POWER8_v2.0, POWER8);
 SPAPR_CPU_CORE_INITFN(POWER8E_v2.1, POWER8E);
 SPAPR_CPU_CORE_INITFN(POWER8NVL_v1.0, POWER8NVL);
+SPAPR_CPU_CORE_INITFN(POWER9_v1.0, POWER9);
 
 typedef struct SPAPRCoreInfo {
     const char *name;
@@ -394,6 +395,10 @@ static const SPAPRCoreInfo spapr_cores[] = {
     { .name = "POWER8NVL_v1.0", .initfn = spapr_cpu_core_POWER8NVL_initfn },
     { .name = "POWER8NVL", .initfn = spapr_cpu_core_POWER8NVL_initfn },
 
+    /* POWER9 and aliases */
+    { .name = "POWER9_v1.0", .initfn = spapr_cpu_core_POWER9_initfn },
+    { .name = "POWER9", .initfn = spapr_cpu_core_POWER9_initfn },
+
     { .name = NULL }
 };
 
diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index 5209e63..901cf40 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -1147,6 +1147,10 @@
                 "POWER8NVL v1.0")
     POWERPC_DEF("970_v2.2",      CPU_POWERPC_970_v22,                970,
                 "PowerPC 970 v2.2")
+
+    POWERPC_DEF("POWER9_v1.0",   CPU_POWERPC_POWER9_BASE,            POWER9,
+                "POWER9 v1.0")
+
     POWERPC_DEF("970fx_v1.0",    CPU_POWERPC_970FX_v10,              970,
                 "PowerPC 970FX v1.0 (G5)")
     POWERPC_DEF("970fx_v2.0",    CPU_POWERPC_970FX_v20,              970,
@@ -1395,6 +1399,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
     { "POWER8E", "POWER8E_v2.1" },
     { "POWER8", "POWER8_v2.0" },
     { "POWER8NVL", "POWER8NVL_v1.0" },
+    { "POWER9", "POWER9_v1.0" },
     { "970", "970_v2.2" },
     { "970fx", "970fx_v3.1" },
     { "970mp", "970mp_v1.1" },
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index f21a44c..7d9e6a2 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -562,6 +562,7 @@ enum {
     CPU_POWERPC_POWER8_v20         = 0x004D0200,
     CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,
     CPU_POWERPC_POWER8NVL_v10      = 0x004C0100,
+    CPU_POWERPC_POWER9_BASE        = 0x004E0000,
     CPU_POWERPC_970_v22            = 0x00390202,
     CPU_POWERPC_970FX_v10          = 0x00391100,
     CPU_POWERPC_970FX_v20          = 0x003C0200,
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 2864105..df2fb65 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -86,6 +86,13 @@ enum powerpc_mmu_t {
     POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
                              | POWERPC_MMU_64K
                              | POWERPC_MMU_AMR | 0x00000004,
+    /* for now , We can add radix later if needed */
+    /* POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
+     * | POWERPC_MMU_AMR | 0x00000005,
+     */
+
+    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_AMR | 0x00000005,
+
     /* Architecture 2.07 "degraded" (no 1T segments)           */
     POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
                              | 0x00000004,
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 3eb3cd7..dd0cf33 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -1935,13 +1935,14 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
     case POWERPC_MMU_2_06a:
     case POWERPC_MMU_2_07:
     case POWERPC_MMU_2_07a:
+    case POWERPC_MMU_3_00:
 #endif /* defined(TARGET_PPC64) */
         env->tlb_need_flush = 0;
         tlb_flush(CPU(cpu), 1);
         break;
     default:
         /* XXX: TODO */
-        cpu_abort(CPU(cpu), "Unknown MMU model\n");
+        cpu_abort(CPU(cpu), "Unknown MMU model %d\n", env->mmu_model);
         break;
     }
 }
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 7cb7842..5b100d0 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7459,7 +7459,8 @@ enum BOOK3S_CPU_TYPE {
     BOOK3S_CPU_POWER5PLUS,
     BOOK3S_CPU_POWER6,
     BOOK3S_CPU_POWER7,
-    BOOK3S_CPU_POWER8
+    BOOK3S_CPU_POWER8,
+    BOOK3S_CPU_POWER9
 };
 
 static void gen_fscr_facility_check(DisasContext *ctx, int facility_sprn,
@@ -8241,6 +8242,7 @@ static void init_proc_book3s_64(CPUPPCState *env, int version)
         break;
     case BOOK3S_CPU_POWER7:
     case BOOK3S_CPU_POWER8:
+    case BOOK3S_CPU_POWER9:
         gen_spr_book3s_ids(env);
         gen_spr_amr(env, version >= BOOK3S_CPU_POWER8);
         gen_spr_book3s_purr(env);
@@ -8293,6 +8295,7 @@ static void init_proc_book3s_64(CPUPPCState *env, int version)
         break;
     case BOOK3S_CPU_POWER7:
     case BOOK3S_CPU_POWER8:
+    case BOOK3S_CPU_POWER9:
     default:
         env->slb_nr = 32;
         break;
@@ -8310,6 +8313,7 @@ static void init_proc_book3s_64(CPUPPCState *env, int version)
         ppcPOWER7_irq_init(ppc_env_get_cpu(env));
         break;
     case BOOK3S_CPU_POWER8:
+    case BOOK3S_CPU_POWER9:
         init_excp_POWER8(env);
         ppcPOWER7_irq_init(ppc_env_get_cpu(env));
         break;
@@ -8772,6 +8776,85 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     pcc->l1_icache_size = 0x8000;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
+static void init_proc_POWER9(CPUPPCState *env)
+{
+    init_proc_book3s_64(env, BOOK3S_CPU_POWER9);
+}
+
+static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER9_BASE) {
+        return true;
+    }
+    return false;
+}
+
+POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
+
+    dc->fw_name = "PowerPC,POWER9";
+    dc->desc = "POWER9";
+    dc->props = powerpc_servercpu_properties;
+    pcc->pvr_match = ppc_pvr_match_power9;
+    pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06 | PCR_COMPAT_2_07;
+    pcc->init_proc = init_proc_POWER9;
+    pcc->check_pow = check_pow_nocheck;
+    pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
+                       PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
+                       PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
+                       PPC_FLOAT_FRSQRTES |
+                       PPC_FLOAT_STFIWX |
+                       PPC_FLOAT_EXT |
+                       PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
+                       PPC_MEM_SYNC | PPC_MEM_EIEIO |
+                       PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
+                       PPC_64B | PPC_64BX | PPC_ALTIVEC |
+                       PPC_SEGMENT_64B | PPC_SLBI |
+                       PPC_POPCNTB | PPC_POPCNTWD |
+                       PPC_CILDST;
+    pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
+                        PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
+                        PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
+                        PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
+                        PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
+                        PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
+                        PPC2_TM | PPC2_PM_ISA206;
+    pcc->msr_mask = (1ull << MSR_SF) |
+                    (1ull << MSR_TM) |
+                    (1ull << MSR_VR) |
+                    (1ull << MSR_VSX) |
+                    (1ull << MSR_EE) |
+                    (1ull << MSR_PR) |
+                    (1ull << MSR_FP) |
+                    (1ull << MSR_ME) |
+                    (1ull << MSR_FE0) |
+                    (1ull << MSR_SE) |
+                    (1ull << MSR_DE) |
+                    (1ull << MSR_FE1) |
+                    (1ull << MSR_IR) |
+                    (1ull << MSR_DR) |
+                    (1ull << MSR_PMM) |
+                    (1ull << MSR_RI) |
+                    (1ull << MSR_LE);
+    pcc->mmu_model = POWERPC_MMU_3_00;
+#if defined(CONFIG_SOFTMMU)
+    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
+    /* segment page size remain the same */
+    pcc->sps = &POWER7_POWER8_sps;
+#endif
+    pcc->excp_model = POWERPC_EXCP_POWER8;
+    pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
+    pcc->bfd_mach = bfd_mach_ppc64;
+    pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
+                 POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
+                 POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
+                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
+    pcc->l1_dcache_size = 0x8000;
+    pcc->l1_icache_size = 0x8000;
+    pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
+}
 
 #if !defined(CONFIG_USER_ONLY)
 
-- 
2.7.4

[Qemu-devel] [RFC v1 02/13] target-ppc: Introduce POWER ISA 3.0 flag

[Nikunj A Dadhania]

This flag will be used for POWER9 instructions.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/cpu.h            | 5 ++++-
 target-ppc/translate_init.c | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 5fce1ff..c499315 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2094,6 +2094,8 @@ enum {
     PPC2_TM            = 0x0000000000020000ULL,
     /* Server PM instructgions (ISA 2.06, Book III)                          */
     PPC2_PM_ISA206     = 0x0000000000040000ULL,
+    /* POWER ISA 3.0                                                         */
+    PPC2_ISA300        = 0x0000000000080000ULL,
 
 #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
                         PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
@@ -2101,7 +2103,8 @@ enum {
                         PPC2_FP_CVT_ISA206 | PPC2_FP_TST_ISA206 | \
                         PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
                         PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
-                        PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206)
+                        PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
+                        PPC2_ISA300)
 };
 
 /*****************************************************************************/
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5b100d0..d207f68 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8820,7 +8820,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
                         PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
                         PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
                         PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
-                        PPC2_TM | PPC2_PM_ISA206;
+                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_TM) |
                     (1ull << MSR_VR) |
-- 
2.7.4

[Qemu-devel] [RFC v1 03/13] target-ppc: adding addpcis instruction

[Nikunj A Dadhania]

ISA 3.0 instruction for adding immediate value with next instruction
address and return the result in the target register.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
---
 target-ppc/translate.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 92030b6..93c7c66 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -432,6 +432,20 @@ static inline uint32_t name(uint32_t opcode)                                  \
     return (((opcode >> (shift1)) & ((1 << (nb1)) - 1)) << nb2) |             \
             ((opcode >> (shift2)) & ((1 << (nb2)) - 1));                      \
 }
+
+#define EXTRACT_HELPER_DXFORM(name,                                           \
+                              d0_bits, shift_op_d0, shift_d0,                 \
+                              d1_bits, shift_op_d1, shift_d1,                 \
+                              d2_bits, shift_op_d2, shift_d2)                 \
+static inline int16_t name(uint32_t opcode)                                   \
+{                                                                             \
+    return                                                                    \
+        (((opcode >> (shift_op_d0)) & ((1 << (d0_bits)) - 1)) << (shift_d0)) | \
+        (((opcode >> (shift_op_d1)) & ((1 << (d1_bits)) - 1)) << (shift_d1)) | \
+        (((opcode >> (shift_op_d2)) & ((1 << (d2_bits)) - 1)) << (shift_d2));  \
+}
+
+
 /* Opcode part 1 */
 EXTRACT_HELPER(opc1, 26, 6);
 /* Opcode part 2 */
@@ -501,6 +515,9 @@ EXTRACT_HELPER(FPL, 25, 1);
 EXTRACT_HELPER(FPFLM, 17, 8);
 EXTRACT_HELPER(FPW, 16, 1);
 
+/* addpcis */
+EXTRACT_HELPER_DXFORM(DX, 10, 6, 6, 5, 16, 1, 1, 0, 0)
+
 /***                            Jump target decoding                       ***/
 /* Immediate address */
 static inline target_ulong LI(uint32_t opcode)
@@ -984,6 +1001,15 @@ static void gen_addis(DisasContext *ctx)
     }
 }
 
+/* addpcis */
+static void gen_addpcis(DisasContext *ctx)
+{
+    target_long d = DX(ctx->opcode);
+
+    tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], ctx->nip);
+    tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rD(ctx->opcode)], d);
+}
+
 static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, TCGv arg1,
                                      TCGv arg2, int sign, int compute_ov)
 {
@@ -9877,6 +9903,7 @@ GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(addis, 0x0F, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
+GEN_HANDLER_E(addpcis, 0x13, 0x2, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(mulhw, 0x1F, 0x0B, 0x02, 0x00000400, PPC_INTEGER),
 GEN_HANDLER(mulhwu, 0x1F, 0x0B, 0x00, 0x00000400, PPC_INTEGER),
 GEN_HANDLER(mullw, 0x1F, 0x0B, 0x07, 0x00000000, PPC_INTEGER),
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 03/13] target-ppc: adding addpcis instruction

[Nikunj A Dadhania]

Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:

> ISA 3.0 instruction for adding immediate value with next instruction
> address and return the result in the target register.
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  target-ppc/translate.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 92030b6..93c7c66 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -432,6 +432,20 @@ static inline uint32_t name(uint32_t opcode)                                  \
>      return (((opcode >> (shift1)) & ((1 << (nb1)) - 1)) << nb2) |             \
>              ((opcode >> (shift2)) & ((1 << (nb2)) - 1));                      \
>  }
> +
> +#define EXTRACT_HELPER_DXFORM(name,                                           \
> +                              d0_bits, shift_op_d0, shift_d0,                 \
> +                              d1_bits, shift_op_d1, shift_d1,                 \
> +                              d2_bits, shift_op_d2, shift_d2)                 \
> +static inline int16_t name(uint32_t opcode)                                   \
> +{                                                                             \
> +    return                                                                    \
> +        (((opcode >> (shift_op_d0)) & ((1 << (d0_bits)) - 1)) << (shift_d0)) | \
> +        (((opcode >> (shift_op_d1)) & ((1 << (d1_bits)) - 1)) << (shift_d1)) | \
> +        (((opcode >> (shift_op_d2)) & ((1 << (d2_bits)) - 1)) << (shift_d2));  \
> +}
> +
> +
>  /* Opcode part 1 */
>  EXTRACT_HELPER(opc1, 26, 6);
>  /* Opcode part 2 */
> @@ -501,6 +515,9 @@ EXTRACT_HELPER(FPL, 25, 1);
>  EXTRACT_HELPER(FPFLM, 17, 8);
>  EXTRACT_HELPER(FPW, 16, 1);
>
> +/* addpcis */
> +EXTRACT_HELPER_DXFORM(DX, 10, 6, 6, 5, 16, 1, 1, 0, 0)
> +
>  /***                            Jump target decoding                       ***/
>  /* Immediate address */
>  static inline target_ulong LI(uint32_t opcode)
> @@ -984,6 +1001,15 @@ static void gen_addis(DisasContext *ctx)
>      }
>  }
>
> +/* addpcis */
> +static void gen_addpcis(DisasContext *ctx)
> +{
> +    target_long d = DX(ctx->opcode);

I found a bug here, d has to be shifted by 16. Will update the patch
accordingly.

> +
> +    tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], ctx->nip);
> +    tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rD(ctx->opcode)], d);
> +}
> +

Regards
Nikunj

[Qemu-devel] [RFC v1 04/13] target-ppc: add cmprb instruction

[Nikunj A Dadhania]

ISA 3.0 Compare Ranged Byte instruction useful for
isupper/islower/isaplha kind of operation.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
[ Initialize CR to 0]
---
 target-ppc/translate.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 93c7c66..d44f7af 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -817,6 +817,45 @@ static void gen_cmpli(DisasContext *ctx)
     }
 }
 
+/* cmprb - range comparison: isupper, isaplha, islower*/
+static void gen_cmprb(DisasContext *ctx)
+{
+    TCGLabel *lab1 = gen_new_label();
+    TCGLabel *lab2 = gen_new_label();
+    TCGv src1 = tcg_temp_local_new();
+    TCGv src2 = tcg_temp_local_new();
+    TCGv src2lo = tcg_temp_local_new();
+    TCGv src2hi = tcg_temp_local_new();
+
+    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
+    tcg_gen_andi_tl(src2, cpu_gpr[rB(ctx->opcode)], 0xFFFFFFFF);
+    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
+    tcg_gen_andi_tl(src2lo, src2, 0xFF);
+    tcg_gen_shri_tl(src2hi, src2, 8);
+    tcg_gen_andi_tl(src2hi, src2hi, 0xFF);
+    tcg_gen_brcond_tl(TCG_COND_GTU, src1, src2hi, lab1);
+    tcg_gen_brcond_tl(TCG_COND_LTU, src1, src2lo, lab1);
+    /* set in_range bit, i.e. CRF_GT */
+    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
+    tcg_gen_br(lab2);
+    gen_set_label(lab1);
+
+    if (ctx->opcode & 0x00200000) {
+        tcg_gen_shri_tl(src2hi, src2, 24);
+        tcg_gen_andi_tl(src2hi, src2hi, 0xFF);
+        tcg_gen_shri_tl(src2lo, src2, 16);
+        tcg_gen_andi_tl(src2lo, src2lo, 0xFF);
+        tcg_gen_brcond_tl(TCG_COND_GTU, src1, src2hi, lab2);
+        tcg_gen_brcond_tl(TCG_COND_LTU, src1, src2lo, lab2);
+        tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
+    }
+    gen_set_label(lab2);
+    tcg_temp_free(src1);
+    tcg_temp_free(src2);
+    tcg_temp_free(src2lo);
+    tcg_temp_free(src2hi);
+}
+
 /* isel (PowerPC 2.03 specification) */
 static void gen_isel(DisasContext *ctx)
 {
@@ -9898,6 +9937,7 @@ GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
 GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_INTEGER),
 GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
 GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
+GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),
 GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
-- 
2.7.4

[Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[Nikunj A Dadhania]

Adding following instructions:

moduw: Modulo Unsigned Word
modsw: Modulo Signed Word

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

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index d44f7af..487dd94 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
 GEN_DIVE(divdeo, divde, 1);
 #endif
 
+static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
+                                     TCGv arg2, int sign)
+{
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *l2 = gen_new_label();
+    TCGv_i32 t0 = tcg_temp_local_new_i32();
+    TCGv_i32 t1 = tcg_temp_local_new_i32();
+    TCGv_i32 t2 = tcg_temp_local_new_i32();
+
+    tcg_gen_trunc_tl_i32(t0, arg1);
+    tcg_gen_trunc_tl_i32(t1, arg2);
+    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
+    if (sign) {
+        TCGLabel *l3 = gen_new_label();
+        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
+        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
+        gen_set_label(l3);
+        tcg_gen_rem_i32(t2, t0, t1);
+    } else {
+        tcg_gen_remu_i32(t2, t0, t1);
+    }
+    tcg_gen_br(l2);
+    gen_set_label(l1);
+    if (sign) {
+        tcg_gen_sari_i32(t2, t0, 31);
+    } else {
+        tcg_gen_movi_i32(t2, 0);
+    }
+    gen_set_label(l2);
+    tcg_gen_extu_i32_tl(ret, t2);
+    tcg_temp_free_i32(t0);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t2);
+}
+
+#define GEN_INT_ARITH_MODW(name, opc3, sign)                                \
+static void glue(gen_, name)(DisasContext *ctx)                             \
+{                                                                           \
+    gen_op_arith_modw(ctx, cpu_gpr[rD(ctx->opcode)],                        \
+                      cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],   \
+                      sign);                                                \
+}
+
+GEN_INT_ARITH_MODW(modsw, 0x18, 1);
+GEN_INT_ARITH_MODW(moduw, 0x08, 0);
+
 /* mulhw  mulhw. */
 static void gen_mulhw(DisasContext *ctx)
 {
@@ -10244,6 +10290,8 @@ GEN_HANDLER_E(divwe, 0x1F, 0x0B, 0x0D, 0, PPC_NONE, PPC2_DIVE_ISA206),
 GEN_HANDLER_E(divweo, 0x1F, 0x0B, 0x1D, 0, PPC_NONE, PPC2_DIVE_ISA206),
 GEN_HANDLER_E(divweu, 0x1F, 0x0B, 0x0C, 0, PPC_NONE, PPC2_DIVE_ISA206),
 GEN_HANDLER_E(divweuo, 0x1F, 0x0B, 0x1C, 0, PPC_NONE, PPC2_DIVE_ISA206),
+GEN_HANDLER_E(modsw, 0x1F, 0x0B, 0x18, 0x00000001, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(moduw, 0x1F, 0x0B, 0x08, 0x00000001, PPC_NONE, PPC2_ISA300),
 
 #if defined(TARGET_PPC64)
 #undef GEN_INT_ARITH_DIVD
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[David Gibson]

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

On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote:
> Adding following instructions:
> 
> moduw: Modulo Unsigned Word
> modsw: Modulo Signed Word
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

As rth has already mentioned this many branches probably means this
wants a helper.

> ---
>  target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index d44f7af..487dd94 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
>  GEN_DIVE(divdeo, divde, 1);
>  #endif
>  
> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
> +                                     TCGv arg2, int sign)
> +{
> +    TCGLabel *l1 = gen_new_label();
> +    TCGLabel *l2 = gen_new_label();
> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
> +    TCGv_i32 t1 = tcg_temp_local_new_i32();
> +    TCGv_i32 t2 = tcg_temp_local_new_i32();
> +
> +    tcg_gen_trunc_tl_i32(t0, arg1);
> +    tcg_gen_trunc_tl_i32(t1, arg2);
> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
> +    if (sign) {
> +        TCGLabel *l3 = gen_new_label();
> +        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
> +        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
> +        gen_set_label(l3);

It's not really clear to be what the logic above is doing.

> +        tcg_gen_rem_i32(t2, t0, t1);
> +    } else {
> +        tcg_gen_remu_i32(t2, t0, t1);
> +    }
> +    tcg_gen_br(l2);
> +    gen_set_label(l1);
> +    if (sign) {
> +        tcg_gen_sari_i32(t2, t0, 31);

AFAICT this sets t2 to either 0 or -1 depending on the sign of t0,
which seems like an odd thing to do.

> +    } else {
> +        tcg_gen_movi_i32(t2, 0);
> +    }
> +    gen_set_label(l2);
> +    tcg_gen_extu_i32_tl(ret, t2);
> +    tcg_temp_free_i32(t0);
> +    tcg_temp_free_i32(t1);
> +    tcg_temp_free_i32(t2);
> +}
> +
> +#define GEN_INT_ARITH_MODW(name, opc3, sign)                                \
> +static void glue(gen_, name)(DisasContext *ctx)                             \
> +{                                                                           \
> +    gen_op_arith_modw(ctx, cpu_gpr[rD(ctx->opcode)],                        \
> +                      cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],   \
> +                      sign);                                                \
> +}
> +
> +GEN_INT_ARITH_MODW(modsw, 0x18, 1);
> +GEN_INT_ARITH_MODW(moduw, 0x08, 0);
> +
>  /* mulhw  mulhw. */
>  static void gen_mulhw(DisasContext *ctx)
>  {
> @@ -10244,6 +10290,8 @@ GEN_HANDLER_E(divwe, 0x1F, 0x0B, 0x0D, 0, PPC_NONE, PPC2_DIVE_ISA206),
>  GEN_HANDLER_E(divweo, 0x1F, 0x0B, 0x1D, 0, PPC_NONE, PPC2_DIVE_ISA206),
>  GEN_HANDLER_E(divweu, 0x1F, 0x0B, 0x0C, 0, PPC_NONE, PPC2_DIVE_ISA206),
>  GEN_HANDLER_E(divweuo, 0x1F, 0x0B, 0x1C, 0, PPC_NONE, PPC2_DIVE_ISA206),
> +GEN_HANDLER_E(modsw, 0x1F, 0x0B, 0x18, 0x00000001, PPC_NONE, PPC2_ISA300),
> +GEN_HANDLER_E(moduw, 0x1F, 0x0B, 0x08, 0x00000001, PPC_NONE, PPC2_ISA300),
>  
>  #if defined(TARGET_PPC64)
>  #undef GEN_INT_ARITH_DIVD

-- 
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 --]

Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote:
>> Adding following instructions:
>> 
>> moduw: Modulo Unsigned Word
>> modsw: Modulo Signed Word
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>
> As rth has already mentioned this many branches probably means this
> wants a helper.
>
>> ---
>>  target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 48 insertions(+)
>> 
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index d44f7af..487dd94 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
>>  GEN_DIVE(divdeo, divde, 1);
>>  #endif
>>  
>> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
>> +                                     TCGv arg2, int sign)
>> +{
>> +    TCGLabel *l1 = gen_new_label();
>> +    TCGLabel *l2 = gen_new_label();
>> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
>> +    TCGv_i32 t1 = tcg_temp_local_new_i32();
>> +    TCGv_i32 t2 = tcg_temp_local_new_i32();
>> +
>> +    tcg_gen_trunc_tl_i32(t0, arg1);
>> +    tcg_gen_trunc_tl_i32(t1, arg2);
>> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);

Result for:
<anything> % 0 and ...

>> +    if (sign) {
>> +        TCGLabel *l3 = gen_new_label();
>> +        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
>> +        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
>> +        gen_set_label(l3);
>
> It's not really clear to be what the logic above is doing.

... For signed case
0x8000_0000 % -1

Is undefined, addressing those cases.

>
>> +        tcg_gen_rem_i32(t2, t0, t1);
>> +    } else {
>> +        tcg_gen_remu_i32(t2, t0, t1);
>> +    }
>> +    tcg_gen_br(l2);
>> +    gen_set_label(l1);
>> +    if (sign) {
>> +        tcg_gen_sari_i32(t2, t0, 31);
>
> AFAICT this sets t2 to either 0 or -1 depending on the sign of t0,
> which seems like an odd thing to do.

Extending the sign later ...

>> +    } else {
>> +        tcg_gen_movi_i32(t2, 0);
>> +    }
>> +    gen_set_label(l2);
>> +    tcg_gen_extu_i32_tl(ret, t2);

... Here.

Regards
Nikunj

Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[David Gibson]

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

On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > [ Unknown signature status ]
> > On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote:
> >> Adding following instructions:
> >> 
> >> moduw: Modulo Unsigned Word
> >> modsw: Modulo Signed Word
> >> 
> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >
> > As rth has already mentioned this many branches probably means this
> > wants a helper.
> >
> >> ---
> >>  target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 48 insertions(+)
> >> 
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index d44f7af..487dd94 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
> >>  GEN_DIVE(divdeo, divde, 1);
> >>  #endif
> >>  
> >> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
> >> +                                     TCGv arg2, int sign)
> >> +{
> >> +    TCGLabel *l1 = gen_new_label();
> >> +    TCGLabel *l2 = gen_new_label();
> >> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
> >> +    TCGv_i32 t1 = tcg_temp_local_new_i32();
> >> +    TCGv_i32 t2 = tcg_temp_local_new_i32();
> >> +
> >> +    tcg_gen_trunc_tl_i32(t0, arg1);
> >> +    tcg_gen_trunc_tl_i32(t1, arg2);
> >> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
> 
> Result for:
> <anything> % 0 and ...
> 
> >> +    if (sign) {
> >> +        TCGLabel *l3 = gen_new_label();
> >> +        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
> >> +        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
> >> +        gen_set_label(l3);
> >
> > It's not really clear to be what the logic above is doing.
> 
> ... For signed case
> 0x8000_0000 % -1
> 
> Is undefined, addressing those cases.

Do you mean the tcg operations have undefined results or that the ppc
instructions have undefined results?  If the latter, then why do you
care about those cases?

> >> +        tcg_gen_rem_i32(t2, t0, t1);
> >> +    } else {
> >> +        tcg_gen_remu_i32(t2, t0, t1);
> >> +    }
> >> +    tcg_gen_br(l2);
> >> +    gen_set_label(l1);
> >> +    if (sign) {
> >> +        tcg_gen_sari_i32(t2, t0, 31);
> >
> > AFAICT this sets t2 to either 0 or -1 depending on the sign of t0,
> > which seems like an odd thing to do.
> 
> Extending the sign later ...

Right, so after sign extension you have a 64-bit 0 or -1.  Still not
seeing what that 0 or -1 result is useful for.

> 
> >> +    } else {
> >> +        tcg_gen_movi_i32(t2, 0);
> >> +    }
> >> +    gen_set_label(l2);
> >> +    tcg_gen_extu_i32_tl(ret, t2);
> 
> ... Here.
> 
> 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 --]

Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>> > [ Unknown signature status ]
>> > On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote:
>> >> Adding following instructions:
>> >> 
>> >> moduw: Modulo Unsigned Word
>> >> modsw: Modulo Signed Word
>> >> 
>> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> >
>> > As rth has already mentioned this many branches probably means this
>> > wants a helper.
>> >
>> >> ---
>> >>  target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  1 file changed, 48 insertions(+)
>> >> 
>> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> >> index d44f7af..487dd94 100644
>> >> --- a/target-ppc/translate.c
>> >> +++ b/target-ppc/translate.c
>> >> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
>> >>  GEN_DIVE(divdeo, divde, 1);
>> >>  #endif
>> >>  
>> >> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
>> >> +                                     TCGv arg2, int sign)
>> >> +{
>> >> +    TCGLabel *l1 = gen_new_label();
>> >> +    TCGLabel *l2 = gen_new_label();
>> >> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
>> >> +    TCGv_i32 t1 = tcg_temp_local_new_i32();
>> >> +    TCGv_i32 t2 = tcg_temp_local_new_i32();
>> >> +
>> >> +    tcg_gen_trunc_tl_i32(t0, arg1);
>> >> +    tcg_gen_trunc_tl_i32(t1, arg2);
>> >> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
>> 
>> Result for:
>> <anything> % 0 and ...
>> 
>> >> +    if (sign) {
>> >> +        TCGLabel *l3 = gen_new_label();
>> >> +        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
>> >> +        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
>> >> +        gen_set_label(l3);
>> >
>> > It's not really clear to be what the logic above is doing.
>> 
>> ... For signed case
>> 0x8000_0000 % -1
>> 
>> Is undefined, addressing those cases.
>
> Do you mean the tcg operations have undefined results or that the ppc
> instructions have undefined results?

TCG side, I haven't tried.

> If the latter, then why do you care about those cases?

Thats how divd is implemented as well, i didn't want to break that. I am
looking at doing both div and mod as helpers.

>> >> +        tcg_gen_rem_i32(t2, t0, t1);
>> >> +    } else {
>> >> +        tcg_gen_remu_i32(t2, t0, t1);
>> >> +    }
>> >> +    tcg_gen_br(l2);
>> >> +    gen_set_label(l1);
>> >> +    if (sign) {
>> >> +        tcg_gen_sari_i32(t2, t0, 31);
>> >
>> > AFAICT this sets t2 to either 0 or -1 depending on the sign of t0,
>> > which seems like an odd thing to do.
>> 
>> Extending the sign later ...
>
> Right, so after sign extension you have a 64-bit 0 or -1.  Still not
> seeing what that 0 or -1 result is useful for.

Oh ok, i got why you got confused. I am re-writing all of it though, but
for understanding:

  if (divisor == 0)
     goto l1;

  if (signed) {
     if (divisor == -1 && dividend == INT_MIN)
        goto l1;
     compute_signed_rem(t2, t0, t1);
  } else {
     compute_unsigned_rem(t2, t0, t1);  
  }
  goto l2; /* jump to setting extending result and return */

l1: /* in case of invalid input set values */
  if (signed)
     t2 = -1 or 0;
  else
     t2 = 0;
l2:
  set (ret, t2)

Regards
Nikunj

Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[David Gibson]

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

On Fri, Jul 22, 2016 at 12:24:55PM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > [ Unknown signature status ]
> > On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote:
> >> David Gibson <david@gibson.dropbear.id.au> writes:
> >> 
> >> > [ Unknown signature status ]
> >> > On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote:
> >> >> Adding following instructions:
> >> >> 
> >> >> moduw: Modulo Unsigned Word
> >> >> modsw: Modulo Signed Word
> >> >> 
> >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> >
> >> > As rth has already mentioned this many branches probably means this
> >> > wants a helper.
> >> >
> >> >> ---
> >> >>  target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> >>  1 file changed, 48 insertions(+)
> >> >> 
> >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> >> index d44f7af..487dd94 100644
> >> >> --- a/target-ppc/translate.c
> >> >> +++ b/target-ppc/translate.c
> >> >> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
> >> >>  GEN_DIVE(divdeo, divde, 1);
> >> >>  #endif
> >> >>  
> >> >> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
> >> >> +                                     TCGv arg2, int sign)
> >> >> +{
> >> >> +    TCGLabel *l1 = gen_new_label();
> >> >> +    TCGLabel *l2 = gen_new_label();
> >> >> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
> >> >> +    TCGv_i32 t1 = tcg_temp_local_new_i32();
> >> >> +    TCGv_i32 t2 = tcg_temp_local_new_i32();
> >> >> +
> >> >> +    tcg_gen_trunc_tl_i32(t0, arg1);
> >> >> +    tcg_gen_trunc_tl_i32(t1, arg2);
> >> >> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
> >> 
> >> Result for:
> >> <anything> % 0 and ...
> >> 
> >> >> +    if (sign) {
> >> >> +        TCGLabel *l3 = gen_new_label();
> >> >> +        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
> >> >> +        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
> >> >> +        gen_set_label(l3);
> >> >
> >> > It's not really clear to be what the logic above is doing.
> >> 
> >> ... For signed case
> >> 0x8000_0000 % -1
> >> 
> >> Is undefined, addressing those cases.
> >
> > Do you mean the tcg operations have undefined results or that the ppc
> > instructions have undefined results?
> 
> TCG side, I haven't tried.
> 
> > If the latter, then why do you care about those cases?
> 
> Thats how divd is implemented as well, i didn't want to break that. I am
> looking at doing both div and mod as helpers.
> 
> >> >> +        tcg_gen_rem_i32(t2, t0, t1);
> >> >> +    } else {
> >> >> +        tcg_gen_remu_i32(t2, t0, t1);
> >> >> +    }
> >> >> +    tcg_gen_br(l2);
> >> >> +    gen_set_label(l1);
> >> >> +    if (sign) {
> >> >> +        tcg_gen_sari_i32(t2, t0, 31);
> >> >
> >> > AFAICT this sets t2 to either 0 or -1 depending on the sign of t0,
> >> > which seems like an odd thing to do.
> >> 
> >> Extending the sign later ...
> >
> > Right, so after sign extension you have a 64-bit 0 or -1.  Still not
> > seeing what that 0 or -1 result is useful for.
> 
> Oh ok, i got why you got confused. I am re-writing all of it though, but
> for understanding:
> 
>   if (divisor == 0)
>      goto l1;
> 
>   if (signed) {
>      if (divisor == -1 && dividend == INT_MIN)
>         goto l1;
>      compute_signed_rem(t2, t0, t1);
>   } else {
>      compute_unsigned_rem(t2, t0, t1);  
>   }
>   goto l2; /* jump to setting extending result and return */
> 
> l1: /* in case of invalid input set values */
>   if (signed)
>      t2 = -1 or 0;
>   else
>      t2 = 0;

Ok, so why do you ever need different result values in the case of
invalid input?  Why is always returning 0 not good enough?

> l2:
>   set (ret, t2)
> 
> 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 --]

Re: [Qemu-devel] [RFC v1 05/13] target-ppc: add modulo word operations

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Fri, Jul 22, 2016 at 12:24:55PM +0530, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>> > [ Unknown signature status ]
>> > On Fri, Jul 22, 2016 at 10:59:18AM +0530, Nikunj A Dadhania wrote:
>> >> David Gibson <david@gibson.dropbear.id.au> writes:
>> >> 
>> >> > [ Unknown signature status ]
>> >> > On Mon, Jul 18, 2016 at 10:35:09PM +0530, Nikunj A Dadhania wrote:
>> >> >> Adding following instructions:
>> >> >> 
>> >> >> moduw: Modulo Unsigned Word
>> >> >> modsw: Modulo Signed Word
>> >> >> 
>> >> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> >> >
>> >> > As rth has already mentioned this many branches probably means this
>> >> > wants a helper.
>> >> >
>> >> >> ---
>> >> >>  target-ppc/translate.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>> >> >>  1 file changed, 48 insertions(+)
>> >> >> 
>> >> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> >> >> index d44f7af..487dd94 100644
>> >> >> --- a/target-ppc/translate.c
>> >> >> +++ b/target-ppc/translate.c
>> >> >> @@ -1178,6 +1178,52 @@ GEN_DIVE(divde, divde, 0);
>> >> >>  GEN_DIVE(divdeo, divde, 1);
>> >> >>  #endif
>> >> >>  
>> >> >> +static inline void gen_op_arith_modw(DisasContext *ctx, TCGv ret, TCGv arg1,
>> >> >> +                                     TCGv arg2, int sign)
>> >> >> +{
>> >> >> +    TCGLabel *l1 = gen_new_label();
>> >> >> +    TCGLabel *l2 = gen_new_label();
>> >> >> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
>> >> >> +    TCGv_i32 t1 = tcg_temp_local_new_i32();
>> >> >> +    TCGv_i32 t2 = tcg_temp_local_new_i32();
>> >> >> +
>> >> >> +    tcg_gen_trunc_tl_i32(t0, arg1);
>> >> >> +    tcg_gen_trunc_tl_i32(t1, arg2);
>> >> >> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t1, 0, l1);
>> >> 
>> >> Result for:
>> >> <anything> % 0 and ...
>> >> 
>> >> >> +    if (sign) {
>> >> >> +        TCGLabel *l3 = gen_new_label();
>> >> >> +        tcg_gen_brcondi_i32(TCG_COND_NE, t1, -1, l3);
>> >> >> +        tcg_gen_brcondi_i32(TCG_COND_EQ, t0, INT32_MIN, l1);
>> >> >> +        gen_set_label(l3);
>> >> >
>> >> > It's not really clear to be what the logic above is doing.
>> >> 
>> >> ... For signed case
>> >> 0x8000_0000 % -1
>> >> 
>> >> Is undefined, addressing those cases.
>> >
>> > Do you mean the tcg operations have undefined results or that the ppc
>> > instructions have undefined results?
>> 
>> TCG side, I haven't tried.
>> 
>> > If the latter, then why do you care about those cases?
>> 
>> Thats how divd is implemented as well, i didn't want to break that. I am
>> looking at doing both div and mod as helpers.
>> 
>> >> >> +        tcg_gen_rem_i32(t2, t0, t1);
>> >> >> +    } else {
>> >> >> +        tcg_gen_remu_i32(t2, t0, t1);
>> >> >> +    }
>> >> >> +    tcg_gen_br(l2);
>> >> >> +    gen_set_label(l1);
>> >> >> +    if (sign) {
>> >> >> +        tcg_gen_sari_i32(t2, t0, 31);
>> >> >
>> >> > AFAICT this sets t2 to either 0 or -1 depending on the sign of t0,
>> >> > which seems like an odd thing to do.
>> >> 
>> >> Extending the sign later ...
>> >
>> > Right, so after sign extension you have a 64-bit 0 or -1.  Still not
>> > seeing what that 0 or -1 result is useful for.
>> 
>> Oh ok, i got why you got confused. I am re-writing all of it though, but
>> for understanding:
>> 
>>   if (divisor == 0)
>>      goto l1;
>> 
>>   if (signed) {
>>      if (divisor == -1 && dividend == INT_MIN)
>>         goto l1;
>>      compute_signed_rem(t2, t0, t1);
>>   } else {
>>      compute_unsigned_rem(t2, t0, t1);  
>>   }
>>   goto l2; /* jump to setting extending result and return */
>> 
>> l1: /* in case of invalid input set values */
>>   if (signed)
>>      t2 = -1 or 0;
>>   else
>>      t2 = 0;
>
> Ok, so why do you ever need different result values in the case of
> invalid input?  Why is always returning 0 not good enough?

Let me go through the spec, as divd does the same thing.

Regards
Nikunj

[Qemu-devel] [RFC v1 06/13] target-ppc: add modulo dword operations

[Nikunj A Dadhania]

Adding following instructions for ISA3.0 support

modud: Modulo Unsigned Dword
modsd: Modulo Signed Dword

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

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 487dd94..a561609 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1224,6 +1224,47 @@ static void glue(gen_, name)(DisasContext *ctx)                             \
 GEN_INT_ARITH_MODW(modsw, 0x18, 1);
 GEN_INT_ARITH_MODW(moduw, 0x08, 0);
 
+#if defined(TARGET_PPC64)
+static inline void gen_op_arith_modd(DisasContext *ctx, TCGv ret, TCGv arg1,
+                                     TCGv arg2, int sign)
+{
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *l2 = gen_new_label();
+    TCGv_i64 t0 = tcg_temp_local_new_i64();
+
+    tcg_gen_brcondi_i64(TCG_COND_EQ, arg2, 0, l1);
+    if (sign) {
+        TCGLabel *l3 = gen_new_label();
+        tcg_gen_brcondi_i64(TCG_COND_NE, arg2, -1, l3);
+        tcg_gen_brcondi_i64(TCG_COND_EQ, arg1, INT64_MIN, l1);
+        gen_set_label(l3);
+        tcg_gen_rem_i64(ret, arg1, arg2);
+    } else {
+        tcg_gen_remu_i64(ret, arg1, arg2);
+    }
+    tcg_gen_br(l2);
+    gen_set_label(l1);
+    if (sign) {
+        tcg_gen_sari_i64(ret, arg1, 63);
+    } else {
+        tcg_gen_movi_i64(ret, 0);
+    }
+    gen_set_label(l2);
+    tcg_temp_free_i64(t0);
+}
+
+#define GEN_INT_ARITH_MODD(name, opc3, sign)                            \
+static void glue(gen_, name)(DisasContext *ctx)                           \
+{                                                                         \
+  gen_op_arith_modd(ctx, cpu_gpr[rD(ctx->opcode)],                        \
+                    cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)],   \
+                    sign);                                                \
+}
+
+GEN_INT_ARITH_MODD(modsd, 0x18, 1);
+GEN_INT_ARITH_MODD(modud, 0x08, 0);
+#endif
+
 /* mulhw  mulhw. */
 static void gen_mulhw(DisasContext *ctx)
 {
@@ -10306,6 +10347,8 @@ GEN_HANDLER_E(divdeu, 0x1F, 0x09, 0x0C, 0, PPC_NONE, PPC2_DIVE_ISA206),
 GEN_HANDLER_E(divdeuo, 0x1F, 0x09, 0x1C, 0, PPC_NONE, PPC2_DIVE_ISA206),
 GEN_HANDLER_E(divde, 0x1F, 0x09, 0x0D, 0, PPC_NONE, PPC2_DIVE_ISA206),
 GEN_HANDLER_E(divdeo, 0x1F, 0x09, 0x1D, 0, PPC_NONE, PPC2_DIVE_ISA206),
+GEN_HANDLER_E(modsd, 0x1F, 0x09, 0x18, 0x00000001, PPC_NONE, PPC2_ISA300),
+GEN_HANDLER_E(modud, 0x1F, 0x09, 0x08, 0x00000001, PPC_NONE, PPC2_ISA300),
 
 #undef GEN_INT_ARITH_MUL_HELPER
 #define GEN_INT_ARITH_MUL_HELPER(name, opc3)                                  \
-- 
2.7.4

[Qemu-devel] [RFC v1 07/13] target-ppc: add cnttzd[.] instruction

[Nikunj A Dadhania]

From: Sandipan Das <sandipandas1990@gmail.com>

Add ISA3.0 Count trailing zeros double word

Signed-off-by: Sandipan Das <sandipandas1990@gmail.com>
[ added ISA300 flag ]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/helper.h     | 1 +
 target-ppc/int_helper.c | 5 +++++
 target-ppc/translate.c  | 9 +++++++++
 3 files changed, 15 insertions(+)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 1f5cfd0..0c29c01 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -44,6 +44,7 @@ DEF_HELPER_FLAGS_2(cmpb, TCG_CALL_NO_RWG_SE, tl, tl, tl)
 DEF_HELPER_3(sraw, tl, env, tl, tl)
 #if defined(TARGET_PPC64)
 DEF_HELPER_FLAGS_1(cntlzd, TCG_CALL_NO_RWG_SE, tl, tl)
+DEF_HELPER_FLAGS_1(cnttzd, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(popcntd, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_2(bpermd, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_3(srad, tl, env, tl, tl)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 7445376..93e8dfa 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -150,6 +150,11 @@ target_ulong helper_cntlzd(target_ulong t)
 {
     return clz64(t);
 }
+
+target_ulong helper_cnttzd(target_ulong t)
+{
+    return ctz64(t);
+}
 #endif
 
 #if defined(TARGET_PPC64)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index a561609..5f38080 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1820,6 +1820,14 @@ static void gen_cntlzd(DisasContext *ctx)
     if (unlikely(Rc(ctx->opcode) != 0))
         gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
 }
+
+/* cnttzd */
+static void gen_cnttzd(DisasContext *ctx)
+{
+    gen_helper_cnttzd(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+    if (unlikely(Rc(ctx->opcode) != 0))
+        gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
+}
 #endif
 
 /***                             Integer rotate                            ***/
@@ -10057,6 +10065,7 @@ GEN_HANDLER_E(prtyw, 0x1F, 0x1A, 0x04, 0x0000F801, PPC_NONE, PPC2_ISA205),
 #if defined(TARGET_PPC64)
 GEN_HANDLER(popcntd, 0x1F, 0x1A, 0x0F, 0x0000F801, PPC_POPCNTWD),
 GEN_HANDLER(cntlzd, 0x1F, 0x1A, 0x01, 0x00000000, PPC_64B),
+GEN_HANDLER_E(cnttzd, 0x1F, 0x1A, 0x11, 0x00000000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(prtyd, 0x1F, 0x1A, 0x05, 0x0000F801, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(bpermd, 0x1F, 0x1C, 0x07, 0x00000001, PPC_NONE, PPC2_PERM_ISA206),
 #endif
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 07/13] target-ppc: add cnttzd[.] instruction

[Richard Henderson]

On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
> +    if (unlikely(Rc(ctx->opcode) != 0))
> +        gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);

Braces.


r~

Re: [Qemu-devel] [RFC v1 07/13] target-ppc: add cnttzd[.] instruction

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>> +    if (unlikely(Rc(ctx->opcode) != 0))
>> +        gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
>
> Braces.

Sure.

Regards
Nikunj

[Qemu-devel] [RFC v1 08/13] target-ppc: add cnttzw[.] instruction

[Nikunj A Dadhania]

Add ISA3.0: Count trailing zeros word instruction.

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/helper.h     |  1 +
 target-ppc/int_helper.c |  5 +++++
 target-ppc/translate.c  | 10 ++++++++++
 3 files changed, 16 insertions(+)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 0c29c01..9c79808 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -38,6 +38,7 @@ DEF_HELPER_4(divweu, tl, env, tl, tl, i32)
 DEF_HELPER_4(divwe, tl, env, tl, tl, i32)
 
 DEF_HELPER_FLAGS_1(cntlzw, TCG_CALL_NO_RWG_SE, tl, tl)
+DEF_HELPER_FLAGS_1(cnttzw, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(popcntb, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_1(popcntw, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_FLAGS_2(cmpb, TCG_CALL_NO_RWG_SE, tl, tl, tl)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 93e8dfa..02b6df3 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -145,6 +145,11 @@ target_ulong helper_cntlzw(target_ulong t)
     return clz32(t);
 }
 
+target_ulong helper_cnttzw(target_ulong t)
+{
+    return ctz32(t);
+}
+
 #if defined(TARGET_PPC64)
 target_ulong helper_cntlzd(target_ulong t)
 {
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 5f38080..a57f7dd 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1580,6 +1580,15 @@ static void gen_cntlzw(DisasContext *ctx)
     if (unlikely(Rc(ctx->opcode) != 0))
         gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
 }
+
+/* cnttzw */
+static void gen_cnttzw(DisasContext *ctx)
+{
+    gen_helper_cnttzw(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
+    if (unlikely(Rc(ctx->opcode) != 0))
+        gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
+}
+
 /* eqv & eqv. */
 GEN_LOGICAL2(eqv, tcg_gen_eqv_tl, 0x08, PPC_INTEGER);
 /* extsb & extsb. */
@@ -10053,6 +10062,7 @@ GEN_HANDLER(subfic, 0x08, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(andi_, "andi.", 0x1C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(andis_, "andis.", 0x1D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(cntlzw, 0x1F, 0x1A, 0x00, 0x00000000, PPC_INTEGER),
+GEN_HANDLER_E(cnttzw, 0x1F, 0x1A, 0x10, 0x00000000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(or, 0x1F, 0x1C, 0x0D, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(xor, 0x1F, 0x1C, 0x09, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(ori, 0x18, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 08/13] target-ppc: add cnttzw[.] instruction

[Richard Henderson]

On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
> +    if (unlikely(Rc(ctx->opcode) != 0))
> +        gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);

Braces.


r~

Re: [Qemu-devel] [RFC v1 08/13] target-ppc: add cnttzw[.] instruction

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>> +    if (unlikely(Rc(ctx->opcode) != 0))
>> +        gen_set_Rc0(ctx, cpu_gpr[rA(ctx->opcode)]);
>
> Braces.

Sure.

Nikunj

[Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Nikunj A Dadhania]

From: Swapnil Bokade <bokadeswapnil@gmail.com>

Search a byte in the stream of 8bytes provided in the register

Signed-off-by: Sandipan Das <sandipandas1990@gmail.com>
[ Modified the logic to use lesser temporaries ]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/translate.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index a57f7dd..8f7ff49 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -856,6 +856,32 @@ static void gen_cmprb(DisasContext *ctx)
     tcg_temp_free(src2hi);
 }
 
+/* cmpeqb */
+static void gen_cmpeqb(DisasContext *ctx)
+{
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *l2 = gen_new_label();
+    TCGv src1 = tcg_temp_local_new();
+    TCGv t0 = tcg_temp_local_new();
+    TCGv arg1 = cpu_gpr[rB(ctx->opcode)];
+    int i;
+
+    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
+    for (i = 0; i < 64; i += 8) {
+        tcg_gen_shri_tl(t0, arg1, i);
+        tcg_gen_andi_tl(t0, t0, 0xFF);
+        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
+    }
+    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
+    tcg_gen_br(l2);
+    gen_set_label(l1);
+    /* Set match bit, i.e. CRF_GT */
+    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
+    gen_set_label(l2);
+    tcg_temp_free(src1);
+    tcg_temp_free(t0);
+}
+
 /* isel (PowerPC 2.03 specification) */
 static void gen_isel(DisasContext *ctx)
 {
@@ -10040,6 +10066,7 @@ GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_INTEGER),
 GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
 GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_INTEGER),
 GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
+GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Nikunj A Dadhania]

Nikunj A Dadhania <nikunj@linux.vnet.ibm.com> writes:

> From: Swapnil Bokade <bokadeswapnil@gmail.com>
>
> Search a byte in the stream of 8bytes provided in the register
>
> Signed-off-by: Sandipan Das <sandipandas1990@gmail.com>

Should have been:

Signed-off-by: Swapnil Bokade <bokadeswapnil@gmail.com>

> [ Modified the logic to use lesser temporaries ]
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

Regards
Nikunj

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Richard Henderson]

On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
> +    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
> +    for (i = 0; i < 64; i += 8) {
> +        tcg_gen_shri_tl(t0, arg1, i);
> +        tcg_gen_andi_tl(t0, t0, 0xFF);
> +        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
> +    }
> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
> +    tcg_gen_br(l2);
> +    gen_set_label(l1);
> +    /* Set match bit, i.e. CRF_GT */
> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);

Ew.  This can be done much better as

   http://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord

Which still might be best done in a helper, because of the constants involved 
(tcg is not nearly so good as gcc in building full 64-bit constants).

C.f. target-alpha/int_helper.c, helper_cmpbe0 (which computes different 
information than cmpeqb, but is still helpful as an example).


r~

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>> +    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
>> +    for (i = 0; i < 64; i += 8) {
>> +        tcg_gen_shri_tl(t0, arg1, i);
>> +        tcg_gen_andi_tl(t0, t0, 0xFF);
>> +        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
>> +    }
>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
>> +    tcg_gen_br(l2);
>> +    gen_set_label(l1);
>> +    /* Set match bit, i.e. CRF_GT */
>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
>
> Ew.  This can be done much better as
>
>    http://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
>
> Which still might be best done in a helper, because of the constants involved 
> (tcg is not nearly so good as gcc in building full 64-bit constants).
>
> C.f. target-alpha/int_helper.c, helper_cmpbe0 (which computes different 
> information than cmpeqb, but is still helpful as an example).

Sure, I will have a look.

Nikunj

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>> +    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
>> +    for (i = 0; i < 64; i += 8) {
>> +        tcg_gen_shri_tl(t0, arg1, i);
>> +        tcg_gen_andi_tl(t0, t0, 0xFF);
>> +        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
>> +    }
>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
>> +    tcg_gen_br(l2);
>> +    gen_set_label(l1);
>> +    /* Set match bit, i.e. CRF_GT */
>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
>
> Ew.  This can be done much better as
>
>    http://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
>
> Which still might be best done in a helper, because of the constants involved 
> (tcg is not nearly so good as gcc in building full 64-bit constants).
>
> C.f. target-alpha/int_helper.c, helper_cmpbe0 (which computes different 
> information than cmpeqb, but is still helpful as an example).

Thanks for the hints, that got reduce to following:

#define haszero(v) (((v) - 0x0101010101010101UL) & ~(v) & 0x8080808080808080UL)
#define hasvalue(x,n)  (haszero((x) ^ (~0UL/255 * (n))))

uint32_t helper_cmpeqb(target_ulong ra, target_ulong rb)
{
    return !!hasvalue(rb, ra);
}

Regards
Nikunj

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Richard Henderson]

On 07/23/2016 12:58 AM, Nikunj A Dadhania wrote:
> Richard Henderson <rth@twiddle.net> writes:
>
>> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>>> +    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
>>> +    for (i = 0; i < 64; i += 8) {
>>> +        tcg_gen_shri_tl(t0, arg1, i);
>>> +        tcg_gen_andi_tl(t0, t0, 0xFF);
>>> +        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
>>> +    }
>>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
>>> +    tcg_gen_br(l2);
>>> +    gen_set_label(l1);
>>> +    /* Set match bit, i.e. CRF_GT */
>>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
>>
>> Ew.  This can be done much better as
>>
>>    http://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
>>
>> Which still might be best done in a helper, because of the constants involved
>> (tcg is not nearly so good as gcc in building full 64-bit constants).
>>
>> C.f. target-alpha/int_helper.c, helper_cmpbe0 (which computes different
>> information than cmpeqb, but is still helpful as an example).
>
> Thanks for the hints, that got reduce to following:
>
> #define haszero(v) (((v) - 0x0101010101010101UL) & ~(v) & 0x8080808080808080UL)
> #define hasvalue(x,n)  (haszero((x) ^ (~0UL/255 * (n))))
>
> uint32_t helper_cmpeqb(target_ulong ra, target_ulong rb)
> {
>     return !!hasvalue(rb, ra);
> }

A couple of things:

(1) I don't see N being masked to 0xff before replicating.
(2) You need to use ULL for 32-bit hosts, or casts, e.g.

#define dup(x)          (((x) & 0xff) * (~(target_ulong)0 / 0xff))
#define haszero(v)      (((v) - dup(0x01)) & ~(v) & dup(0x80))
#define hasvalue(x, n)  haszero((x) ^ dup(n))

(3) You probably ought to go ahead and add compute the proper crf value here:

   return hasvalue(rb, ra) ? 1 << CRF_GT : 0;

so that within the translator you just have

   gen_helper_cmpeqb(cpu_crf[...], cpu_gpr[...], cpu_gpr[...])



r~

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Nikunj A Dadhania]

Richard Henderson <rth@twiddle.net> writes:

> On 07/23/2016 12:58 AM, Nikunj A Dadhania wrote:
>> Richard Henderson <rth@twiddle.net> writes:
>>
>>> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>>>> +    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
>>>> +    for (i = 0; i < 64; i += 8) {
>>>> +        tcg_gen_shri_tl(t0, arg1, i);
>>>> +        tcg_gen_andi_tl(t0, t0, 0xFF);
>>>> +        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
>>>> +    }
>>>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
>>>> +    tcg_gen_br(l2);
>>>> +    gen_set_label(l1);
>>>> +    /* Set match bit, i.e. CRF_GT */
>>>> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
>>>
>>> Ew.  This can be done much better as
>>>
>>>    http://graphics.stanford.edu/~seander/bithacks.html#ZeroInWord
>>>
>>> Which still might be best done in a helper, because of the constants involved
>>> (tcg is not nearly so good as gcc in building full 64-bit constants).
>>>
>>> C.f. target-alpha/int_helper.c, helper_cmpbe0 (which computes different
>>> information than cmpeqb, but is still helpful as an example).
>>
>> Thanks for the hints, that got reduce to following:
>>
>> #define haszero(v) (((v) - 0x0101010101010101UL) & ~(v) & 0x8080808080808080UL)
>> #define hasvalue(x,n)  (haszero((x) ^ (~0UL/255 * (n))))
>>
>> uint32_t helper_cmpeqb(target_ulong ra, target_ulong rb)
>> {
>>     return !!hasvalue(rb, ra);
>> }

I had it like this:

+    tcg_gen_ext8u_tl(src1, cpu_gpr[rA(ctx->opcode)]);
+    gen_helper_cmpeqb(crf, src1, cpu_gpr[rB(ctx->opcode)]);
+    tcg_gen_shli_i32(crf, crf, CRF_GT);

> A couple of things:
>
> (1) I don't see N being masked to 0xff before replicating.

Was doing that in caller, but as suggested better to do it in the define.

> (2) You need to use ULL for 32-bit hosts, or casts, e.g.

Having it defined only for 64-bit

> #define dup(x)          (((x) & 0xff) * (~(target_ulong)0 / 0xff))
> #define haszero(v)      (((v) - dup(0x01)) & ~(v) & dup(0x80))
> #define hasvalue(x, n)  haszero((x) ^ dup(n))

Yes, had similar stuff but with different names:

+/* if x = 0xab, returns 0xababababababababa */
+#define pattern(x) (~0UL/255 * (x))
+
+/* substract 1 from each byte, AND with inverse, check if MSB is set at each
+ * byte.
+ * i.e. ((0x00 - 0x01) & ~(0x00)) & 0x80
+ *      (0xFF & 0xFF) & 0x80 = 0x80 (zero found)
+ */
+#define haszero(v) (((v) - pattern(0x01)) & ~(v) & pattern(0x80))
+
+/* When you XOR the pattern and there is a match, that byte will be zero */
+#define hasvalue(x,n)  (haszero((x) ^ pattern(n)))


> (3) You probably ought to go ahead and add compute the proper crf value here:
>
>    return hasvalue(rb, ra) ? 1 << CRF_GT : 0;

Sure

> so that within the translator you just have
>
>    gen_helper_cmpeqb(cpu_crf[...], cpu_gpr[...], cpu_gpr[...])

Right, that will be a one-liner, will change.

Regards
Nikunj

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[Richard Henderson]

On 07/23/2016 11:38 AM, Nikunj A Dadhania wrote:
>> > (2) You need to use ULL for 32-bit hosts, or casts, e.g.
> Having it defined only for 64-bit
>

32-bit *host*, not 32-bit guest.

I.e. i686 host emulating ppc64 guest.


r~

Re: [Qemu-devel] [RFC v1 09/13] target-ppc: add cmpeqb instruction

[David Gibson]

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

On Mon, Jul 18, 2016 at 10:35:13PM +0530, Nikunj A Dadhania wrote:
> From: Swapnil Bokade <bokadeswapnil@gmail.com>
> 
> Search a byte in the stream of 8bytes provided in the register
> 
> Signed-off-by: Sandipan Das <sandipandas1990@gmail.com>
> [ Modified the logic to use lesser temporaries ]
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>

rth's reference may obsolete the suggestions below.

> ---
>  target-ppc/translate.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a57f7dd..8f7ff49 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -856,6 +856,32 @@ static void gen_cmprb(DisasContext *ctx)
>      tcg_temp_free(src2hi);
>  }
>  
> +/* cmpeqb */
> +static void gen_cmpeqb(DisasContext *ctx)
> +{
> +    TCGLabel *l1 = gen_new_label();
> +    TCGLabel *l2 = gen_new_label();
> +    TCGv src1 = tcg_temp_local_new();
> +    TCGv t0 = tcg_temp_local_new();
> +    TCGv arg1 = cpu_gpr[rB(ctx->opcode)];
> +    int i;
> +
> +    tcg_gen_andi_tl(src1, cpu_gpr[rA(ctx->opcode)], 0xFF);
> +    for (i = 0; i < 64; i += 8) {
> +        tcg_gen_shri_tl(t0, arg1, i);
> +        tcg_gen_andi_tl(t0, t0, 0xFF);

Shifting direct from the original arg each time seems awkward when you
can just shift a working reg by 8 bits each loop.  I suspect that
could save you a temporary.

> +        tcg_gen_brcond_tl(TCG_COND_EQ, src1, t0, l1);
> +    }
> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 0);
> +    tcg_gen_br(l2);
> +    gen_set_label(l1);
> +    /* Set match bit, i.e. CRF_GT */
> +    tcg_gen_movi_i32(cpu_crf[crfD(ctx->opcode)], 1 << CRF_GT);
> +    gen_set_label(l2);

You should only need one branch at most, either by initializing the
CRF first, or by moving a variable to it.

> +    tcg_temp_free(src1);
> +    tcg_temp_free(t0);
> +}
> +
>  /* isel (PowerPC 2.03 specification) */
>  static void gen_isel(DisasContext *ctx)
>  {
> @@ -10040,6 +10066,7 @@ GEN_HANDLER(cmp, 0x1F, 0x00, 0x00, 0x00400000, PPC_INTEGER),
>  GEN_HANDLER(cmpi, 0x0B, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
>  GEN_HANDLER(cmpl, 0x1F, 0x00, 0x01, 0x00400000, PPC_INTEGER),
>  GEN_HANDLER(cmpli, 0x0A, 0xFF, 0xFF, 0x00400000, PPC_INTEGER),
> +GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
>  GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),

-- 
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] [RFC v1 10/13] target-ppc: add setb instruction

[Nikunj A Dadhania]

From: Vivek Andrew Sha <vivekandrewsha@gmail.com>

Returns:
  -1 if bit 0 of CR field is set
  0  if bit 1 of CR field is set
  1  otherwise.

Signed-off-by: Vivek Andrew Sha <vivekandrewsha@gmail.com>
[ reworded commit, used 32bit ops as crf is 32bits ]
Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/translate.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 8f7ff49..9464942 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -4879,6 +4879,35 @@ static void gen_mtspr(DisasContext *ctx)
     }
 }
 
+#if defined(TARGET_PPC64)
+/* setb */
+static void gen_setb(DisasContext *ctx)
+{
+    TCGLabel *l1 = gen_new_label();
+    TCGLabel *l2 = gen_new_label();
+    TCGLabel *out = gen_new_label();
+    TCGv_i32 t0 = tcg_temp_local_new_i32();
+    TCGv_i64 ret = tcg_temp_local_new_i64();
+    int crf = crfS(ctx->opcode);
+
+    tcg_gen_andi_i32(t0, cpu_crf[crf], 0x3);
+    tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 0, l1);
+    tcg_gen_andi_i32(t0, cpu_crf[crf], 0x1);
+    tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 1, l2);
+    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 1);
+    tcg_gen_br(out);
+    gen_set_label(l2);
+    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
+    tcg_gen_br(out);
+    gen_set_label(l1);
+    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 0);
+    gen_set_label(out);
+
+    tcg_temp_free_i32(t0);
+    tcg_temp_free_i64(ret);
+}
+#endif
+
 /***                         Cache management                              ***/
 
 /* dcbf */
@@ -10195,6 +10224,7 @@ GEN_HANDLER(mftb, 0x1F, 0x13, 0x0B, 0x00000001, PPC_MFTB),
 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),
 #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] [RFC v1 10/13] target-ppc: add setb instruction

[Richard Henderson]

On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
> +    tcg_gen_andi_i32(t0, cpu_crf[crf], 0x3);
> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 0, l1);
> +    tcg_gen_andi_i32(t0, cpu_crf[crf], 0x1);
> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 1, l2);
> +    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 1);
> +    tcg_gen_br(out);
> +    gen_set_label(l2);
> +    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
> +    tcg_gen_br(out);
> +    gen_set_label(l1);
> +    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 0);
> +    gen_set_label(out);

Without branches:

   tcg_gen_setcondi_i32(TCG_COND_GEU, t0, cpu_crf[crf], 4);
   tcg_gen_movi_i32(t8, 8);
   tcg_gen_movi_i32(tm1, -1);
   tcg_gen_movcond_i32(TCG_COND_GEU, t0, cpu_crf[crf], t8, tm1, t0);
   tcg_gen_ext_i32_tl(cpu_gpr[rD(ctx->opcode)], t0);


r~

Re: [Qemu-devel] [RFC v1 10/13] target-ppc: add setb instruction

[David Gibson]

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

On Mon, Jul 18, 2016 at 10:35:14PM +0530, Nikunj A Dadhania wrote:
> From: Vivek Andrew Sha <vivekandrewsha@gmail.com>
> 
> Returns:
>   -1 if bit 0 of CR field is set
>   0  if bit 1 of CR field is set
>   1  otherwise.

Um.. that description is pretty inadequate.  Retuns where? Which CR
field?

> Signed-off-by: Vivek Andrew Sha <vivekandrewsha@gmail.com>
> [ reworded commit, used 32bit ops as crf is 32bits ]
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target-ppc/translate.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 8f7ff49..9464942 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4879,6 +4879,35 @@ static void gen_mtspr(DisasContext *ctx)
>      }
>  }
>  
> +#if defined(TARGET_PPC64)
> +/* setb */
> +static void gen_setb(DisasContext *ctx)
> +{
> +    TCGLabel *l1 = gen_new_label();
> +    TCGLabel *l2 = gen_new_label();
> +    TCGLabel *out = gen_new_label();
> +    TCGv_i32 t0 = tcg_temp_local_new_i32();
> +    TCGv_i64 ret = tcg_temp_local_new_i64();
> +    int crf = crfS(ctx->opcode);
> +
> +    tcg_gen_andi_i32(t0, cpu_crf[crf], 0x3);
> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 0, l1);
> +    tcg_gen_andi_i32(t0, cpu_crf[crf], 0x1);
> +    tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 1, l2);
> +    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 1);
> +    tcg_gen_br(out);
> +    gen_set_label(l2);
> +    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], -1);
> +    tcg_gen_br(out);
> +    gen_set_label(l1);
> +    tcg_gen_movi_i64(cpu_gpr[rD(ctx->opcode)], 0);
> +    gen_set_label(out);
> +
> +    tcg_temp_free_i32(t0);
> +    tcg_temp_free_i64(ret);
> +}
> +#endif
> +
>  /***                         Cache management                              ***/
>  
>  /* dcbf */
> @@ -10195,6 +10224,7 @@ GEN_HANDLER(mftb, 0x1F, 0x13, 0x0B, 0x00000001, PPC_MFTB),
>  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),
>  #endif
>  GEN_HANDLER(mtmsr, 0x1F, 0x12, 0x04, 0x001EF801, PPC_MISC),
>  GEN_HANDLER(mtspr, 0x1F, 0x13, 0x0E, 0x00000000, PPC_MISC),

-- 
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 --]

Re: [Qemu-devel] [RFC v1 10/13] target-ppc: add setb instruction

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Mon, Jul 18, 2016 at 10:35:14PM +0530, Nikunj A Dadhania wrote:
>> From: Vivek Andrew Sha <vivekandrewsha@gmail.com>
>> 
>> Returns:
>>   -1 if bit 0 of CR field is set
>>   0  if bit 1 of CR field is set
>>   1  otherwise.
>
> Um.. that description is pretty inadequate.  Retuns where? Which CR
> field?

I will update, its encoded in the opcode.

Regards
Nikunj

[Qemu-devel] [RFC v1 11/13] target-ppc: add maddld instruction

[Nikunj A Dadhania]

maddld: Multiply-Add Low Doubleword

Multiplies two 64-bit registers (RA * RB), adds third register(RC) to
the result(quadword) and returns the lower dword in the target
register(RT).

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

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 9464942..9717048 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7760,6 +7760,33 @@ GEN_VAFORM_PAIRED(vmsumshm, vmsumshs, 20)
 GEN_VAFORM_PAIRED(vsel, vperm, 21)
 GEN_VAFORM_PAIRED(vmaddfp, vnmsubfp, 23)
 
+#if defined(TARGET_PPC64)
+static void gen_maddld(DisasContext *ctx)
+{
+    TCGv_i64 lo = tcg_temp_new_i64();
+    TCGv_i64 hi = tcg_temp_new_i64();
+    TCGv_i64 t1 = tcg_temp_new_i64();
+    TCGv_i64 t2 = tcg_temp_new_i64();
+    TCGv_i64 zero = tcg_const_i64(0);
+    TCGv_i64 neg = tcg_const_i64(-1);
+
+    if (Rc(ctx->opcode)) {
+        tcg_gen_muls2_i64(lo, hi, cpu_gpr[rA(ctx->opcode)],
+                          cpu_gpr[rB(ctx->opcode)]);
+        tcg_gen_movi_i64(t2, -1);
+        tcg_gen_movcond_i64(TCG_COND_GE, t2, cpu_gpr[rC(ctx->opcode)], zero, zero, neg);
+    }
+    tcg_gen_mov_i64(t1, zero);
+    tcg_gen_add2_i64(cpu_gpr[rD(ctx->opcode)], t1, lo, hi, cpu_gpr[rC(ctx->opcode)], t2);
+    tcg_temp_free_i64(lo);
+    tcg_temp_free_i64(hi);
+    tcg_temp_free_i64(t1);
+    tcg_temp_free_i64(t2);
+    tcg_temp_free_i64(zero);
+    tcg_temp_free_i64(neg);
+}
+#endif /* defined(TARGET_PPC64) */
+
 GEN_VXFORM_NOA(vclzb, 1, 28)
 GEN_VXFORM_NOA(vclzh, 1, 29)
 GEN_VXFORM_NOA(vclzw, 1, 30)
@@ -10373,6 +10400,9 @@ GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x00000001, PPC_ALTIVEC),
 GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC),
 GEN_HANDLER(mtvscr, 0x04, 0x2, 0x19, 0x03ff0000, PPC_ALTIVEC),
 GEN_HANDLER(vmladduhm, 0x04, 0x11, 0xFF, 0x00000000, PPC_ALTIVEC),
+#if defined(TARGET_PPC64)
+GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
+#endif
 GEN_HANDLER2(evsel0, "evsel", 0x04, 0x1c, 0x09, 0x00000000, PPC_SPE),
 GEN_HANDLER2(evsel1, "evsel", 0x04, 0x1d, 0x09, 0x00000000, PPC_SPE),
 GEN_HANDLER2(evsel2, "evsel", 0x04, 0x1e, 0x09, 0x00000000, PPC_SPE),
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 11/13] target-ppc: add maddld instruction

[Richard Henderson]

On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
> +static void gen_maddld(DisasContext *ctx)
> +{
> +    TCGv_i64 lo = tcg_temp_new_i64();
> +    TCGv_i64 hi = tcg_temp_new_i64();
> +    TCGv_i64 t1 = tcg_temp_new_i64();
> +    TCGv_i64 t2 = tcg_temp_new_i64();
> +    TCGv_i64 zero = tcg_const_i64(0);
> +    TCGv_i64 neg = tcg_const_i64(-1);
> +
> +    if (Rc(ctx->opcode)) {
> +        tcg_gen_muls2_i64(lo, hi, cpu_gpr[rA(ctx->opcode)],
> +                          cpu_gpr[rB(ctx->opcode)]);
> +        tcg_gen_movi_i64(t2, -1);
> +        tcg_gen_movcond_i64(TCG_COND_GE, t2, cpu_gpr[rC(ctx->opcode)], zero, zero, neg);
> +    }
> +    tcg_gen_mov_i64(t1, zero);
> +    tcg_gen_add2_i64(cpu_gpr[rD(ctx->opcode)], t1, lo, hi, cpu_gpr[rC(ctx->opcode)], t2);
> +    tcg_temp_free_i64(lo);
> +    tcg_temp_free_i64(hi);
> +    tcg_temp_free_i64(t1);
> +    tcg_temp_free_i64(t2);
> +    tcg_temp_free_i64(zero);
> +    tcg_temp_free_i64(neg);
> +}

None of this double-word arithmetic is required.
This produces a truncated 64-bit result; the high bits aren't used.

Why the conditional on Rc?  I see no special case for R0.


r~

Re: [Qemu-devel] [RFC v1 11/13] target-ppc: add maddld instruction

[Richard Henderson]

On 07/21/2016 12:24 PM, Richard Henderson wrote:
> On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
>> +static void gen_maddld(DisasContext *ctx)
>> +{
>> +    TCGv_i64 lo = tcg_temp_new_i64();
>> +    TCGv_i64 hi = tcg_temp_new_i64();
>> +    TCGv_i64 t1 = tcg_temp_new_i64();
>> +    TCGv_i64 t2 = tcg_temp_new_i64();
>> +    TCGv_i64 zero = tcg_const_i64(0);
>> +    TCGv_i64 neg = tcg_const_i64(-1);
>> +
>> +    if (Rc(ctx->opcode)) {
>> +        tcg_gen_muls2_i64(lo, hi, cpu_gpr[rA(ctx->opcode)],
>> +                          cpu_gpr[rB(ctx->opcode)]);
>> +        tcg_gen_movi_i64(t2, -1);
>> +        tcg_gen_movcond_i64(TCG_COND_GE, t2, cpu_gpr[rC(ctx->opcode)], zero,
>> zero, neg);
>> +    }
>> +    tcg_gen_mov_i64(t1, zero);
>> +    tcg_gen_add2_i64(cpu_gpr[rD(ctx->opcode)], t1, lo, hi,
>> cpu_gpr[rC(ctx->opcode)], t2);
>> +    tcg_temp_free_i64(lo);
>> +    tcg_temp_free_i64(hi);
>> +    tcg_temp_free_i64(t1);
>> +    tcg_temp_free_i64(t2);
>> +    tcg_temp_free_i64(zero);
>> +    tcg_temp_free_i64(neg);
>> +}
>
> None of this double-word arithmetic is required.
> This produces a truncated 64-bit result; the high bits aren't used.
>
> Why the conditional on Rc?  I see no special case for R0.

Answering my own question, this is the low bit of the opcode, not rC.

Anyway, the conditional is still pointless, because the lsb of the opcode is 
always set, unlike the high-part multiplies.


r~

[Qemu-devel] [RFC v1 12/13] target-ppc: add maddhd and maddhdu instruction

[Nikunj A Dadhania]

maddhd: Multiply-Add High Doubleword
maddhdu: Multiply-Add High Doubleword Unsigned

Above two instruction are dual form and differ by 1 bit
(31st bit)

Multiplies two 64-bit registers (RA * RB), adds third register(RC) to
the result(quadword) and returns the higher dword in the target
register(RT).

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 9717048..6c5a4a6 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7785,6 +7785,36 @@ static void gen_maddld(DisasContext *ctx)
     tcg_temp_free_i64(zero);
     tcg_temp_free_i64(neg);
 }
+
+/* maddhd maddhdu */
+static void gen_maddhd_maddhdu(DisasContext *ctx)
+{
+    TCGv_i64 lo = tcg_temp_new_i64();
+    TCGv_i64 hi = tcg_temp_new_i64();
+    TCGv_i64 t1 = tcg_temp_new_i64();
+    TCGv_i64 t2 = tcg_temp_new_i64();
+    TCGv_i64 zero = tcg_const_i64(0);
+    TCGv_i64 neg = tcg_const_i64(-1);
+
+    if (Rc(ctx->opcode)) {
+        tcg_gen_mulu2_i64(lo, hi, cpu_gpr[rA(ctx->opcode)],
+                          cpu_gpr[rB(ctx->opcode)]);
+        tcg_gen_mov_i64(t2, zero);
+    } else {
+        tcg_gen_muls2_i64(lo, hi, cpu_gpr[rA(ctx->opcode)],
+                          cpu_gpr[rB(ctx->opcode)]);
+        tcg_gen_movi_i64(t2, -1);
+        tcg_gen_movcond_i64(TCG_COND_GE, t2, cpu_gpr[rC(ctx->opcode)], zero, zero, neg);
+    }
+    tcg_gen_mov_i64(t1, zero);
+    tcg_gen_add2_i64(t1, cpu_gpr[rD(ctx->opcode)], lo, hi, cpu_gpr[rC(ctx->opcode)], t2);
+    tcg_temp_free_i64(lo);
+    tcg_temp_free_i64(hi);
+    tcg_temp_free_i64(t1);
+    tcg_temp_free_i64(t2);
+    tcg_temp_free_i64(zero);
+    tcg_temp_free_i64(neg);
+}
 #endif /* defined(TARGET_PPC64) */
 
 GEN_VXFORM_NOA(vclzb, 1, 28)
@@ -10401,6 +10431,8 @@ GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC),
 GEN_HANDLER(mtvscr, 0x04, 0x2, 0x19, 0x03ff0000, PPC_ALTIVEC),
 GEN_HANDLER(vmladduhm, 0x04, 0x11, 0xFF, 0x00000000, PPC_ALTIVEC),
 #if defined(TARGET_PPC64)
+GEN_HANDLER_E(maddhd_maddhdu, 0x04, 0x18, 0xFF, 0x00000000, PPC_NONE,
+              PPC2_ISA300),
 GEN_HANDLER_E(maddld, 0x04, 0x19, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
 #endif
 GEN_HANDLER2(evsel0, "evsel", 0x04, 0x1c, 0x09, 0x00000000, PPC_SPE),
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 12/13] target-ppc: add maddhd and maddhdu instruction

[Richard Henderson]

On 07/18/2016 10:35 PM, Nikunj A Dadhania wrote:
> +        tcg_gen_movi_i64(t2, -1);
> +        tcg_gen_movcond_i64(TCG_COND_GE, t2, cpu_gpr[rC(ctx->opcode)], zero, zero, neg);

This is a simple sign-extension of rC.  Better as

   tcg_gen_sari_i64(t2, cpu_gpr[rC(ctx->opcode)], 63);

> +    tcg_gen_mov_i64(t1, zero);
> +    tcg_gen_add2_i64(t1, cpu_gpr[rD(ctx->opcode)], lo, hi, cpu_gpr[rC(ctx->opcode)], t2);

Pointless store of t1.

Indeed, since we're discarding the value of t1, you might as well re-use t2 in 
the output and drop the t1 variable altogether.  And, thus, rename t2 to 
something else.  ;-)



r~

[Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[Nikunj A Dadhania]

ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
indirect opcode table and corresponding parsing routines.

EO (11:12) Expanded opcode field
Formats: XX1

EO (11:15) Expanded opcode field
Formats: VX, X, XX2

Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
---
 target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
 target-ppc/translate_init.c | 103 ++++++++++++++++++++++++++++++++------------
 2 files changed, 136 insertions(+), 40 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 6c5a4a6..733d68d 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -40,6 +40,7 @@
 /* Include definitions for instructions classes and implementations flags */
 //#define PPC_DEBUG_DISAS
 //#define DO_PPC_STATISTICS
+//#define PPC_DUMP_CPU
 
 #ifdef PPC_DEBUG_DISAS
 #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
@@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
 #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)      \
 GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
 
+#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)     \
+GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
+
 typedef struct opcode_t {
-    unsigned char opc1, opc2, opc3;
+    unsigned char opc1, opc2, opc3, opc4;
 #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
-    unsigned char pad[5];
+    unsigned char pad[4];
 #else
-    unsigned char pad[1];
+    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */
 #endif
     opc_handler_t handler;
     const char *oname;
@@ -452,6 +456,8 @@ EXTRACT_HELPER(opc1, 26, 6);
 EXTRACT_HELPER(opc2, 1, 5);
 /* Opcode part 3 */
 EXTRACT_HELPER(opc3, 6, 5);
+/* Opcode part 4 */
+EXTRACT_HELPER(opc4, 16, 5);
 /* Update Cr0 flags */
 EXTRACT_HELPER(Rc, 0, 1);
 /* Update Cr6 flags (Altivec) */
@@ -589,6 +595,7 @@ EXTRACT_HELPER(SP, 19, 2);
     .opc1 = op1,                                                              \
     .opc2 = op2,                                                              \
     .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
     .pad  = { 0, },                                                           \
     .handler = {                                                              \
         .inval1  = invl,                                                      \
@@ -604,6 +611,7 @@ EXTRACT_HELPER(SP, 19, 2);
     .opc1 = op1,                                                              \
     .opc2 = op2,                                                              \
     .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
     .pad  = { 0, },                                                           \
     .handler = {                                                              \
         .inval1  = invl1,                                                     \
@@ -620,6 +628,7 @@ EXTRACT_HELPER(SP, 19, 2);
     .opc1 = op1,                                                              \
     .opc2 = op2,                                                              \
     .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
     .pad  = { 0, },                                                           \
     .handler = {                                                              \
         .inval1  = invl,                                                      \
@@ -630,12 +639,29 @@ EXTRACT_HELPER(SP, 19, 2);
     },                                                                        \
     .oname = onam,                                                            \
 }
+#define GEN_OPCODE3(name, op1, op2, op3, op4, invl, _typ, _typ2)              \
+{                                                                             \
+    .opc1 = op1,                                                              \
+    .opc2 = op2,                                                              \
+    .opc3 = op3,                                                              \
+    .opc4 = op4,                                                              \
+    .pad  = { 0, },                                                           \
+    .handler = {                                                              \
+        .inval1  = invl,                                                      \
+        .type = _typ,                                                         \
+        .type2 = _typ2,                                                       \
+        .handler = &gen_##name,                                               \
+        .oname = stringify(name),                                             \
+    },                                                                        \
+    .oname = stringify(name),                                                 \
+}
 #else
 #define GEN_OPCODE(name, op1, op2, op3, invl, _typ, _typ2)                    \
 {                                                                             \
     .opc1 = op1,                                                              \
     .opc2 = op2,                                                              \
     .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
     .pad  = { 0, },                                                           \
     .handler = {                                                              \
         .inval1  = invl,                                                      \
@@ -650,6 +676,7 @@ EXTRACT_HELPER(SP, 19, 2);
     .opc1 = op1,                                                              \
     .opc2 = op2,                                                              \
     .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
     .pad  = { 0, },                                                           \
     .handler = {                                                              \
         .inval1  = invl1,                                                     \
@@ -665,6 +692,7 @@ EXTRACT_HELPER(SP, 19, 2);
     .opc1 = op1,                                                              \
     .opc2 = op2,                                                              \
     .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
     .pad  = { 0, },                                                           \
     .handler = {                                                              \
         .inval1  = invl,                                                      \
@@ -674,6 +702,21 @@ EXTRACT_HELPER(SP, 19, 2);
     },                                                                        \
     .oname = onam,                                                            \
 }
+#define GEN_OPCODE3(name, op1, op2, op3, op4, invl, _typ, _typ2)              \
+{                                                                             \
+    .opc1 = op1,                                                              \
+    .opc2 = op2,                                                              \
+    .opc3 = op3,                                                              \
+    .opc4 = op4,                                                              \
+    .pad  = { 0, },                                                           \
+    .handler = {                                                              \
+        .inval1  = invl,                                                      \
+        .type = _typ,                                                         \
+        .type2 = _typ2,                                                       \
+        .handler = &gen_##name,                                               \
+    },                                                                        \
+    .oname = stringify(name),                                                 \
+}
 #endif
 
 /* SPR load/store helpers */
@@ -11946,9 +11989,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
         } else {
             ctx.opcode = cpu_ldl_code(env, ctx.nip);
         }
-        LOG_DISAS("translate opcode %08x (%02x %02x %02x) (%s)\n",
-                    ctx.opcode, opc1(ctx.opcode), opc2(ctx.opcode),
-                    opc3(ctx.opcode), ctx.le_mode ? "little" : "big");
+        LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
+                  ctx.opcode, opc1(ctx.opcode), opc2(ctx.opcode),
+                  opc3(ctx.opcode), opc4(ctx.opcode),
+                  ctx.le_mode ? "little" : "big");
         ctx.nip += 4;
         table = env->opcodes;
         handler = table[opc1(ctx.opcode)];
@@ -11958,14 +12002,19 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
             if (is_indirect_opcode(handler)) {
                 table = ind_table(handler);
                 handler = table[opc3(ctx.opcode)];
+                if (is_indirect_opcode(handler)) {
+                    table = ind_table(handler);
+                    handler = table[opc4(ctx.opcode)];
+                }
             }
         }
         /* Is opcode *REALLY* valid ? */
         if (unlikely(handler->handler == &gen_invalid)) {
             qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported opcode: "
-                          "%02x - %02x - %02x (%08x) " TARGET_FMT_lx " %d\n",
+                          "%02x - %02x - %02x - %02x (%08x) " TARGET_FMT_lx " %d\n",
                           opc1(ctx.opcode), opc2(ctx.opcode),
-                          opc3(ctx.opcode), ctx.opcode, ctx.nip - 4, (int)msr_ir);
+                          opc3(ctx.opcode), opc4(ctx.opcode),
+                          ctx.opcode, ctx.nip - 4, (int)msr_ir);
         } else {
             uint32_t inval;
 
@@ -11977,10 +12026,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
 
             if (unlikely((ctx.opcode & inval) != 0)) {
                 qemu_log_mask(LOG_GUEST_ERROR, "invalid bits: %08x for opcode: "
-                              "%02x - %02x - %02x (%08x) " TARGET_FMT_lx "\n",
+                              "%02x - %02x - %02x - %02x (%08x) " TARGET_FMT_lx "\n",
                               ctx.opcode & inval, opc1(ctx.opcode),
                               opc2(ctx.opcode), opc3(ctx.opcode),
-                              ctx.opcode, ctx.nip - 4);
+                              opc4(ctx.opcode), ctx.opcode, ctx.nip - 4);
                 gen_inval_exception(ctxp, POWERPC_EXCP_INVAL_INVAL);
                 break;
             }
@@ -12006,9 +12055,9 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
             break;
         }
         if (tcg_check_temp_count()) {
-            fprintf(stderr, "Opcode %02x %02x %02x (%08x) leaked temporaries\n",
+            fprintf(stderr, "Opcode %02x %02x %02x %02x (%08x) leaked temporaries\n",
                     opc1(ctx.opcode), opc2(ctx.opcode), opc3(ctx.opcode),
-                    ctx.opcode);
+                    opc4(ctx.opcode), ctx.opcode);
             exit(1);
         }
     }
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d207f68..7c723e9 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9252,13 +9252,45 @@ static int register_dblind_insn (opc_handler_t **ppc_opcodes,
     return 0;
 }
 
+static int register_trplind_insn (opc_handler_t **ppc_opcodes,
+                                  unsigned char idx1, unsigned char idx2,
+                                  unsigned char idx3, unsigned char idx4,
+                                  opc_handler_t *handler)
+{
+    opc_handler_t **table;
+
+    if (register_ind_in_table(ppc_opcodes, idx1, idx2, NULL) < 0) {
+        printf("*** ERROR: unable to join indirect table idx "
+               "[%02x-%02x]\n", idx1, idx2);
+        return -1;
+    }
+    table = ind_table(ppc_opcodes[idx1]);
+    if (register_ind_in_table(table, idx2, idx3, NULL) < 0) {
+        printf("*** ERROR: unable to join 2nd-level indirect table idx "
+               "[%02x-%02x-%02x]\n", idx1, idx2, idx3);
+        return -1;
+    }
+    table = ind_table(table[idx2]);
+    if (register_ind_in_table(table, idx3, idx4, handler) < 0) {
+        printf("*** ERROR: unable to insert opcode "
+               "[%02x-%02x-%02x-%02x]\n", idx1, idx2, idx3, idx4);
+        return -1;
+    }
+    return 0;
+}
 static int register_insn (opc_handler_t **ppc_opcodes, opcode_t *insn)
 {
     if (insn->opc2 != 0xFF) {
         if (insn->opc3 != 0xFF) {
-            if (register_dblind_insn(ppc_opcodes, insn->opc1, insn->opc2,
-                                     insn->opc3, &insn->handler) < 0)
-                return -1;
+            if (insn->opc4 != 0xFF) {
+                if (register_trplind_insn(ppc_opcodes, insn->opc1, insn->opc2,
+                                          insn->opc3, insn->opc4, &insn->handler) < 0)
+                    return -1;
+            } else {
+                if (register_dblind_insn(ppc_opcodes, insn->opc1, insn->opc2,
+                                         insn->opc3, &insn->handler) < 0)
+                    return -1;
+            }
         } else {
             if (register_ind_insn(ppc_opcodes, insn->opc1,
                                   insn->opc2, &insn->handler) < 0)
@@ -9334,7 +9366,7 @@ static void dump_ppc_insns (CPUPPCState *env)
 {
     opc_handler_t **table, *handler;
     const char *p, *q;
-    uint8_t opc1, opc2, opc3;
+    uint8_t opc1, opc2, opc3, opc4;
 
     printf("Instructions set:\n");
     /* opc1 is 6 bits long */
@@ -9354,34 +9386,49 @@ static void dump_ppc_insns (CPUPPCState *env)
                     for (opc3 = 0; opc3 < PPC_CPU_INDIRECT_OPCODES_LEN;
                             opc3++) {
                         handler = table[opc3];
-                        if (handler->handler != &gen_invalid) {
-                            /* Special hack to properly dump SPE insns */
-                            p = strchr(handler->oname, '_');
-                            if (p == NULL) {
-                                printf("INSN: %02x %02x %02x (%02d %04d) : "
-                                       "%s\n",
-                                       opc1, opc2, opc3, opc1,
-                                       (opc3 << 5) | opc2,
-                                       handler->oname);
-                            } else {
-                                q = "speundef";
-                                if ((p - handler->oname) != strlen(q) ||
-                                    memcmp(handler->oname, q, strlen(q)) != 0) {
-                                    /* First instruction */
-                                    printf("INSN: %02x %02x %02x (%02d %04d) : "
-                                           "%.*s\n",
-                                           opc1, opc2 << 1, opc3, opc1,
-                                           (opc3 << 6) | (opc2 << 1),
-                                           (int)(p - handler->oname),
+                        if (is_indirect_opcode(handler)) {
+                            table = ind_table(handler);
+                            /* opc4 is 5 bits long */
+                            for (opc4 = 0; opc4 < PPC_CPU_INDIRECT_OPCODES_LEN;
+                                 opc4++) {
+                                handler = table[opc4];
+                                if (handler->handler != &gen_invalid) {
+                                    printf("INSN: %02x %02x %02x %02x -- (%02d %04d %02d) : %s\n",
+                                           opc1, opc2, opc3, opc4,
+                                           opc1, (opc3 << 5) | opc2, opc4,
                                            handler->oname);
                                 }
-                                if (strcmp(p + 1, q) != 0) {
-                                    /* Second instruction */
+                            }
+                        } else {
+                            if (handler->handler != &gen_invalid) {
+                                /* Special hack to properly dump SPE insns */
+                                p = strchr(handler->oname, '_');
+                                if (p == NULL) {
                                     printf("INSN: %02x %02x %02x (%02d %04d) : "
                                            "%s\n",
-                                           opc1, (opc2 << 1) | 1, opc3, opc1,
-                                           (opc3 << 6) | (opc2 << 1) | 1,
-                                           p + 1);
+                                           opc1, opc2, opc3, opc1,
+                                           (opc3 << 5) | opc2,
+                                           handler->oname);
+                                } else {
+                                    q = "speundef";
+                                    if ((p - handler->oname) != strlen(q) ||
+                                        memcmp(handler->oname, q, strlen(q)) != 0) {
+                                        /* First instruction */
+                                        printf("INSN: %02x %02x %02x (%02d %04d) : "
+                                               "%.*s\n",
+                                               opc1, opc2 << 1, opc3, opc1,
+                                               (opc3 << 6) | (opc2 << 1),
+                                               (int)(p - handler->oname),
+                                               handler->oname);
+                                    }
+                                    if (strcmp(p + 1, q) != 0) {
+                                        /* Second instruction */
+                                        printf("INSN: %02x %02x %02x (%02d %04d) : "
+                                               "%s\n",
+                                               opc1, (opc2 << 1) | 1, opc3, opc1,
+                                               (opc3 << 6) | (opc2 << 1) | 1,
+                                               p + 1);
+                                    }
                                 }
                             }
                         }
-- 
2.7.4

Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[David Gibson]

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

On Mon, Jul 18, 2016 at 10:35:17PM +0530, Nikunj A Dadhania wrote:
> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
> indirect opcode table and corresponding parsing routines.
> 
> EO (11:12) Expanded opcode field
> Formats: XX1
> 
> EO (11:15) Expanded opcode field
> Formats: VX, X, XX2
> 
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
>  target-ppc/translate_init.c | 103 ++++++++++++++++++++++++++++++++------------
>  2 files changed, 136 insertions(+), 40 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 6c5a4a6..733d68d 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -40,6 +40,7 @@
>  /* Include definitions for instructions classes and implementations flags */
>  //#define PPC_DEBUG_DISAS
>  //#define DO_PPC_STATISTICS
> +//#define PPC_DUMP_CPU
>  
>  #ifdef PPC_DEBUG_DISAS
>  #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
> @@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
>  #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)      \
>  GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
>  
> +#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)     \
> +GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
> +
>  typedef struct opcode_t {
> -    unsigned char opc1, opc2, opc3;
> +    unsigned char opc1, opc2, opc3, opc4;
>  #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
> -    unsigned char pad[5];
> +    unsigned char pad[4];
>  #else
> -    unsigned char pad[1];
> +    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */

IIUC the point here is to align entries to the wordsize.  If the
worsize is 32-bit you shouldn't need any extra padding here.

-- 
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 --]

Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Mon, Jul 18, 2016 at 10:35:17PM +0530, Nikunj A Dadhania wrote:
>> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
>> indirect opcode table and corresponding parsing routines.
>> 
>> EO (11:12) Expanded opcode field
>> Formats: XX1
>> 
>> EO (11:15) Expanded opcode field
>> Formats: VX, X, XX2
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>>  target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
>>  target-ppc/translate_init.c | 103 ++++++++++++++++++++++++++++++++------------
>>  2 files changed, 136 insertions(+), 40 deletions(-)
>> 
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index 6c5a4a6..733d68d 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -40,6 +40,7 @@
>>  /* Include definitions for instructions classes and implementations flags */
>>  //#define PPC_DEBUG_DISAS
>>  //#define DO_PPC_STATISTICS
>> +//#define PPC_DUMP_CPU
>>  
>>  #ifdef PPC_DEBUG_DISAS
>>  #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
>> @@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
>>  #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)      \
>>  GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
>>  
>> +#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)     \
>> +GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
>> +
>>  typedef struct opcode_t {
>> -    unsigned char opc1, opc2, opc3;
>> +    unsigned char opc1, opc2, opc3, opc4;
>>  #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
>> -    unsigned char pad[5];
>> +    unsigned char pad[4];
>>  #else
>> -    unsigned char pad[1];
>> +    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */
>
> IIUC the point here is to align entries to the wordsize.  If the
> worsize is 32-bit you shouldn't need any extra padding here.

You are right, the reason I had added this here is to keep the code
clean in the GEN_OPCODEx

#define GEN_OPCODE(name, op1, op2, op3, op4, invl, _typ, _typ2)   \
{                                                                  \
    .opc1 = op1,                                                   \
    .opc2 = op2,                                                   \
    .opc3 = op3,                                                   \
    .opc4 = 0xff,                                                  \
#if HOST_LONG_BITS == 64                                           \
    .pad  = { 0, },                                                \
#endif                                                             \
    .handler = {                                                   \
        .inval1  = invl,                                           \
        .type = _typ,                                              \
        .type2 = _typ2,                                            \
        .handler = &gen_##name,                                    \
        .oname = stringify(name),                                  \
    },                                                             \
    .oname = stringify(name),                                      \
}

I am fine with both the approach, but thought of the current one as
cleaner, we would waste 4byte per opcode in 32-bit case.

Regards
Nikunj

Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[David Gibson]

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

On Fri, Jul 22, 2016 at 11:05:54AM +0530, Nikunj A Dadhania wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > [ Unknown signature status ]
> > On Mon, Jul 18, 2016 at 10:35:17PM +0530, Nikunj A Dadhania wrote:
> >> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
> >> indirect opcode table and corresponding parsing routines.
> >> 
> >> EO (11:12) Expanded opcode field
> >> Formats: XX1
> >> 
> >> EO (11:15) Expanded opcode field
> >> Formats: VX, X, XX2
> >> 
> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> >> ---
> >>  target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
> >>  target-ppc/translate_init.c | 103 ++++++++++++++++++++++++++++++++------------
> >>  2 files changed, 136 insertions(+), 40 deletions(-)
> >> 
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index 6c5a4a6..733d68d 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -40,6 +40,7 @@
> >>  /* Include definitions for instructions classes and implementations flags */
> >>  //#define PPC_DEBUG_DISAS
> >>  //#define DO_PPC_STATISTICS
> >> +//#define PPC_DUMP_CPU
> >>  
> >>  #ifdef PPC_DEBUG_DISAS
> >>  #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
> >> @@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
> >>  #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)      \
> >>  GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
> >>  
> >> +#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)     \
> >> +GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
> >> +
> >>  typedef struct opcode_t {
> >> -    unsigned char opc1, opc2, opc3;
> >> +    unsigned char opc1, opc2, opc3, opc4;
> >>  #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
> >> -    unsigned char pad[5];
> >> +    unsigned char pad[4];
> >>  #else
> >> -    unsigned char pad[1];
> >> +    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */
> >
> > IIUC the point here is to align entries to the wordsize.  If the
> > worsize is 32-bit you shouldn't need any extra padding here.
> 
> You are right, the reason I had added this here is to keep the code
> clean in the GEN_OPCODEx
> 
> #define GEN_OPCODE(name, op1, op2, op3, op4, invl, _typ, _typ2)   \
> {                                                                  \
>     .opc1 = op1,                                                   \
>     .opc2 = op2,                                                   \
>     .opc3 = op3,                                                   \
>     .opc4 = 0xff,                                                  \
> #if HOST_LONG_BITS == 64                                           \
>     .pad  = { 0, },                                                \
> #endif                                                             \

Hrm.. you're using C99 designated initializers, which means I'm pretty
sure you can just leave out the pad field, since you don't care about
it's value.  That should avoid the need for an ifdef.

>     .handler = {                                                   \
>         .inval1  = invl,                                           \
>         .type = _typ,                                              \
>         .type2 = _typ2,                                            \
>         .handler = &gen_##name,                                    \
>         .oname = stringify(name),                                  \
>     },                                                             \
>     .oname = stringify(name),                                      \
> }
> 
> I am fine with both the approach, but thought of the current one as
> cleaner, we would waste 4byte per opcode in 32-bit case.
> 
> 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 --]

Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[Nikunj A Dadhania]

David Gibson <david@gibson.dropbear.id.au> writes:

> [ Unknown signature status ]
> On Fri, Jul 22, 2016 at 11:05:54AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <david@gibson.dropbear.id.au> writes:
>> 
>> > [ Unknown signature status ]
>> > On Mon, Jul 18, 2016 at 10:35:17PM +0530, Nikunj A Dadhania wrote:
>> >> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
>> >> indirect opcode table and corresponding parsing routines.
>> >> 
>> >> EO (11:12) Expanded opcode field
>> >> Formats: XX1
>> >> 
>> >> EO (11:15) Expanded opcode field
>> >> Formats: VX, X, XX2
>> >> 
>> >> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> >> ---
>> >>  target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
>> >>  target-ppc/translate_init.c | 103 ++++++++++++++++++++++++++++++++------------
>> >>  2 files changed, 136 insertions(+), 40 deletions(-)
>> >> 
>> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> >> index 6c5a4a6..733d68d 100644
>> >> --- a/target-ppc/translate.c
>> >> +++ b/target-ppc/translate.c
>> >> @@ -40,6 +40,7 @@
>> >>  /* Include definitions for instructions classes and implementations flags */
>> >>  //#define PPC_DEBUG_DISAS
>> >>  //#define DO_PPC_STATISTICS
>> >> +//#define PPC_DUMP_CPU
>> >>  
>> >>  #ifdef PPC_DEBUG_DISAS
>> >>  #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
>> >> @@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
>> >>  #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)      \
>> >>  GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
>> >>  
>> >> +#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)     \
>> >> +GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
>> >> +
>> >>  typedef struct opcode_t {
>> >> -    unsigned char opc1, opc2, opc3;
>> >> +    unsigned char opc1, opc2, opc3, opc4;
>> >>  #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
>> >> -    unsigned char pad[5];
>> >> +    unsigned char pad[4];
>> >>  #else
>> >> -    unsigned char pad[1];
>> >> +    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */
>> >
>> > IIUC the point here is to align entries to the wordsize.  If the
>> > worsize is 32-bit you shouldn't need any extra padding here.
>> 
>> You are right, the reason I had added this here is to keep the code
>> clean in the GEN_OPCODEx
>> 
>> #define GEN_OPCODE(name, op1, op2, op3, op4, invl, _typ, _typ2)   \
>> {                                                                  \
>>     .opc1 = op1,                                                   \
>>     .opc2 = op2,                                                   \
>>     .opc3 = op3,                                                   \
>>     .opc4 = 0xff,                                                  \
>> #if HOST_LONG_BITS == 64                                           \
>>     .pad  = { 0, },                                                \
>> #endif                                                             \
>
> Hrm.. you're using C99 designated initializers, which means I'm pretty
> sure you can just leave out the pad field, since you don't care about
> it's value.  That should avoid the need for an ifdef.

Sure then, will update accordingly.

Regards,
Nikunj

Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[Bharata B Rao]

On Mon, Jul 18, 2016 at 10:35 PM, Nikunj A Dadhania
<nikunj@linux.vnet.ibm.com> wrote:
> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
> indirect opcode table and corresponding parsing routines.
>
> EO (11:12) Expanded opcode field
> Formats: XX1
>
> EO (11:15) Expanded opcode field
> Formats: VX, X, XX2
>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target-ppc/translate.c      |  73 +++++++++++++++++++++++++------
>  target-ppc/translate_init.c | 103 ++++++++++++++++++++++++++++++++------------
>  2 files changed, 136 insertions(+), 40 deletions(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 6c5a4a6..733d68d 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -40,6 +40,7 @@
>  /* Include definitions for instructions classes and implementations flags */
>  //#define PPC_DEBUG_DISAS
>  //#define DO_PPC_STATISTICS
> +//#define PPC_DUMP_CPU
>
>  #ifdef PPC_DEBUG_DISAS
>  #  define LOG_DISAS(...) qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__)
> @@ -367,12 +368,15 @@ GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
>  #define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)      \
>  GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
>
> +#define GEN_HANDLER_E_2(name, opc1, opc2, opc3, opc4, inval, type, type2)     \
> +GEN_OPCODE3(name, opc1, opc2, opc3, opc4, inval, type, type2)
> +
>  typedef struct opcode_t {
> -    unsigned char opc1, opc2, opc3;
> +    unsigned char opc1, opc2, opc3, opc4;
>  #if HOST_LONG_BITS == 64 /* Explicitly align to 64 bits */
> -    unsigned char pad[5];
> +    unsigned char pad[4];
>  #else
> -    unsigned char pad[1];
> +    unsigned char pad[4]; /* 4-byte pad to maintain pad in opcode table */
>  #endif
>      opc_handler_t handler;
>      const char *oname;
> @@ -452,6 +456,8 @@ EXTRACT_HELPER(opc1, 26, 6);
>  EXTRACT_HELPER(opc2, 1, 5);
>  /* Opcode part 3 */
>  EXTRACT_HELPER(opc3, 6, 5);
> +/* Opcode part 4 */
> +EXTRACT_HELPER(opc4, 16, 5);
>  /* Update Cr0 flags */
>  EXTRACT_HELPER(Rc, 0, 1);
>  /* Update Cr6 flags (Altivec) */
> @@ -589,6 +595,7 @@ EXTRACT_HELPER(SP, 19, 2);
>      .opc1 = op1,                                                              \
>      .opc2 = op2,                                                              \
>      .opc3 = op3,                                                              \
> +    .opc4 = 0xff,                                                             \
>      .pad  = { 0, },                                                           \
>      .handler = {                                                              \
>          .inval1  = invl,                                                      \
> @@ -604,6 +611,7 @@ EXTRACT_HELPER(SP, 19, 2);
>      .opc1 = op1,                                                              \
>      .opc2 = op2,                                                              \
>      .opc3 = op3,                                                              \
> +    .opc4 = 0xff,                                                             \
>      .pad  = { 0, },                                                           \
>      .handler = {                                                              \
>          .inval1  = invl1,                                                     \
> @@ -620,6 +628,7 @@ EXTRACT_HELPER(SP, 19, 2);
>      .opc1 = op1,                                                              \
>      .opc2 = op2,                                                              \
>      .opc3 = op3,                                                              \
> +    .opc4 = 0xff,                                                             \
>      .pad  = { 0, },                                                           \
>      .handler = {                                                              \
>          .inval1  = invl,                                                      \
> @@ -630,12 +639,29 @@ EXTRACT_HELPER(SP, 19, 2);
>      },                                                                        \
>      .oname = onam,                                                            \
>  }
> +#define GEN_OPCODE3(name, op1, op2, op3, op4, invl, _typ, _typ2)              \
> +{                                                                             \
> +    .opc1 = op1,                                                              \
> +    .opc2 = op2,                                                              \
> +    .opc3 = op3,                                                              \
> +    .opc4 = op4,                                                              \
> +    .pad  = { 0, },                                                           \
> +    .handler = {                                                              \
> +        .inval1  = invl,                                                      \
> +        .type = _typ,                                                         \
> +        .type2 = _typ2,                                                       \
> +        .handler = &gen_##name,                                               \
> +        .oname = stringify(name),                                             \
> +    },                                                                        \
> +    .oname = stringify(name),                                                 \
> +}
>  #else
>  #define GEN_OPCODE(name, op1, op2, op3, invl, _typ, _typ2)                    \
>  {                                                                             \
>      .opc1 = op1,                                                              \
>      .opc2 = op2,                                                              \
>      .opc3 = op3,                                                              \
> +    .opc4 = 0xff,                                                             \
>      .pad  = { 0, },                                                           \
>      .handler = {                                                              \
>          .inval1  = invl,                                                      \
> @@ -650,6 +676,7 @@ EXTRACT_HELPER(SP, 19, 2);
>      .opc1 = op1,                                                              \
>      .opc2 = op2,                                                              \
>      .opc3 = op3,                                                              \
> +    .opc4 = 0xff,                                                             \
>      .pad  = { 0, },                                                           \
>      .handler = {                                                              \
>          .inval1  = invl1,                                                     \
> @@ -665,6 +692,7 @@ EXTRACT_HELPER(SP, 19, 2);
>      .opc1 = op1,                                                              \
>      .opc2 = op2,                                                              \
>      .opc3 = op3,                                                              \
> +    .opc4 = 0xff,                                                             \
>      .pad  = { 0, },                                                           \
>      .handler = {                                                              \
>          .inval1  = invl,                                                      \
> @@ -674,6 +702,21 @@ EXTRACT_HELPER(SP, 19, 2);
>      },                                                                        \
>      .oname = onam,                                                            \
>  }
> +#define GEN_OPCODE3(name, op1, op2, op3, op4, invl, _typ, _typ2)              \
> +{                                                                             \
> +    .opc1 = op1,                                                              \
> +    .opc2 = op2,                                                              \
> +    .opc3 = op3,                                                              \
> +    .opc4 = op4,                                                              \
> +    .pad  = { 0, },                                                           \
> +    .handler = {                                                              \
> +        .inval1  = invl,                                                      \
> +        .type = _typ,                                                         \
> +        .type2 = _typ2,                                                       \
> +        .handler = &gen_##name,                                               \
> +    },                                                                        \
> +    .oname = stringify(name),                                                 \
> +}
>  #endif
>
>  /* SPR load/store helpers */
> @@ -11946,9 +11989,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>          } else {
>              ctx.opcode = cpu_ldl_code(env, ctx.nip);
>          }
> -        LOG_DISAS("translate opcode %08x (%02x %02x %02x) (%s)\n",
> -                    ctx.opcode, opc1(ctx.opcode), opc2(ctx.opcode),
> -                    opc3(ctx.opcode), ctx.le_mode ? "little" : "big");
> +        LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
> +                  ctx.opcode, opc1(ctx.opcode), opc2(ctx.opcode),
> +                  opc3(ctx.opcode), opc4(ctx.opcode),
> +                  ctx.le_mode ? "little" : "big");
>          ctx.nip += 4;
>          table = env->opcodes;
>          handler = table[opc1(ctx.opcode)];
> @@ -11958,14 +12002,19 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>              if (is_indirect_opcode(handler)) {
>                  table = ind_table(handler);
>                  handler = table[opc3(ctx.opcode)];
> +                if (is_indirect_opcode(handler)) {
> +                    table = ind_table(handler);
> +                    handler = table[opc4(ctx.opcode)];
> +                }
>              }
>          }
>          /* Is opcode *REALLY* valid ? */
>          if (unlikely(handler->handler == &gen_invalid)) {
>              qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported opcode: "
> -                          "%02x - %02x - %02x (%08x) " TARGET_FMT_lx " %d\n",
> +                          "%02x - %02x - %02x - %02x (%08x) " TARGET_FMT_lx " %d\n",
>                            opc1(ctx.opcode), opc2(ctx.opcode),
> -                          opc3(ctx.opcode), ctx.opcode, ctx.nip - 4, (int)msr_ir);
> +                          opc3(ctx.opcode), opc4(ctx.opcode),
> +                          ctx.opcode, ctx.nip - 4, (int)msr_ir);
>          } else {
>              uint32_t inval;
>
> @@ -11977,10 +12026,10 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>
>              if (unlikely((ctx.opcode & inval) != 0)) {
>                  qemu_log_mask(LOG_GUEST_ERROR, "invalid bits: %08x for opcode: "
> -                              "%02x - %02x - %02x (%08x) " TARGET_FMT_lx "\n",
> +                              "%02x - %02x - %02x - %02x (%08x) " TARGET_FMT_lx "\n",
>                                ctx.opcode & inval, opc1(ctx.opcode),
>                                opc2(ctx.opcode), opc3(ctx.opcode),
> -                              ctx.opcode, ctx.nip - 4);
> +                              opc4(ctx.opcode), ctx.opcode, ctx.nip - 4);
>                  gen_inval_exception(ctxp, POWERPC_EXCP_INVAL_INVAL);
>                  break;
>              }
> @@ -12006,9 +12055,9 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>              break;
>          }
>          if (tcg_check_temp_count()) {
> -            fprintf(stderr, "Opcode %02x %02x %02x (%08x) leaked temporaries\n",
> +            fprintf(stderr, "Opcode %02x %02x %02x %02x (%08x) leaked temporaries\n",
>                      opc1(ctx.opcode), opc2(ctx.opcode), opc3(ctx.opcode),
> -                    ctx.opcode);
> +                    opc4(ctx.opcode), ctx.opcode);
>              exit(1);
>          }
>      }
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index d207f68..7c723e9 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9252,13 +9252,45 @@ static int register_dblind_insn (opc_handler_t **ppc_opcodes,
>      return 0;
>  }
>
> +static int register_trplind_insn (opc_handler_t **ppc_opcodes,
> +                                  unsigned char idx1, unsigned char idx2,
> +                                  unsigned char idx3, unsigned char idx4,
> +                                  opc_handler_t *handler)
> +{
> +    opc_handler_t **table;
> +
> +    if (register_ind_in_table(ppc_opcodes, idx1, idx2, NULL) < 0) {
> +        printf("*** ERROR: unable to join indirect table idx "
> +               "[%02x-%02x]\n", idx1, idx2);
> +        return -1;
> +    }
> +    table = ind_table(ppc_opcodes[idx1]);
> +    if (register_ind_in_table(table, idx2, idx3, NULL) < 0) {
> +        printf("*** ERROR: unable to join 2nd-level indirect table idx "
> +               "[%02x-%02x-%02x]\n", idx1, idx2, idx3);
> +        return -1;
> +    }
> +    table = ind_table(table[idx2]);
> +    if (register_ind_in_table(table, idx3, idx4, handler) < 0) {
> +        printf("*** ERROR: unable to insert opcode "
> +               "[%02x-%02x-%02x-%02x]\n", idx1, idx2, idx3, idx4);
> +        return -1;
> +    }
> +    return 0;
> +}

If you are adding a 3rd level opcode table, explicit freeing of the
same from ppc_cpu_unrealizefn() is necessary right ?

Regards,
Bharata.

Re: [Qemu-devel] [RFC v1 13/13] target-ppc: introduce opc4 for Expanded Opcode

[Nikunj A Dadhania]

Bharata B Rao <bharata.rao@gmail.com> writes:

> On Mon, Jul 18, 2016 at 10:35 PM, Nikunj A Dadhania
> <nikunj@linux.vnet.ibm.com> wrote:
>> ISA 3.0 has introduced EO - Expanded Opcode. Introduce third level
>> indirect opcode table and corresponding parsing routines.
>>
>> EO (11:12) Expanded opcode field
>> Formats: XX1
>>
>> EO (11:15) Expanded opcode field
>> Formats: VX, X, XX2
>>
>> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
>> ---
>> +static int register_trplind_insn (opc_handler_t **ppc_opcodes,
>> +                                  unsigned char idx1, unsigned char idx2,
>> +                                  unsigned char idx3, unsigned char idx4,
>> +                                  opc_handler_t *handler)
>> +{
>> +    opc_handler_t **table;
>> +
>> +    if (register_ind_in_table(ppc_opcodes, idx1, idx2, NULL) < 0) {
>> +        printf("*** ERROR: unable to join indirect table idx "
>> +               "[%02x-%02x]\n", idx1, idx2);
>> +        return -1;
>> +    }
>> +    table = ind_table(ppc_opcodes[idx1]);
>> +    if (register_ind_in_table(table, idx2, idx3, NULL) < 0) {
>> +        printf("*** ERROR: unable to join 2nd-level indirect table idx "
>> +               "[%02x-%02x-%02x]\n", idx1, idx2, idx3);
>> +        return -1;
>> +    }
>> +    table = ind_table(table[idx2]);
>> +    if (register_ind_in_table(table, idx3, idx4, handler) < 0) {
>> +        printf("*** ERROR: unable to insert opcode "
>> +               "[%02x-%02x-%02x-%02x]\n", idx1, idx2, idx3, idx4);
>> +        return -1;
>> +    }
>> +    return 0;
>> +}
>
> If you are adding a 3rd level opcode table, explicit freeing of the
> same from ppc_cpu_unrealizefn() is necessary right ?

Yes, you are right, will add in my next revision.

Regards,
Nikunj