All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions
@ 2020-12-16  9:07 Gustavo Romero
  2020-12-16  9:07 ` [PATCH v2 1/7] target/ppc: Add infrastructure for " Gustavo Romero
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:07 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, mroth, clg, david,
	alex.bennee, rth

This series aims to add support for the new prefixed instructions
introduced with POWER10 CPUs (ISA v3.1).

v2:

- Fixes accordingly to Alex Bennée's review:
  Removed fixes for BUILD_DIR and quiet-command since
  they are already upstream.
  Reverted all the tests to ppc64le-linux-user as they
  are not softmmu tests. To run it:
  
  $ ../configure --target-list=ppc64le-linux-user
  $ make -j $(nproc) check-tcg

- Fixes accordingly to David's review:
  Clarifications for the opcode lookup table and its
  namespaces (normal insn, prefixed type 0/1, and prefixed 2/3).
  Clarification about R macro.
  Fixed missing switch 'break' in parse_prefix_subtype and c1_idx break.
  Fixed duplicated comments.

- Fixed build when target != TARGET_PPC64.


Michael Roth (7):
  target/ppc: Add infrastructure for prefixed instructions
  target/ppc: Add support for prefixed load/store instructions
  tests/tcg: Add tests for prefixed load/store instructions
  target/ppc: Add support for paired vector load/store instructions
  tests/tcg: Add tests for paired vector load/store instructions
  target/ppc: Add support for prefixed load/store FP instructions
  tests/tcg: Add tests for prefixed load/store FP instructions

 target/ppc/cpu.h                              |  30 +-
 target/ppc/helper.h                           |   3 +
 target/ppc/internal.h                         |  27 +
 target/ppc/mem_helper.c                       |  61 ++
 target/ppc/translate.c                        | 442 +++++++-
 target/ppc/translate/fp-impl.c.inc            |  48 +
 target/ppc/translate/fp-ops.c.inc             |   6 +
 target/ppc/translate/vsx-impl.c.inc           |  66 ++
 target/ppc/translate_init.c.inc               |  11 +-
 tests/tcg/ppc64                               |   1 +
 tests/tcg/ppc64le/Makefile.target             |  29 +
 .../test-paired-load-store-vsx.c              | 567 +++++++++++
 .../test-prefixed-load-store-fp.c             | 270 +++++
 .../test-prefixed-load-store.c                | 945 ++++++++++++++++++
 14 files changed, 2499 insertions(+), 7 deletions(-)
 create mode 120000 tests/tcg/ppc64
 create mode 100644 tests/tcg/ppc64le/Makefile.target
 create mode 100644 tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c
 create mode 100644 tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c
 create mode 100644 tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c

-- 
2.17.1



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/7] target/ppc: Add infrastructure for prefixed instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
@ 2020-12-16  9:07 ` Gustavo Romero
  2021-01-05  4:39   ` David Gibson
  2020-12-16  9:07 ` [PATCH v2 2/7] target/ppc: Add support for prefixed load/store instructions Gustavo Romero
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:07 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

Some prefixed instructions (Type 0 and 1, e.g. 8-byte Load/Store or 8LS),
have a completely seperate namespace for their primary opcodes.

Other prefixed instructions (Type 2 and 3, e.g. Modified Load/Store or MLS)
actually re-use existing opcodes to provide a modified variant. We could
handle these by extending the existing opcode handlers to check for the
prefix and handle accordingly, but in some cases it is cleaner to define
seperate handlers for these in their own table entry, and avoids the need
to add error-handling to existing handlers for cases where they are called
for a prefixed Type 2/3 instruction but don't implement the handling for
them. In the future we can re-work things to support both approaches if
cases arise where that seems warranted.

Then we have the normal non-prefixed instructions.

To handle all 3 of these cases we extend the table size 3x, with normal
instructions indexed directly by their primary opcodes, Type 0/1 indexed by
primary opcode + PPC_CPU_PREFIXED_OPCODE_OFFSET, and Type 2/3 indexed by
primary opcode + PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET.

Various exception/SRR handling changes related to prefixed instructions are
yet to be implemented. For instance, no alignment interrupt is generated if
a prefixed instruction crosses the 64-byte boundary; no SRR bit related to
prefixed instructions is set on any interrupt, like for FP unavailable
interrupt, data storage interrupt, etc.

For details, please see PowerISA v3.1, particularly the following sections:

1.6 Instruction Formats, p. 11
1.6.3  Instruction Prefix Formats, p. 22
3.3.2  Fixed-Point Load Instructions, p. 51
4.6.2  Floating-Point Load Instructions, p. 149
Chapter 7 Interrupts, p. 1247

Signed-off-by: Michael Roth <mroth@lamentation.net>
[ gromero: - comments clean up and updates
           - additional comments in the commit log ]
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 target/ppc/cpu.h                |  30 +++++-
 target/ppc/internal.h           |  14 +++
 target/ppc/translate.c          | 170 +++++++++++++++++++++++++++++++-
 target/ppc/translate_init.c.inc |  11 ++-
 4 files changed, 218 insertions(+), 7 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e8aa185d4f..1e40a923bd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -979,8 +979,34 @@ struct ppc_radix_page_info {
 #define PPC_TLB_EPID_LOAD 8
 #define PPC_TLB_EPID_STORE 9
 
-#define PPC_CPU_OPCODES_LEN          0x40
-#define PPC_CPU_INDIRECT_OPCODES_LEN 0x20
+/*
+ * The primary op-code field extracted from the instruction (PO) is used as the
+ * index in the top-level QEMU opcode table. That table is then further used to
+ * find the proper handler, or a pointer to another table (OPC1, OPC2 etc). For
+ * the normal opcodes (PO field of normal insns) the 64 first entries are used.
+ *
+ * Prefixed instructions can be divided into two major groups of instructions:
+ * the first group is formed by type 0 and 1, and the second group is by type 2
+ * and 3. Opcodes (PO) related to prefixed type 0/1 can have the same opcodes
+ * as the normal instructions but don't have any semantics related to them. But
+ * since it's an primary opcode anyway it was decided to simply extend the
+ * top-level table that handles the primary opcodes for the normal instructions.
+ * On the other handle, prefixed instructions type 2/3 work like modifiers for
+ * existing normal instructions. In that case it would be possible to reuse the
+ * handlers for the normal instructions, but that would require changing the
+ * normal instruction handler in a way it would have to check, for instance, on
+ * which ISA it has to take care of prefixes insns, and that is already done by
+ * GEN_HANDLER_* helpers when registering a new instruction, where there is a
+ * a field to inform on which ISA the instruction exists. Hence, it was decided
+ * to add another 64 entries for specific handler for the prefixed instructions
+ * of type 2/3. Thus the size of top-level PO lookup table is 3*64 = 192, hence
+ * representing three namespaces for decoding PO: for normal insns, for type 0/1
+ * and for type 2/3.
+ */
+#define PPC_CPU_OPCODES_LEN                     0xc0 /* 64 + 64 + 64 = 192 */
+#define PPC_CPU_INDIRECT_OPCODES_LEN            0x20
+#define PPC_CPU_PREFIXED_OPCODE_OFFSET          0x40 /* Prefixed type 0/1  */
+#define PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET 0x80 /* Prefixed type 2/3  */
 
 struct CPUPPCState {
     /* Most commonly used resources during translated code execution first */
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 15d655b356..d03d691a44 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -129,6 +129,7 @@ EXTRACT_SHELPER(SIMM5, 16, 5);
 EXTRACT_HELPER(UIMM5, 16, 5);
 /* 4 bits unsigned immediate value */
 EXTRACT_HELPER(UIMM4, 16, 4);
+
 /* Bit count */
 EXTRACT_HELPER(NB, 11, 5);
 /* Shift count */
@@ -146,6 +147,19 @@ EXTRACT_HELPER(TO, 21, 5);
 
 EXTRACT_HELPER(CRM, 12, 8);
 
+/*
+ * Instruction prefix fields
+ *
+ * as per PowerISA 3.1 1.6.3
+ */
+
+/* prefixed instruction type */
+EXTRACT_HELPER(PREFIX_TYPE, 24, 2);
+/* 1-bit sub-type */
+EXTRACT_HELPER(PREFIX_ST1, 23, 1);
+/* 4-bit sub-type */
+EXTRACT_HELPER(PREFIX_ST4, 20, 4);
+
 #ifndef CONFIG_USER_ONLY
 EXTRACT_HELPER(SR, 16, 4);
 #endif
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index fedb9b2271..eab6561094 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -180,6 +180,8 @@ struct DisasContext {
     uint32_t flags;
     uint64_t insns_flags;
     uint64_t insns_flags2;
+    uint32_t prefix;
+    uint32_t prefix_subtype;
 };
 
 /* Return true iff byteswap is needed in a scalar memop */
@@ -376,6 +378,12 @@ GEN_OPCODE(name, opc1, opc2, opc3, inval, type, PPC_NONE)
 #define GEN_HANDLER_E(name, opc1, opc2, opc3, inval, type, type2)             \
 GEN_OPCODE(name, opc1, opc2, opc3, inval, type, type2)
 
+#define GEN_HANDLER_E_PREFIXED(name, opc1, opc2, opc3, inval, type, type2)    \
+GEN_OPCODE_PREFIXED(name, opc1, opc2, opc3, inval, type, type2, false)
+
+#define GEN_HANDLER_E_PREFIXED_M(name, opc1, opc2, opc3, inval, type, type2)  \
+GEN_OPCODE_PREFIXED(name, opc1, opc2, opc3, inval, type, type2, true)
+
 #define GEN_HANDLER2(name, onam, opc1, opc2, opc3, inval, type)               \
 GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
 
@@ -395,6 +403,8 @@ typedef struct opcode_t {
 #endif
     opc_handler_t handler;
     const char *oname;
+    bool prefixed;
+    bool modified;
 } opcode_t;
 
 /* Helpers for priv. check */
@@ -449,6 +459,23 @@ typedef struct opcode_t {
     },                                                                        \
     .oname = stringify(name),                                                 \
 }
+#define GEN_OPCODE_PREFIXED(name, op1, op2, op3, invl, _typ, _typ2, _modified)\
+{                                                                             \
+    .opc1 = op1,                                                              \
+    .opc2 = op2,                                                              \
+    .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
+    .handler = {                                                              \
+        .inval1  = invl,                                                      \
+        .type = _typ,                                                         \
+        .type2 = _typ2,                                                       \
+        .handler = &gen_##name,                                               \
+        .oname = stringify(name),                                             \
+    },                                                                        \
+    .oname = stringify(name),                                                 \
+    .prefixed = true,                                                         \
+    .modified = _modified,                                                    \
+}
 #define GEN_OPCODE_DUAL(name, op1, op2, op3, invl1, invl2, _typ, _typ2)       \
 {                                                                             \
     .opc1 = op1,                                                              \
@@ -525,6 +552,22 @@ typedef struct opcode_t {
     },                                                                        \
     .oname = stringify(name),                                                 \
 }
+#define GEN_OPCODE_PREFIXED(name, op1, op2, op3, invl, _typ, _typ2, _modified)\
+{                                                                             \
+    .opc1 = op1,                                                              \
+    .opc2 = op2,                                                              \
+    .opc3 = op3,                                                              \
+    .opc4 = 0xff,                                                             \
+    .handler = {                                                              \
+        .inval1  = invl,                                                      \
+        .type = _typ,                                                         \
+        .type2 = _typ2,                                                       \
+        .handler = &gen_##name,                                               \
+    },                                                                        \
+    .oname = stringify(name),                                                 \
+    .prefixed = true,                                                         \
+    .modified = _modified,                                                    \
+}
 #define GEN_OPCODE_DUAL(name, op1, op2, op3, invl1, invl2, _typ, _typ2)       \
 {                                                                             \
     .opc1 = op1,                                                              \
@@ -7991,6 +8034,101 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
     return true;
 }
 
+
+
+/*
+ * Prefixed instruction sub-types
+ *
+ * as per PowerISA 3.1 1.6.3
+ */
+enum {
+    /* prefixed instruction but subtype doesn't match any of below ones */
+    PREFIX_ST_INVALID   = -1,
+    /* non-prefixed instruction (normal instruction) */
+    PREFIX_ST_NONE      = 0,
+    /* Type 0b00 prefixed insn sub-types */
+    PREFIX_ST_8LS,      /* 8-byte load/store */
+    PREFIX_ST_8MLS,     /* 8-byte masked load/store */
+    /* Type 0b01 prefixed insn sub-types */
+    PREFIX_ST_8RR,      /* 8-byte reg-to-reg */
+    PREFIX_ST_8MRR,     /* 8-byte masked reg-to-reg */
+    /* Type 0b10 prefixed insn sub-types */
+    PREFIX_ST_MLS,      /* modified load/store */
+    PREFIX_ST_MMLS,     /* modified masked load/store */
+    /* Type 0b11 prefixed insn sub-types */
+    PREFIX_ST_MRR,      /* modified reg-to-reg */
+    PREFIX_ST_MMRR,     /* modified mask reg-to-reg */
+    PREFIX_ST_MMIRR,    /* modified masked immediate reg-to-reg */
+};
+
+static int32_t parse_prefix_subtype(uint32_t prefix)
+{
+    int32_t prefix_subtype = PREFIX_ST_INVALID;
+
+    /* primary opcode 1 is reserved for instruction prefixes */
+    if (opc1(prefix) != 1) {
+        return PREFIX_ST_NONE;
+    }
+
+    switch (PREFIX_TYPE(prefix)) {
+    case 0:
+        if (PREFIX_ST1(prefix) == 0) {
+            prefix_subtype = PREFIX_ST_8LS;
+        } else if (PREFIX_ST1(prefix) == 1) {
+            prefix_subtype = PREFIX_ST_8MLS;
+        }
+        break;
+    case 1:
+        if (PREFIX_ST4(prefix) == 0) {
+            prefix_subtype = PREFIX_ST_8RR;
+        } else if (PREFIX_ST4(prefix) == 8) {
+            prefix_subtype = PREFIX_ST_8MRR;
+        }
+        break;
+    case 2:
+        if (PREFIX_ST1(prefix) == 0) {
+            prefix_subtype = PREFIX_ST_MLS;
+        } else if (PREFIX_ST1(prefix) == 1) {
+            prefix_subtype = PREFIX_ST_MMLS;
+        }
+        break;
+    case 3:
+        if (PREFIX_ST4(prefix) == 0) {
+            prefix_subtype = PREFIX_ST_MRR;
+        } else if (PREFIX_ST4(prefix) == 8) {
+            prefix_subtype = PREFIX_ST_MMRR;
+        } else if (PREFIX_ST4(prefix) == 9) {
+            prefix_subtype = PREFIX_ST_MMIRR;
+        }
+        break;
+    }
+
+    return prefix_subtype;
+}
+
+static uint32_t opc1_idx(DisasContext *ctx)
+{
+    uint32_t table_offset = 0;
+
+    switch (ctx->prefix_subtype) {
+    case PREFIX_ST_8LS:
+    case PREFIX_ST_8MLS:
+    case PREFIX_ST_8RR:
+    case PREFIX_ST_8MRR:
+        table_offset = PPC_CPU_PREFIXED_OPCODE_OFFSET;
+        break;
+    case PREFIX_ST_MLS:
+    case PREFIX_ST_MMLS:
+    case PREFIX_ST_MRR:
+    case PREFIX_ST_MMRR:
+    case PREFIX_ST_MMIRR:
+        table_offset = PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET;
+        break;
+    }
+
+    return table_offset + opc1(ctx->opcode);
+}
+
 static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
@@ -8004,14 +8142,40 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 
     ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
                                       need_byteswap(ctx));
+    /* check for prefix */
+    ctx->prefix_subtype = parse_prefix_subtype(ctx->opcode);
+    if (ctx->prefix_subtype == PREFIX_ST_INVALID) {
+        qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported prefix: "
+                      "%08x " TARGET_FMT_lx "\n",
+                      ctx->prefix, ctx->base.pc_next);
+    } else if (ctx->prefix_subtype != PREFIX_ST_NONE) {
+        /*
+         * this is the 4-byte prefix of an instruction, read the
+         * next 4 and advance the PC
+         *
+         * TODO: we can optimize this to do a single load since we
+         * read in target_long anyways already
+         *
+         * double-check endianess cases.
+         *
+         * engineering note about endianess changing based on rfid
+         * or interrupt. does this need to be accounted for here?
+         */
+        ctx->prefix = ctx->opcode;
+        ctx->base.pc_next += 4;
+        ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
+                                          need_byteswap(ctx));
+    } else {
+        ctx->prefix = 0;
+    }
 
-    LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
+    LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) prefix %08x (%s)\n",
               ctx->opcode, opc1(ctx->opcode), opc2(ctx->opcode),
-              opc3(ctx->opcode), opc4(ctx->opcode),
+              opc3(ctx->opcode), opc4(ctx->opcode), ctx->prefix,
               ctx->le_mode ? "little" : "big");
     ctx->base.pc_next += 4;
     table = cpu->opcodes;
-    handler = table[opc1(ctx->opcode)];
+    handler = table[opc1_idx(ctx)];
     if (is_indirect_opcode(handler)) {
         table = ind_table(handler);
         handler = table[opc2(ctx->opcode)];
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index bb66526280..0ea8c2c5c1 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -9563,8 +9563,13 @@ static int insert_in_table(opc_handler_t **table, unsigned char idx,
 }
 
 static int register_direct_insn(opc_handler_t **ppc_opcodes,
-                                unsigned char idx, opc_handler_t *handler)
+                                unsigned char idx, opc_handler_t *handler,
+                                bool prefixed, bool modified)
 {
+    if (prefixed) {
+        idx = modified ? idx + PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET
+                       : idx + PPC_CPU_PREFIXED_OPCODE_OFFSET;
+    }
     if (insert_in_table(ppc_opcodes, idx, handler) < 0) {
         printf("*** ERROR: opcode %02x already assigned in main "
                "opcode table\n", idx);
@@ -9688,7 +9693,8 @@ static int register_insn(opc_handler_t **ppc_opcodes, opcode_t *insn)
             }
         }
     } else {
-        if (register_direct_insn(ppc_opcodes, insn->opc1, &insn->handler) < 0) {
+        if (register_direct_insn(ppc_opcodes, insn->opc1, &insn->handler,
+                                 insn->prefixed, insn->modified) < 0) {
             return -1;
         }
     }
@@ -9766,6 +9772,7 @@ static void dump_ppc_insns(CPUPPCState *env)
     for (opc1 = 0x00; opc1 < PPC_CPU_OPCODES_LEN; opc1++) {
         table = env->opcodes;
         handler = table[opc1];
+        /* TODO: need to update opcode dump to account for prefixed space */
         if (is_indirect_opcode(handler)) {
             /* opc2 is 5 bits long */
             for (opc2 = 0; opc2 < PPC_CPU_INDIRECT_OPCODES_LEN; opc2++) {
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/7] target/ppc: Add support for prefixed load/store instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
  2020-12-16  9:07 ` [PATCH v2 1/7] target/ppc: Add infrastructure for " Gustavo Romero
@ 2020-12-16  9:07 ` Gustavo Romero
  2020-12-16  9:08 ` [PATCH v2 3/7] tests/tcg: Add tests " Gustavo Romero
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:07 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

This commit adds support for the following prefixed load and store
instructions for GPRs:

  plbz, plh{a,z}, plw{a,z}, pld, plq
  pstb, psth, pstw, pstd, pstq
  paddi

Signed-off-by: Michael Roth <mroth@lamentation.net>
[ gromero: - fix for gen_addr_imm34_index()
           - fix build when target != TARGET_PPC64
           - removed redundant PREFIX_R bit helper
           - changes in commit log ]
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 target/ppc/internal.h  |   6 +
 target/ppc/translate.c | 264 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 270 insertions(+)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index d03d691a44..f67bd30730 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -159,6 +159,12 @@ EXTRACT_HELPER(PREFIX_TYPE, 24, 2);
 EXTRACT_HELPER(PREFIX_ST1, 23, 1);
 /* 4-bit sub-type */
 EXTRACT_HELPER(PREFIX_ST4, 20, 4);
+/* 18 bits signed immediate value */
+EXTRACT_SHELPER(SIMM18, 0, 18);
+/* 18 bits unsigned immediate value */
+EXTRACT_HELPER(UIMM18, 0, 18);
+/* R bit controls if CIA should be added when computing the EA */
+EXTRACT_HELPER(R, 20, 1);
 
 #ifndef CONFIG_USER_ONLY
 EXTRACT_HELPER(SR, 16, 4);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index eab6561094..0bca3a02e4 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2495,6 +2495,37 @@ static inline void gen_addr_add(DisasContext *ctx, TCGv ret, TCGv arg1,
     }
 }
 
+/* returns true if exception was generated */
+static inline int gen_addr_imm34_index(DisasContext *ctx, TCGv EA)
+{
+    uint64_t offset;
+
+    /* ISA says that if R=1 then RA must be 0, otherwise the form is invalid */
+    if (R(ctx->prefix) == 1) {
+        if (unlikely(rA(ctx->opcode) != 0)) {
+            gen_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+            return true;
+        }
+        /*
+         * To find out the address of a prefixed instruction
+         * it's necessary to rewind 8 bytes because they are
+         * twice the size of non-prefixed instructions.
+         */
+        tcg_gen_movi_tl(EA, ctx->base.pc_next - 8);
+    } else {
+        gen_addr_register(ctx, EA);
+    }
+
+    offset = (uint64_t)UIMM18(ctx->prefix) << 16;
+    offset |= UIMM(ctx->opcode);
+    /* sign-extend 34 bit offset to 64-bits... */
+    if (extract64(offset, 33, 1) == 1) {
+        offset |= -1UL << 34;
+    }
+    tcg_gen_addi_tl(EA, EA, offset);
+    return false;
+}
+
 static inline void gen_align_no_le(DisasContext *ctx)
 {
     gen_exception_err(ctx, POWERPC_EXCP_ALIGN,
@@ -2714,6 +2745,35 @@ static void gen_ld(DisasContext *ctx)
     tcg_temp_free(EA);
 }
 
+static void gen_pld(DisasContext *ctx)
+{
+    TCGv EA;
+
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld64_i64(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+/* paddi */
+static void gen_paddi(DisasContext *ctx)
+{
+    TCGv EA;
+
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld64_i64(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+
+out:
+    tcg_temp_free(EA);
+}
+
 /* lq */
 static void gen_lq(DisasContext *ctx)
 {
@@ -2776,6 +2836,195 @@ static void gen_lq(DisasContext *ctx)
     }
     tcg_temp_free(EA);
 }
+
+static void gen_plq(DisasContext *ctx)
+{
+    int ra, rd;
+    TCGv EA, rd0, rd1;
+
+    ra = rA(ctx->opcode);
+    rd = rD(ctx->opcode);
+    if (unlikely((rd & 1) || rd == ra)) {
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+        return;
+    }
+
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+
+    rd0 = cpu_gpr[rd];
+    rd1 = cpu_gpr[rd + 1];
+
+    if (tb_cflags(ctx->base.tb) & CF_PARALLEL) {
+        /* Restart with exclusive lock.  */
+        gen_helper_exit_atomic(cpu_env);
+        ctx->base.is_jmp = DISAS_NORETURN;
+    } else {
+        tcg_gen_qemu_ld_i64(rd0, EA, ctx->mem_idx,
+                            ctx->le_mode ? MO_LEQ : MO_BEQ);
+        gen_addr_add(ctx, EA, EA, 8);
+        tcg_gen_qemu_ld_i64(rd1, EA, ctx->mem_idx,
+                            ctx->le_mode ? MO_LEQ : MO_BEQ);
+    }
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_plbz(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld8u(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_plhz(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld16u(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_plha(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld16s(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_plwz(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld32u(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_plwa(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_ld32s(ctx, cpu_gpr[rD(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_pstb(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_st8(ctx, cpu_gpr[rS(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_psth(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_st16(ctx, cpu_gpr[rS(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_pstw(DisasContext *ctx)
+{
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_st32(ctx, cpu_gpr[rS(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_pstd(DisasContext *ctx) {
+    TCGv EA;
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+    gen_qemu_st64_i64(ctx, cpu_gpr[rS(ctx->opcode)], EA);
+out:
+    tcg_temp_free(EA);
+}
+
+static void gen_pstq(DisasContext *ctx) {
+    int ra, rs;
+    TCGv EA, rs0, rs1;
+
+    ra = rA(ctx->opcode);
+    rs = rD(ctx->opcode);
+    if (unlikely((rs & 1) || rs == ra)) {
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+        return;
+    }
+
+    gen_set_access_type(ctx, ACCESS_INT);
+    EA = tcg_temp_new();
+    if (gen_addr_imm34_index(ctx, EA)) {
+        goto out;
+    }
+
+    rs0 = cpu_gpr[rs];
+    rs1 = cpu_gpr[rs + 1];
+
+    if (tb_cflags(ctx->base.tb) & CF_PARALLEL) {
+        /* Restart with exclusive lock.  */
+        gen_helper_exit_atomic(cpu_env);
+        ctx->base.is_jmp = DISAS_NORETURN;
+    } else {
+        tcg_gen_qemu_st_i64(rs0, EA, ctx->mem_idx,
+                            ctx->le_mode ? MO_LEQ : MO_BEQ);
+        gen_addr_add(ctx, EA, EA, 8);
+        tcg_gen_qemu_st_i64(rs1, EA, ctx->mem_idx,
+                            ctx->le_mode ? MO_LEQ : MO_BEQ);
+    }
+out:
+    tcg_temp_free(EA);
+}
+
+
 #endif
 
 /***                              Integer store                            ***/
@@ -7067,6 +7316,9 @@ 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),
+#if defined(TARGET_PPC64)
+GEN_HANDLER_E_PREFIXED_M(paddi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+#endif
 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),
@@ -7128,6 +7380,18 @@ GEN_HANDLER2_E(extswsli1, "extswsli", 0x1F, 0x1B, 0x1B, 0x00000000,
 GEN_HANDLER(ld, 0x3A, 0xFF, 0xFF, 0x00000000, PPC_64B),
 GEN_HANDLER(lq, 0x38, 0xFF, 0xFF, 0x00000000, PPC_64BX),
 GEN_HANDLER(std, 0x3E, 0xFF, 0xFF, 0x00000000, PPC_64B),
+GEN_HANDLER_E_PREFIXED_M(plbz, 0x22, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED_M(plhz, 0x28, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED_M(plha, 0x2a, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED_M(plwz, 0x20, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED(plwa, 0x29, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED(pld, 0x39, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED(plq, 0x38, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED_M(pstb, 0x26, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED_M(psth, 0x2c, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED_M(pstw, 0x24, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED(pstd, 0x3d, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED(pstq, 0x3c, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),
 #endif
 /* handles lfdp, lxsd, lxssp */
 GEN_HANDLER_E(dform39, 0x39, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA205),
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/7] tests/tcg: Add tests for prefixed load/store instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
  2020-12-16  9:07 ` [PATCH v2 1/7] target/ppc: Add infrastructure for " Gustavo Romero
  2020-12-16  9:07 ` [PATCH v2 2/7] target/ppc: Add support for prefixed load/store instructions Gustavo Romero
@ 2020-12-16  9:08 ` Gustavo Romero
  2020-12-16  9:08 ` [PATCH v2 4/7] target/ppc: Add support for paired vector " Gustavo Romero
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:08 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

This commit adds various tests to exercise the implementation of prefixed
load/store instructions on POWER10.

Signed-off-by: Michael Roth <mroth@lamentation.net>
[ gromero - fix to avoid alignment interrupt, don't cross 64-byte boundary
          - fix displacement for pl{bz,hz,ha,wz,wa,d} to skip branch insn.
          - tweaks in debug output
          - commit log ]
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 tests/tcg/ppc64                               |   1 +
 tests/tcg/ppc64le/Makefile.target             |  16 +
 .../test-prefixed-load-store.c                | 945 ++++++++++++++++++
 3 files changed, 962 insertions(+)
 create mode 120000 tests/tcg/ppc64
 create mode 100644 tests/tcg/ppc64le/Makefile.target
 create mode 100644 tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c

diff --git a/tests/tcg/ppc64 b/tests/tcg/ppc64
new file mode 120000
index 0000000000..e25d62b735
--- /dev/null
+++ b/tests/tcg/ppc64
@@ -0,0 +1 @@
+ppc64le/
\ No newline at end of file
diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
new file mode 100644
index 0000000000..aabc046eb5
--- /dev/null
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -0,0 +1,16 @@
+# -*- Mode: makefile -*-
+#
+# PPC - included from tests/tcg/Makefile
+#
+
+EXTRA_RUNS+=run-test-mmap
+
+VPATH += $(SRC_PATH)/tests/tcg/$(TARGET_NAME)/instruction-tests
+QEMU_OPTS += -cpu power10
+
+PPC_TESTS = test-prefixed-load-store
+
+TESTS += $(PPC_TESTS)
+
+test-prefixed-load-store: test-prefixed-load-store.c
+	gcc $< -o test-prefixed-load-store
diff --git a/tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c b/tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c
new file mode 100644
index 0000000000..f5948ada85
--- /dev/null
+++ b/tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c
@@ -0,0 +1,945 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <endian.h>
+#include <string.h>
+
+bool debug = false;
+
+#define dprintf(...) \
+    do { \
+        if (debug == true) { \
+            fprintf(stderr, "%s: ", __func__); \
+            fprintf(stderr, __VA_ARGS__); \
+        } \
+    } while (0);
+
+bool le;
+
+#define PSTB(_RS, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1 << 26 | 2 << 24 | (" #_R ") << 20 | (" #_d0 ");" \
+    ".long 38 << 26 | (" #_RS ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"
+#define PSTH(_RS, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1 << 26 | 2 << 24 | (" #_R ") << 20 | (" #_d0 ");" \
+    ".long 44 << 26 | (" #_RS ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"
+#define PSTW(_RS, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1 << 26 | 2 << 24 | (" #_R ") << 20 | (" #_d0 ");" \
+    ".long 36 << 26 | (" #_RS ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"
+#define PSTD(_RS, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1 << 26 | (" #_R ") << 20 | (" #_d0 ");" \
+    ".long 61 << 26 | (" #_RS ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"
+
+#define PST_CALL(op, src, dest_ptr, offset_upper18, offset_lower16, r) \
+    do {                                                               \
+        asm(                                                           \
+            op(%1, %0, offset_upper18, offset_lower16, r)              \
+            : "+r" (dest_ptr)                                          \
+            : "r" (src));                                              \
+    } while (0);
+
+void check_pst(uint64_t src, uint64_t dest, uint64_t dest_orig, int width) {
+    uint64_t dest_orig_mask;
+    uint64_t src_mask = (width == 8) ? -1UL : (1UL << (8*width)) - 1;
+
+    if (le) {
+        dest_orig_mask = -1UL << (8*width);
+        assert(dest == ((dest_orig & dest_orig_mask) | ((src & src_mask))));
+    } else {
+        dest_orig_mask = (-1UL << (8*width)) >> (8*width);
+        assert(dest == ((dest_orig & dest_orig_mask) | ((src & src_mask) << (8*(8-width)))));
+    }
+}
+
+void test_pst_offset(int width) {
+    uint64_t dest_orig = 0x2726252423222120;
+    uint64_t src = 0x1716151413111110;
+    uint64_t dest = dest_orig;
+    void *dest_ptr, *dest_ptr_offset;
+
+    dest_ptr = &dest;
+
+    switch (width) {
+    case 1:
+        dest_ptr_offset = dest_ptr - 1;
+        PST_CALL(PSTB, src, dest_ptr_offset, 0, 1, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0xFFFF;
+        PST_CALL(PSTB, src, dest_ptr_offset, 0, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr + 1;
+        PST_CALL(PSTB, src, dest_ptr_offset, 0x3FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0x1FFFFFFFF;
+        PST_CALL(PSTB, src, dest_ptr_offset, 0x1FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        break;
+    case 2:
+        dest_ptr_offset = dest_ptr - 1;
+        PST_CALL(PSTH, src, dest_ptr_offset, 0, 1, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0xFFFF;
+        PST_CALL(PSTH, src, dest_ptr_offset, 0, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr + 1;
+        PST_CALL(PSTH, src, dest_ptr_offset, 0x3FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0x1FFFFFFFF;
+        PST_CALL(PSTH, src, dest_ptr_offset, 0x1FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        break;
+    case 4:
+        dest_ptr_offset = dest_ptr - 1;
+        PST_CALL(PSTW, src, dest_ptr_offset, 0, 1, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0xFFFF;
+        PST_CALL(PSTW, src, dest_ptr_offset, 0, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr + 1;
+        PST_CALL(PSTW, src, dest_ptr_offset, 0x3FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0x1FFFFFFFF;
+        PST_CALL(PSTW, src, dest_ptr_offset, 0x1FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        break;
+    case 8:
+        dest_ptr_offset = dest_ptr - 1;
+        PST_CALL(PSTD, src, dest_ptr_offset, 0, 1, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0xFFFF;
+        PST_CALL(PSTD, src, dest_ptr_offset, 0, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr + 1;
+        PST_CALL(PSTD, src, dest_ptr_offset, 0x3FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        dest_ptr_offset = dest_ptr - 0x1FFFFFFFF;
+        PST_CALL(PSTD, src, dest_ptr_offset, 0x1FFFF, 0xFFFF, 0);
+        check_pst(src, dest, dest_orig, width);
+        break;
+    default:
+        assert(false);
+    }
+}
+
+void test_pst(int width) {
+    uint64_t dest_orig = 0x2726252423222120;
+    uint64_t src = 0x1716151413111110;
+    uint64_t dest, dest_copy;
+    void *dest_ptr = &dest;
+    void *dest_copy_ptr = &dest_copy;
+
+    /* sanity check against non-prefixed ops */
+    dest_copy = dest_orig;
+    switch (width) {
+    case 1:
+        asm(
+            "stb %1, 0(%0)"
+            : "+r" (dest_copy_ptr)
+            : "r" (src));
+        break;
+    case 2:
+        asm(
+            "sth %1, 0(%0)"
+            : "+r" (dest_copy_ptr)
+            : "r" (src));
+        break;
+    case 4:
+        asm(
+            "stw %1, 0(%0)"
+            : "+r" (dest_copy_ptr)
+            : "r" (src));
+        break;
+    case 8:
+        asm(
+            "std %1, 0(%0)"
+            : "+r" (dest_copy_ptr)
+            : "r" (src));
+        break;
+    default:
+        assert(false);
+    }
+
+    dest = dest_orig;
+    switch (width) {
+    case 1:
+        PST_CALL(PSTB, src, dest_ptr, 0, 0, 0);
+        break;
+    case 2:
+        PST_CALL(PSTH, src, dest_ptr, 0, 0, 0);
+        break;
+    case 4:
+        PST_CALL(PSTW, src, dest_ptr, 0, 0, 0);
+        break;
+    case 8:
+        PST_CALL(PSTD, src, dest_ptr, 0, 0, 0);
+        break;
+    default:
+        assert(false);
+    }
+
+    assert(dest == dest_copy);
+    check_pst(src, dest, dest_orig, width);
+}
+
+void test_pstb(void) {
+    test_pst(1);
+    test_pst_offset(1);
+}
+
+void test_psth(void) {
+    test_pst(2);
+    test_pst_offset(2);
+}
+
+void test_pstw(void) {
+    test_pst(4);
+    test_pst_offset(4);
+}
+
+void test_pstd(void) {
+    test_pst(8);
+    test_pst_offset(8);
+}
+
+#define PLBZ(_RT, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 34<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PLHZ(_RT, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 40<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PLHA(_RT, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 42<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PLWZ(_RT, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 32<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PLWA(_RT, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1<<26 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 41<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PLD(_RT, _RA, _d0, _d1, _R) \
+    ".align 6;" \
+    ".long 1<<26 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 57<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+
+#define PL_CALL(op, src, src_ptr, dest, offset_upper18, offset_lower16, r)  \
+    do {                                                                    \
+        asm(                                                                \
+            op(%0, %2, offset_upper18, offset_lower16, r)                   \
+            : "+r" (dest)                                                   \
+            : "r" (src), "r" (src_ptr));                                    \
+    } while (0);
+
+void check_pl_z(uint64_t src, uint64_t dest, int width) {
+    uint64_t src_mask;
+
+    if (le) {
+        src_mask = (width == 8) ? -1UL : (1UL << (8*width)) - 1;
+        assert(dest == (src & src_mask));
+    } else {
+        src_mask = (width == 8) ? -1UL : -1UL << (8*(8-width));
+        assert(dest == (src & src_mask) >> (8*(8-width)));
+    }
+}
+
+void check_pl_a(uint64_t src, uint64_t dest, int width) {
+    uint64_t src_mask, sign_mask;
+
+    /* TODO: docs suggest testing high-order bit of src byte/halfword/etc, but
+     * QEMU seems to use high-order bit of src double in every case?
+     *
+     * but for le, it's based on the former? afa qemu goes???
+     */
+    if (le) {
+        sign_mask = (src & (1UL << (width*8-1))) ? -1UL << (8*width) : 0;
+    } else {
+        sign_mask = (src & (1UL << 63)) ? -1UL << (8*width) : 0;
+    }
+
+    if (le) {
+        src_mask = (width == 8) ? -1UL : (1UL << (8*width)) - 1;
+        assert(dest == ((src & src_mask) | sign_mask));
+    } else {
+        src_mask = (width == 8) ? -1UL : -1UL << (8*(8-width));
+        assert(dest == (((src & src_mask) >> (8*(8-width))) | sign_mask));
+    }
+}
+
+void test_pl_a(int width, uint64_t src, uint64_t dest_orig) {
+    uint64_t dest = 0, dest_copy;
+    void *src_ptr = &src;
+    void *src_ptr_offset;
+
+    /* sanity check against non-prefixed ops */
+    dest_copy = dest_orig;
+
+    switch (width) {
+    case 2:
+        asm(
+            "lha %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    case 4:
+        asm(
+            "lwa %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    case 8:
+        asm(
+            "ld %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    default:
+        assert(false);
+    }
+
+    switch (width) {
+    case 2:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLHA, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLHA, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLHA, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLHA, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLHA, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        break;
+    case 4:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLWA, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLWA, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLWA, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLWA, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLWA, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        break;
+    case 8:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_a(src, dest, width);
+        break;
+    default:
+        assert(false);
+    }
+
+    assert(dest == dest_copy);
+}
+
+void test_pl_z(int width, uint64_t src, uint64_t dest_orig) {
+    uint64_t dest = 0, dest_copy;
+    void *src_ptr = &src;
+    void *src_ptr_offset;
+
+    /* sanity check against non-prefixed ops */
+    dest_copy = dest_orig;
+
+    switch (width) {
+    case 1:
+        asm(
+            "lbz %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    case 2:
+        asm(
+            "lhz %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    case 4:
+        asm(
+            "lwz %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    case 8:
+        asm(
+            "ld %0, 0(%2)"
+            : "+r" (dest_copy)
+            : "r" (src), "r" (src_ptr));
+        break;
+    default:
+        assert(false);
+    }
+
+    dest = dest_orig;
+    switch (width) {
+    case 1:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLBZ, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLBZ, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLBZ, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLBZ, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLBZ, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        break;
+    case 2:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLHZ, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLHZ, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLHZ, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLHZ, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLHZ, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        break;
+    case 4:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLWZ, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLWZ, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLWZ, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLWZ, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLWZ, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        break;
+    case 8:
+        dest = dest_orig;
+        src_ptr_offset = src_ptr;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0, 0, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 1;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0, 1, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0xFFFF;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr + 1;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0x3FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        dest = dest_orig;
+        src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+        PL_CALL(PLD, src, src_ptr_offset, dest, 0x1FFFF, 0xFFFF, 0);
+        check_pl_z(src, dest, width);
+        break;
+    default:
+        assert(false);
+    }
+
+    assert(dest == dest_copy);
+}
+
+void test_plbz(void) {
+    test_pl_z(1, 0x8716151413111110, 0x0726252423222120);
+    test_pl_z(1, 0x1716151413111110, 0x0726252423222120);
+    test_pl_z(1, 0x1716151413111180, 0x0726252423222120);
+}
+
+void test_plhz(void) {
+    test_pl_z(2, 0x8716151483111110, 0x0726252423222120);
+    test_pl_z(1, 0x1716151413111110, 0x0726252423222120);
+    test_pl_z(1, 0x1716151413118110, 0x0726252423222120);
+}
+
+void test_plha(void) {
+    test_pl_a(2, 0x8716151483111110, 0x0726252423222120);
+    test_pl_a(2, 0x1716151413111110, 0x0726252423222120);
+    test_pl_a(2, 0x1716151413118110, 0x0726252423222120);
+}
+
+void test_plwz(void) {
+    test_pl_z(4, 0x8716151483111110, 0x0726252423222120);
+    test_pl_z(4, 0x1716151413111110, 0x0726252423222120);
+    test_pl_z(4, 0x1716151483111110, 0x0726252423222120);
+}
+
+void test_plwa(void) {
+    test_pl_a(4, 0x8716151483111110, 0x0726252423222120);
+    test_pl_a(4, 0x1716151413111110, 0x0726252423222120);
+    test_pl_a(4, 0x1716151483111110, 0x0726252423222120);
+}
+
+void test_pld(void) {
+    test_pl_a(8, 0x8716151483111110, 0x0726252423222120);
+    test_pl_a(8, 0x1716151413111110, 0x0726252423222120);
+}
+
+#define QUADWORD_HI 0x0f0e0d0c0b0a0908
+#define QUADWORD_LO 0x0706050403020100
+
+#define PSTQ(_RS, _RA, _d0, _d1, _R) \
+    ".long 1<<26 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 60<<26 | (" #_RS ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+
+void test_pstq(void) {
+    register uint64_t rs0 asm("r22");
+    register uint64_t rs1 asm("r23");
+    uint64_t storage[2] = { 0 };
+    void *src_ptr = storage;
+
+    if (le) {
+        /*
+         * MEM(EA, 16) <- RSp+1||RSp
+         * where RQ[15..0] = RSp+1||RSp = rs1[7..0] || rs0[7..0]
+         */
+        rs0 = QUADWORD_LO;
+        rs1 = QUADWORD_HI;
+    } else {
+        /*
+         * MEM(EA, 16) <- RSp||RSp+1
+         * where RQ[0..15] = RSp||RSp+1 = rs0[0..7] || rs1[0..7]
+         */
+        rs0 = QUADWORD_HI;
+        rs1 = QUADWORD_LO;
+    }
+
+    asm(
+        PSTQ(22, %0, 0, 0, 0)
+        : "+r" (src_ptr)
+        : "r" (rs0), "r" (rs1));
+
+    if (le) {
+        assert(storage[0] == QUADWORD_LO);
+        assert(storage[1] == QUADWORD_HI);
+    } else {
+        assert(storage[0] == QUADWORD_HI);
+        assert(storage[1] == QUADWORD_LO);
+    }
+
+    /* sanity check against stq */
+    asm(
+        "stq 22, 0(%0)"
+        : "+r" (src_ptr)
+        : "r" (rs0), "r" (rs1));
+
+    if (le) {
+        assert(storage[0] == QUADWORD_HI);
+        assert(storage[1] == QUADWORD_LO);
+    } else {
+        assert(storage[0] == QUADWORD_HI);
+        assert(storage[1] == QUADWORD_LO);
+    }
+}
+
+#define PLQ(_RT, _RA, _d0, _d1, _R) \
+    ".long 1<<26 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 56<<26 | (" #_RT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+
+void test_plq(void) {
+    register uint64_t rdest0 asm("r20") = 7;
+    register uint64_t rdest1 asm("r21") = 8;
+    uint64_t dest0a = 7;
+    uint64_t dest0b = 7;
+    uint64_t dest1a = 7;
+    uint64_t dest1b = 7;
+    uint8_t src[16];
+    void *src_ptr = &src;
+    int i;
+
+    for (i = 0; i < 16; i++) {
+        src[i] = i;
+    }
+
+    /*
+     * PLQ:
+     *
+     * loads to RTp+1||RTp for little-endian
+     *          RTp||RTp+1 for big-endian
+     *
+     * so we'd expect:
+     *
+     * value: 0x0f0e..08 || 0706..00
+     *
+     * little-endian:
+     *
+     * uint64_t storage[2] = { 0x0706050403020100,
+     *                         0x0f0e0d0c0b0a0908 };
+     * plq 20,0(storage):
+     *   r21[0..7]         || r20[0..7]
+     *   0x0001020304050607   0x08090a0b0c0d0e0f
+     *
+     * big-endian:
+     *
+     * uint64_t storage[2] = { 0x0f0e0d0c0b0a0908,
+     *                         0x0706050403020100 };
+     *
+     * plq 20,0(storage):
+     *   r20[0..7]         || r21[0..7]
+     *   0x0f0e0d0c0b0a0908   0x0706050403020100
+     *
+     * Note: According to spec, for GPRs at least, GPR byte ordering is always
+     * big-endian with regard to loads/stores. Hence the need to "reverse load"
+     * in the case of loading little-endian value into a register, as opposed to
+     * simply assuming both the storage and the register would both use
+     * host-endian.
+     *
+     * But, this is just as far as the documentation goes, which is always
+     * left-to-right/big-endian byte ordering. The actual hardware register
+     * stores byte 0 in a little-endian to value to byte 0 in the register, so
+     * registers are loaded host-endian even though the documentation sort of
+     * suggests otherwise in some cases.
+     */
+    asm(
+        PLQ(20, %2, 0, 0, 0)
+        : "=r" (rdest0), "=r" (rdest1)
+        : "r" (src_ptr));
+
+    dest0a = rdest0;
+    dest1a = rdest1;
+
+    /* loads to dest0||dest1 for both endians */
+    asm(
+        "lq 20, 0(%2)"
+        : "=r" (rdest0), "=r" (rdest1)
+        : "r" (src_ptr));
+
+    dest0b = rdest0;
+    dest1b = rdest1;
+
+    if (le) {
+        assert(dest0a == ((uint64_t*)src)[0]);
+        assert(dest1a == ((uint64_t*)src)[1]);
+        assert(dest0a == dest1b);
+        assert(dest1a == dest0b);
+    } else {
+        assert(dest0a == ((uint64_t*)src)[0]);
+        assert(dest1a == ((uint64_t*)src)[1]);
+        assert(dest0a == dest0b);
+        assert(dest1a == dest1b);
+    }
+
+    /* TODO: PC-relative and negative offsets just like all the others */
+}
+
+void test_plq2(void) {
+    register uint64_t rdest0 asm("r20") = 7;
+    register uint64_t rdest1 asm("r21") = 8;
+    register uint64_t rdest0b asm("r22") = 7;
+    register uint64_t rdest1b asm("r23") = 8;
+    uint64_t storage[2];
+    void *src_ptr = storage;
+
+    if (le) {
+        storage[0] = QUADWORD_LO;
+        storage[1] = QUADWORD_HI;
+    } else {
+        storage[0] = QUADWORD_HI;
+        storage[1] = QUADWORD_LO;
+    }
+
+    /*
+     * PLQ:
+     *
+     * loads to RTp+1||RTp for little-endian
+     *          RTp||RTp+1 for big-endian
+     *
+     * loads into register using host-endian encoding
+     * calls it "reverse-order" for little-endian, but
+     * the byte-ordering is switched based on endianess
+     * so we still copy mem[0] to reg[0], etc., in all
+     * cases. i.e. storage endian encoding is maintained
+     * in the register encoding after load, even though
+     * documentation might still call it reverse and
+     * reference left-to-right byte ordering in some
+     * cases even for little-endian
+     *
+     * so we'd expect:
+     *
+     * value: 0x0f0e..08 || 0706..00
+     *
+     * little-endian:
+     *
+     * uint64_t storage[2] = { 0x0706050403020100,
+     *                         0x0f0e0d0c0b0a0908 };
+     * plq 20,0(storage):
+     *   RTquad[15..0] = r21[7..0] || r20[7..0]
+     *   r21[7..0]         || r20[7..0]
+     *   0x0f0e0d0c0b0a0908   0x0706050403020100
+     *
+     * big-endian:
+     *
+     * uint64_t storage[2] = { 0x0f0e0d0c0b0a0908,
+     *                         0x0706050403020100 };
+     *
+     * plq 20,0(storage):
+     *   RTquad[0..15] = r20[0..7] || r21[0..7]
+     *   r20[0..7]         || r21[0..7]
+     *   0x0f0e0d0c0b0a0908   0x0706050403020100
+     **/
+    asm(
+        PLQ(20, %2, 0, 0, 0)
+        : "=r" (rdest0), "=r" (rdest1)
+        : "r" (src_ptr));
+
+    if (le) {
+        assert(rdest0 == QUADWORD_LO);
+        assert(rdest1 == QUADWORD_HI);
+    } else {
+        assert(rdest0 == QUADWORD_HI);
+        assert(rdest1 == QUADWORD_LO);
+    }
+
+    /* sanity check against lq */
+    asm(
+        "lq 22, 0(%2)"
+        : "=r" (rdest0b), "=r" (rdest1b)
+        : "r" (src_ptr));
+
+    if (le) {
+        assert(rdest0 == rdest1b);
+        assert(rdest1 == rdest0b);
+    } else {
+        assert(rdest0 == rdest0b);
+        assert(rdest1 == rdest1b);
+    }
+}
+
+void test_plbz_cia(void) {
+    uint64_t dest = 0;
+
+    asm(
+        PLBZ(%0, 0, 0, 8 /* skip plbz */ + 4 /* skip b */, 1)
+        "b 1f\n"
+        ".byte 0x1a\n"
+        ".byte 0x1b\n"
+        ".byte 0x1c\n"
+        ".byte 0x1d\n"
+        "1: nop\n"
+        : "+r" (dest));
+
+    assert(dest == 0x1a);
+}
+
+void test_plhz_cia(void) {
+    uint64_t dest = 0;
+
+    asm(
+        PLHZ(%0, 0, 0, 8 /* skip plhz */ + 4 /* skip b */, 1)
+        "b 1f\n"
+        ".byte 0x1a\n"
+        ".byte 0x1b\n"
+        ".byte 0x1c\n"
+        ".byte 0x1d\n"
+        "1: nop\n"
+        : "+r" (dest));
+
+    if (le) {
+        assert(dest == 0x1b1a);
+    } else {
+        assert(dest == 0x1a1b);
+    }
+}
+
+void test_plha_cia(void) {
+    uint64_t dest = 0;
+
+    asm(
+        PLHA(%0, 0, 0, 8 /* skip plha */ + 4 /* skip b */, 1)
+        "b 1f\n"
+        ".byte 0x8a\n"
+        ".byte 0x8b\n"
+        ".byte 0x1c\n"
+        ".byte 0x1d\n"
+        ".byte 0x2a\n"
+        ".byte 0x2b\n"
+        ".byte 0x2c\n"
+        ".byte 0x2d\n"
+        "1: nop\n"
+        : "+r" (dest));
+
+    if (le) {
+        assert(dest == 0xFFFFFFFFFFFF8b8a);
+    } else {
+        assert(dest == 0xFFFFFFFFFFFF8a8b);
+    }
+}
+
+void test_plwz_cia(void) {
+    uint64_t dest = 0;
+
+    asm(
+        PLWZ(%0, 0, 0, 8 /* skip plwz */ + 4 /* skip b */, 1)
+        "b 1f\n"
+        ".byte 0x1a\n"
+        ".byte 0x1b\n"
+        ".byte 0x1c\n"
+        ".byte 0x1d\n"
+        "1: nop\n"
+        : "+r" (dest));
+
+    if (le) {
+        assert(dest == 0x1d1c1b1a);
+    } else {
+        assert(dest == 0x1a1b1c1d);
+    }
+}
+
+void test_plwa_cia(void) {
+    uint64_t dest = 0;
+
+    asm(
+        PLWA(%0, 0, 0, 8 /* skip plwa */ + 4 /* skip b */, 1)
+        "b 1f\n"
+        ".byte 0x8a\n"
+        ".byte 0x1b\n"
+        ".byte 0x1c\n"
+        ".byte 0x8d\n"
+        ".byte 0x2a\n"
+        ".byte 0x2b\n"
+        ".byte 0x2c\n"
+        ".byte 0x2d\n"
+        "1: nop\n"
+        : "+r" (dest));
+
+    if (le) {
+        assert(dest == 0xFFFFFFFF8d1c1b8a);
+    } else {
+        assert(dest == 0xFFFFFFFF8a1b1c8d);
+    }
+}
+
+void test_pld_cia(void) {
+    uint64_t dest = 0;
+
+    asm(
+        PLD(%0, 0, 0, 8 /* skip pld */ + 4 /* skip b */, 1)
+        "b 1f\n"
+        ".byte 0x1a\n"
+        ".byte 0x1b\n"
+        ".byte 0x1c\n"
+        ".byte 0x1d\n"
+        ".byte 0x2a\n"
+        ".byte 0x2b\n"
+        ".byte 0x2c\n"
+        ".byte 0x2d\n"
+        "1: nop\n"
+        : "+r" (dest));
+
+    if (le) {
+        assert(dest == 0x2d2c2b2a1d1c1b1a);
+    } else {
+        assert(dest == 0x1a1b1c1d2a2b2c2d);
+    }
+}
+
+#define do_test(testname) \
+    if (debug) \
+        fprintf(stderr, "-> running test: " #testname "\n"); \
+    test_##testname(); \
+
+int main(int argc, char **argv)
+{
+    le = (htole16(1) == 1);
+
+    if (argc > 1 && !strcmp(argv[1], "-d")) {
+        debug = true;
+    }
+
+    do_test(pstb);
+    do_test(psth);
+    do_test(pstw);
+    do_test(pstd);
+    do_test(plbz);
+    do_test(plhz);
+    do_test(plha);
+    do_test(psth);
+    do_test(pld);
+
+    do_test(pstq);
+    do_test(plq);
+    do_test(plq2);
+
+    do_test(plbz_cia);
+    do_test(plhz_cia);
+    do_test(plha_cia);
+    do_test(plwz_cia);
+    do_test(plwa_cia);
+    do_test(pld_cia);
+
+    dprintf("All tests passed\n");
+    return 0;
+}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/7] target/ppc: Add support for paired vector load/store instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
                   ` (2 preceding siblings ...)
  2020-12-16  9:08 ` [PATCH v2 3/7] tests/tcg: Add tests " Gustavo Romero
@ 2020-12-16  9:08 ` Gustavo Romero
  2020-12-16  9:08 ` [PATCH v2 5/7] tests/tcg: Add tests " Gustavo Romero
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:08 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

This adds support for the following load/store instructions for paired
vector registers:

  lxvp, plxvp, lxvpx
  stxvp, pstxvp, stxvpx

Signed-off-by: Michael Roth <mroth@lamentation.net>
[ gromero: - fix in helper_load_paired_vec(), for LE
           - fix in helper_store_paired_vec(), for LE
           - fix build when target != PPC64 ]
Sgined-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 target/ppc/helper.h                 |  3 ++
 target/ppc/internal.h               |  7 +++
 target/ppc/mem_helper.c             | 61 ++++++++++++++++++++++++++
 target/ppc/translate.c              |  8 ++++
 target/ppc/translate/vsx-impl.c.inc | 66 +++++++++++++++++++++++++++++
 5 files changed, 145 insertions(+)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6a4dccf70c..e8ecd2e878 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -279,6 +279,9 @@ DEF_HELPER_4(lxvl, void, env, tl, vsr, tl)
 DEF_HELPER_4(lxvll, void, env, tl, vsr, tl)
 DEF_HELPER_4(stxvl, void, env, tl, vsr, tl)
 DEF_HELPER_4(stxvll, void, env, tl, vsr, tl)
+/* lxvp/stxvp, plxvp/pstxvp, lxvpx/stxvpx */
+DEF_HELPER_4(store_paired_vec, void, env, tl, vsr, vsr)
+DEF_HELPER_4(load_paired_vec, void, env, tl, vsr, vsr)
 #endif
 DEF_HELPER_4(vsumsws, void, env, avr, avr, avr)
 DEF_HELPER_4(vsum2sws, void, env, avr, avr, avr)
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index f67bd30730..27a5311e3a 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -80,6 +80,12 @@ static inline int16_t name(uint32_t opcode)                                   \
         (((opcode >> (shift_op_d2)) & ((1 << (d2_bits)) - 1)) << (shift_d2));  \
 }
 
+#define EXTRACT_HELPER_SPLIT_SHIFTED(name, shift1, nb1, shift2, nb2, shift3)  \
+static inline uint32_t name(uint32_t opcode)                                  \
+{                                                                             \
+    return extract32(opcode, shift1, nb1) << (nb2 + shift3) |                 \
+               extract32(opcode, shift2, nb2) << shift3;                      \
+}
 
 /* Opcode part 1 */
 EXTRACT_HELPER(opc1, 26, 6);
@@ -226,6 +232,7 @@ EXTRACT_HELPER(SP, 19, 2);
 EXTRACT_HELPER(IMM8, 11, 8);
 EXTRACT_HELPER(DCMX, 16, 7);
 EXTRACT_HELPER_SPLIT_3(DCMX_XV, 5, 16, 0, 1, 2, 5, 1, 6, 6);
+EXTRACT_HELPER_SPLIT_SHIFTED(xTp, 21, 1, 22, 4, 1);
 
 void helper_compute_fprf_float16(CPUPPCState *env, float16 arg);
 void helper_compute_fprf_float32(CPUPPCState *env, float32 arg);
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 98f589552b..8d35a19c68 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -532,6 +532,67 @@ STVE(stvewx, cpu_stl_data_ra, bswap32, u32)
 #undef I
 #undef LVE
 
+#ifdef TARGET_PPC64
+void helper_load_paired_vec(CPUPPCState *env, target_ulong addr,
+                            ppc_vsr_t *xt0, ppc_vsr_t *xt1)
+{
+    ppc_vsr_t t0, t1;
+    int i;
+
+    t0.s128 = int128_zero();
+    t1.s128 = int128_zero();
+
+    if (msr_le) {
+        for (i = 0; i < 16; i++) {
+            t1.VsrB(15 - i) = cpu_ldub_data_ra(env, addr, GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+        for (i = 0; i < 16; i++) {
+            t0.VsrB(15 - i) = cpu_ldub_data_ra(env, addr, GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+    } else { // TODO: check if it's correct for BE.
+        for (i = 0; i < 16; i++) {
+            t0.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+        for (i = 0; i < 16; i++) {
+            t1.VsrB(i) = cpu_ldub_data_ra(env, addr, GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+    }
+
+    *xt0 = t0;
+    *xt1 = t1;
+}
+
+void helper_store_paired_vec(CPUPPCState *env, target_ulong addr,
+                             ppc_vsr_t *xt0, ppc_vsr_t *xt1)
+{
+    int i;
+
+    if (msr_le) {
+        for (i = 0; i < 16; i++) {
+            cpu_stb_data_ra(env, addr, xt1->VsrB(15 - i), GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+        for (i = 0; i < 16; i++) {
+            cpu_stb_data_ra(env, addr, xt0->VsrB(15 - i), GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+    } else { // TODO: check if it's correct for BE.
+        for (i = 0; i < 16; i++) {
+            cpu_stb_data_ra(env, addr, xt0->VsrB(i), GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+        for (i = 0; i < 16; i++) {
+            cpu_stb_data_ra(env, addr, xt1->VsrB(i), GETPC());
+            addr = addr_add(env, addr, 1);
+        }
+    }
+}
+#endif /* TARGET_PPC64 */
+
 #ifdef TARGET_PPC64
 #define GET_NB(rb) ((rb >> 56) & 0xFF)
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0bca3a02e4..25a3c1198b 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7397,6 +7397,14 @@ GEN_HANDLER_E_PREFIXED(pstq, 0x3c, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310)
 GEN_HANDLER_E(dform39, 0x39, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA205),
 /* handles stfdp, lxv, stxsd, stxssp, stxv */
 GEN_HANDLER_E(dform3D, 0x3D, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA205),
+#if defined(TARGET_PPC64)
+GEN_HANDLER_E_PREFIXED(plxvp, 0x3A, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA310),
+GEN_HANDLER_E_PREFIXED(pstxvp, 0x3E, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA310),
+/* handles lxvp, stxvp */
+GEN_HANDLER_E(dqform6, 0x6, 0x0, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA310),
+GEN_HANDLER_E(lxvpx, 0x1F, 0xD, 0xA, 0x00000000, PPC_NONE, PPC2_ISA310),
+GEN_HANDLER_E(stxvpx, 0x1F, 0xD, 0xE, 0x00000000, PPC_NONE, PPC2_ISA310),
+#endif
 GEN_HANDLER(lmw, 0x2E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(stmw, 0x2F, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(lswi, 0x1F, 0x15, 0x12, 0x00000001, PPC_STRING),
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index b518de46db..6640b7ae05 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -376,6 +376,72 @@ VSX_VECTOR_LOAD_STORE_LENGTH(stxvl)
 VSX_VECTOR_LOAD_STORE_LENGTH(stxvll)
 #endif
 
+#ifdef TARGET_PPC64
+
+static void gen_dqform6(DisasContext *ctx)
+{
+    TCGv EA;
+    TCGv_ptr xt0, xt1;
+
+    EA = tcg_temp_new();
+    xt0 = gen_vsr_ptr(xTp(ctx->opcode));
+    xt1 = gen_vsr_ptr(xTp(ctx->opcode) + 1);
+    gen_set_access_type(ctx, ACCESS_INT);
+    gen_addr_imm_index(ctx, EA, 0x0F);
+
+    if (extract32(ctx->opcode, 0, 4) == 1) {
+        /* stxvp */
+        gen_helper_store_paired_vec(cpu_env, EA, xt0, xt1);
+    } else {
+        /* lxvp */
+        gen_helper_load_paired_vec(cpu_env, EA, xt0, xt1);
+    }
+
+    tcg_temp_free(EA);
+    tcg_temp_free_ptr(xt0);
+    tcg_temp_free_ptr(xt1);
+}
+
+#define VSX_VECTOR_LOAD_STORE_PAIRED(name, dform, op)              \
+static void gen_##name(DisasContext *ctx)                          \
+{                                                                  \
+    TCGv EA;                                                       \
+    TCGv_ptr xt0, xt1;                                             \
+                                                                   \
+    if (unlikely(!ctx->altivec_enabled)) {                         \
+        gen_exception(ctx, POWERPC_EXCP_VPU);                      \
+        return;                                                    \
+    }                                                              \
+                                                                   \
+    EA = tcg_temp_new();                                           \
+    xt0 = gen_vsr_ptr(xTp(ctx->opcode));                           \
+    xt1 = gen_vsr_ptr(xTp(ctx->opcode) + 1);                       \
+    gen_set_access_type(ctx, ACCESS_INT);                          \
+                                                                   \
+     if (dform) {                                                  \
+        /* pstxvp, plxvp, 8LS, D-form */                           \
+        if (gen_addr_imm34_index(ctx, EA)) {                       \
+            goto out;                                              \
+        }                                                          \
+    } else {                                                       \
+        /* lxvpx, stxvpx, X-form */                                \
+        gen_addr_reg_index(ctx, EA);                               \
+    }                                                              \
+                                                                   \
+    gen_helper_##op##_paired_vec(cpu_env, EA, xt0, xt1);           \
+out:                                                               \
+    tcg_temp_free(EA);                                             \
+    tcg_temp_free_ptr(xt0);                                        \
+    tcg_temp_free_ptr(xt1);                                        \
+}
+
+VSX_VECTOR_LOAD_STORE_PAIRED(plxvp, 1, load)
+VSX_VECTOR_LOAD_STORE_PAIRED(pstxvp, 1, store)
+VSX_VECTOR_LOAD_STORE_PAIRED(lxvpx, 0, load)
+VSX_VECTOR_LOAD_STORE_PAIRED(stxvpx, 0, store)
+
+#endif
+
 #define VSX_LOAD_SCALAR_DS(name, operation)                       \
 static void gen_##name(DisasContext *ctx)                         \
 {                                                                 \
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 5/7] tests/tcg: Add tests for paired vector load/store instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
                   ` (3 preceding siblings ...)
  2020-12-16  9:08 ` [PATCH v2 4/7] target/ppc: Add support for paired vector " Gustavo Romero
@ 2020-12-16  9:08 ` Gustavo Romero
  2020-12-16  9:08 ` [PATCH v2 6/7] target/ppc: Add support for prefixed load/store FP instructions Gustavo Romero
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:08 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

Signed-off-by: Michael Roth <mroth@lamentation.net>
[ gromero: - aligned prefixed insns., avoiding cross 64-byte boundary
           - fix to skip branch insn. just after PLXVP
           - fix various buffer offsets used in comparisons, for LE only
           - tweaks in debug output ]
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 tests/tcg/ppc64le/Makefile.target             |   9 +
 .../test-paired-load-store-vsx.c              | 567 ++++++++++++++++++
 2 files changed, 576 insertions(+)
 create mode 100644 tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c

diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
index aabc046eb5..aaa43a83a7 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -11,6 +11,15 @@ QEMU_OPTS += -cpu power10
 PPC_TESTS = test-prefixed-load-store
 
 TESTS += $(PPC_TESTS)
+PPC_VSX_TESTS = test-paired-load-store-vsx
+
+VSX_CFLAGS = -maltivec -mvsx
+$(PPC_VSX_TESTS): CFLAGS+=$(VSX_CFLAGS)
+
+PPC_TESTS += $(PPC_VSX_TESTS)
 
 test-prefixed-load-store: test-prefixed-load-store.c
 	gcc $< -o test-prefixed-load-store
+
+test-paired-load-store-vsx: test-paired-load-store-vsx.c
+	gcc $< -o test-paired-load-store-vsx
diff --git a/tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c b/tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c
new file mode 100644
index 0000000000..61c25d9f41
--- /dev/null
+++ b/tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c
@@ -0,0 +1,567 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <altivec.h>
+#include <endian.h>
+#include <string.h>
+
+bool debug = false;
+
+#define dprintf(...) \
+    do { \
+        if (debug == true) { \
+            fprintf(stderr, "%s: ", __func__); \
+            fprintf(stderr, __VA_ARGS__); \
+        } \
+    } while (0);
+
+bool le;
+
+#define PLXVP(_Tp, _RA, _d0, _d1, _R, _TX) \
+    ".align 6;" \
+    ".long 1 << 26 | (" #_R ") << 20 | (" #_d0 ");" \
+    ".long 58 << 26 | (" #_Tp ") << 22 | (" #_TX ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"
+
+void test_plxvp_cia(void) {
+    register vector unsigned char v0 asm("vs8") = { 0 };
+    register vector unsigned char v1 asm("vs9") = { 0 };
+    int i;
+
+    /* load defined bytes below into vs8,vs9 using CIA with relative offset */
+    asm(
+        PLXVP(4, 0, 0, 8 /* skip plxvp */ + 4 /* skip b */, 1, 0)
+        "b 1f;"
+        ".byte 0;"
+        ".byte 1;"
+        ".byte 2;"
+        ".byte 3;"
+        ".byte 4;"
+        ".byte 5;"
+        ".byte 6;"
+        ".byte 7;"
+        ".byte 8;"
+        ".byte 9;"
+        ".byte 10;"
+        ".byte 11;"
+        ".byte 12;"
+        ".byte 13;"
+        ".byte 14;"
+        ".byte 15;"
+        ".byte 16;"
+        ".byte 17;"
+        ".byte 18;"
+        ".byte 19;"
+        ".byte 20;"
+        ".byte 21;"
+        ".byte 22;"
+        ".byte 23;"
+        ".byte 24;"
+        ".byte 25;"
+        ".byte 26;"
+        ".byte 27;"
+        ".byte 28;"
+        ".byte 29;"
+        ".byte 30;"
+        ".byte 31;"
+        "1: nop;"
+        : "+wa" (v0), "+wa" (v1));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == 16 + i);
+        else
+            assert(v0[i] == (31 - i)); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == i);
+        else
+            assert(v1[i] == 15 - i); // FIXME
+    }
+}
+
+void test_plxvp(void) {
+    register vector unsigned char v0 asm("vs6") = { 0 };
+    register vector unsigned char v1 asm("vs7") = { 0 };
+    uint8_t buf[64] __attribute__((aligned(16)));
+    uint8_t *buf_ptr = (uint8_t *)&buf;
+    int i;
+
+    for (i = 0; i < 64; i++) {
+        buf[i] = i;
+    }
+
+    /* load buf[0:31] into vs6,vs7 using EA with no offset */
+    asm(PLXVP(3, %2, 0, 0, 0, 0)
+        : "+wa" (v0), "+wa" (v1)
+        : "r" (buf_ptr));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[16 + i]);
+        else
+            assert(v0[i] == buf[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[i]);
+        else
+            assert(v1[i] == buf[16 + i]); // FIXME
+    }
+
+    /* load buf[32:63] into vs6,vs7 using EA with d1 offset */
+    buf_ptr = buf_ptr + 32 - 0x1000;
+    asm(PLXVP(3, %2, 0, 0x1000, 0, 0)
+        : "+wa" (v0), "+wa" (v1)
+        : "r" (buf_ptr));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[32 + 16 + i]);
+        else
+            assert(v0[i] == buf[32 + i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[32 + i]);
+        else
+            assert(v1[i] == buf[48 + i]); // FIXME
+    }
+
+    /* load buf[0:31] into vs6,vs7 using EA with d0||d1 offset */
+    buf_ptr = buf;
+    buf_ptr = buf_ptr - ((0x1000 << 16) | 0x1000);
+    asm(PLXVP(3, %2, 0x1000, 0x1000, 0, 0)
+        : "+wa" (v0), "+wa" (v1)
+        : "r" (buf_ptr));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[16 + i]);
+        else
+            assert(v0[i] == buf[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[i]);
+        else
+            assert(v1[i] == buf[16 + i]); // FIXME
+    }
+
+    /* TODO: test signed offset */
+    /* TODO: PC-relative addresses */
+    /* load buf[32:63] into vs6,vs7 using EA with negative d0||d1 offset */
+    buf_ptr = buf;
+    buf_ptr = buf_ptr + 32 + 0x1000;
+    asm(PLXVP(3, %2, 0x3ffff, 0xf000, 0, 0)
+        : "+wa" (v0), "+wa" (v1)
+        : "r" (buf_ptr));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[32 + 16 + i]);
+        else
+            assert(v0[i] == buf[32 + i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[32 + i]);
+        else
+            assert(v1[i] == buf[48 + i]); // FIXME
+    }
+}
+
+#define PSTXVP(_Sp, _RA, _d0, _d1, _R, _SX) \
+    ".align 6;" \
+    ".long 1 << 26 | (" #_R ") << 20 | (" #_d0 ");" \
+    ".long 62 << 26 | (" #_Sp ") << 22 | (" #_SX ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"
+
+void test_pstxvp(void) {
+    register vector unsigned char v0 asm("vs6") = {
+// FIXME: reorder values for readability
+        0, 1, 2, 3,
+        4, 5, 6, 7,
+        8, 9, 10, 11,
+        12, 13, 14, 15
+    };
+    register vector unsigned char v1 asm("vs7") = {
+// FIXME: reorder values for readability
+        16, 17, 18, 19,
+        20, 21, 22, 23,
+        24, 25, 26, 27,
+        28, 29, 30, 31
+    };
+    uint8_t buf[64] __attribute__((aligned(16)));
+    uint8_t *buf_ptr = (uint8_t *)&buf;
+    int i;
+
+    for (i = 0; i < 64; i++) {
+        buf[i] = 0;
+    }
+
+    /* store vs6,vs7 into buf[0:31] using EA with no offset */
+    asm(PSTXVP(3, %0, 0, 0, 0, 0)
+        : "+r" (buf_ptr)
+        : "wa" (v0), "wa" (v1));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[16 + i]);
+        else
+            assert(v0[i] == buf[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[i]);
+        else
+            assert(v1[i] == buf[16 + i]); // FIXME
+    }
+
+    /* store vs6,vs7 into buf[32:63] using EA with d1 offset */
+    buf_ptr = buf_ptr + 32 - 0x1000;
+    asm(PSTXVP(3, %0, 0, 0x1000, 0, 0)
+        : "+r" (buf_ptr)
+        : "wa" (v0), "wa" (v1));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[32 + 16 + i]);
+        else
+            assert(v0[i] == buf[32 + i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[32 + i]);
+        else
+            assert(v1[i] == buf[48 + i]); // FIXME
+    }
+
+    /* store buf[0:31] into vs6,vs7 using EA with d0||d1 offset */
+    buf_ptr = buf;
+    buf_ptr = buf_ptr - ((0x1000 << 16) | 0x1000);
+    asm(PSTXVP(3, %0, 0x1000, 0x1000, 0, 0)
+        : "+r" (buf_ptr)
+        : "wa" (v0), "wa" (v1));
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[16 + i]);
+        else
+            assert(v0[i] == buf[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[i]);
+        else
+            assert(v1[i] == buf[16 + i]); // FIXME
+    }
+
+    /* TODO: test signed offset */
+    /* TODO: PC-relative addresses */
+}
+
+/* TODO: we force 2 instead of 1 in opc2 currently to hack around
+ * QEMU impl, need a single handler to deal with the 1 in bit 31
+ */
+#define STXVP(_Sp, _RA, _DQ, _SX) \
+    ".align 6;" \
+    ".long 6 << 26 | (" #_Sp ") << 22 | (" #_SX ") << 21 | (" #_RA ") << 16 | (" #_DQ ") << 4 | 1;"
+
+void test_stxvp(void) {
+    register vector unsigned char v0 asm("vs4") = {
+        0, 1, 2, 3,
+        4, 5, 6, 7,
+        8, 9, 10, 11,
+        12, 13, 14, 15
+    };
+    register vector unsigned char v1 asm("vs5") = {
+        16, 17, 18, 19,
+        20, 21, 22, 23,
+        24, 25, 26, 27,
+        28, 29, 30, 31
+    };
+    uint8_t buf[64] __attribute__((aligned(16)));
+    uint8_t *buf_ptr = (uint8_t *)&buf;
+    int i;
+
+    for (i = 0; i < 64; i++) {
+        buf[i] = 7;
+    }
+
+    /* store v0,v1 into buf[0:31] using EA with no offset */
+    asm(STXVP(2, %0, 0, 0)
+        : "+r" (buf_ptr)
+        : "wa" (v0), "wa" (v1)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(buf[i] == v1[i]);
+        else
+            assert(buf[i] == v0[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(buf[16 + i] == v0[i]);
+        else
+            assert(buf[16 + i] == v1[i]); // FIXME
+    }
+
+    /* store v0,v1 into buf[32:63] using EA with offset 0x40 */
+    buf_ptr = buf_ptr + 32 - 0x40;
+    asm(STXVP(2, %0, 4, 0)
+        : "+r" (buf_ptr)
+        : "wa" (v0), "wa" (v1)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(buf[32 + i] == v1[i]);
+        else
+            assert(buf[32 + i] == v0[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(buf[32 + 16 + i] == v0[i]);
+        else
+            assert(buf[48 + i] == v1[i]); // FIXME
+    }
+
+    /* TODO: test signed offset */
+    /* TODO: PC-relative addresses */
+}
+
+#define LXVP(_Tp, _RA, _DQ, _TX) \
+    ".long 6 << 26 | (" #_Tp ") << 22 | (" #_TX ") << 21 | (" #_RA ") << 16 | (" #_DQ ") << 4;"
+
+void test_lxvp(void) {
+    register vector unsigned char v0 asm("vs4") = { 0 };
+    register vector unsigned char v1 asm("vs5") = { 0 };
+    uint8_t buf[64] __attribute__((aligned(16)));
+    uint8_t *buf_ptr = (uint8_t *)&buf;
+    int i;
+
+    for (i = 0; i < 64; i++) {
+        buf[i] = i;
+    }
+
+    /* load buf[0:31] into v0,v1 using EA with no offset */
+    asm(LXVP(2, %2, 0, 0)
+        : "=wa" (v0), "=wa" (v1)
+        : "r" (buf_ptr)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le) {
+            assert(v0[i] == buf[16 + i]);
+
+         } else
+            assert(v0[i] == buf[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[i]);
+        else
+            assert(v1[i] == buf[16+i]); // FIXME
+    }
+
+    /* load buf[32:63] into v0,v1 using EA with 0x40 offset */
+    buf_ptr = buf_ptr + 32 - 0x40;
+    asm(LXVP(2, %2, 4, 0)
+        : "=wa" (v0), "=wa" (v1)
+        : "r" (buf_ptr)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[32+16+i]);
+        else
+            assert(v0[i] == buf[32+i]); // FIXME
+
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[32+i]);
+        else
+            assert(v1[i] == buf[48+i]); // FIXME
+    }
+
+    /* TODO: signed offsets */
+    /* TODO: PC-relative addresses */
+}
+
+#define LXVPX(_Tp, _RA, _RB, _TX) \
+    ".long 31 << 26 | (" #_Tp ") << 22 | (" #_TX ") << 21 | (" #_RA ") << 16 | (" #_RB ") << 11 | 333 << 1;"
+
+void test_lxvpx(void) {
+    register vector unsigned char v0 asm("vs8") = { 0 };
+    register vector unsigned char v1 asm("vs9") = { 0 };
+    uint8_t buf[64] __attribute__((aligned(16)));
+    uint8_t *buf_ptr = (uint8_t *)&buf;
+    uint32_t offset;
+    int i;
+
+    for (i = 0; i < 64; i++) {
+        buf[i] = i;
+    }
+
+    /* load buf[0:31] into v0,v1 using EA with no offset */
+    offset = 0;
+    asm(LXVPX(4, %2, %3, 0)
+        : "=wa" (v0), "=wa" (v1)
+        : "r" (buf_ptr), "r" (offset)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[16 + i]);
+        else
+            assert(v0[i] == buf[i]); // FIXME
+
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[i]);
+        else
+            assert(v1[i] == buf[16+i]); // FIXME
+    }
+
+    /* load buf[32:63] into v0,v1 using EA with 0x40 offset */
+    offset = 0x40;
+    buf_ptr = buf_ptr + 32 - offset;
+    asm(LXVPX(4, %2, %3, 0)
+        : "=wa" (v0), "=wa" (v1)
+        : "r" (buf_ptr), "r" (offset)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[32 + 16 + i]);
+        else
+            assert(v0[i] == buf[32+i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[32 + i]);
+        else
+            assert(v1[i] == buf[48+i]); // FIXME
+    }
+
+    /* TODO: signed offsets */
+    /* TODO: PC-relative addresses */
+}
+
+#define STXVPX(_Sp, _RA, _RB, _SX) \
+    ".long 31 << 26 | (" #_Sp ") << 22 | (" #_SX ") << 21 | (" #_RA ") << 16 | (" #_RB ") << 11 | 461 << 1;"
+
+void test_stxvpx(void) {
+    register vector unsigned char v0 asm("vs10") = {
+// FIXME: reorder for readability
+        0, 1, 2, 3,
+        4, 5, 6, 7,
+        8, 9, 10, 11,
+        12, 13, 14, 15
+    };
+    register vector unsigned char v1 asm("vs11") = {
+// FIXME: ditto
+        16, 17, 18, 19,
+        20, 21, 22, 23,
+        24, 25, 26, 27,
+        28, 29, 30, 31
+    };
+    uint8_t buf[64] __attribute__((aligned(16)));
+    uint8_t *buf_ptr = (uint8_t *)&buf;
+    uint32_t offset;
+    int i;
+
+    for (i = 0; i < 64; i++) {
+        buf[i] = 7;
+    }
+
+    /* store v0,v1 into buf[0:31] using EA with no offset */
+    offset = 0;
+    asm(STXVPX(5, %0, %1, 0)
+        : "+r" (buf_ptr)
+        : "r" (offset), "wa" (v0), "wa" (v1)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(buf[i] == v1[i]);
+        else
+            assert(buf[i] == v0[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[16 + i]);
+        else
+            assert(buf[16 + i] == v1[i]); // FIXME
+    }
+
+    /* store v0,v1 into buf[32:63] using EA with offset 0x40 */
+    offset = 0x40;
+    buf_ptr = buf_ptr + 32 - offset;
+    asm(STXVPX(5, %0, %1, 0)
+        : "+r" (buf_ptr)
+        : "r" (offset), "wa" (v0), "wa" (v1)
+        );
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v0[i] == buf[32 + 16 + i]);
+        else
+            assert(buf[48 + i] == v1[i]); // FIXME
+    }
+
+    for (i = 0; i < 16; i++) {
+        if (le)
+            assert(v1[i] == buf[32 + i]);
+        else
+            assert(buf[32 + i] == v0[i]); // FIXME
+    }
+
+    /* TODO: test signed offset */
+    /* TODO: PC-relative addresses */
+}
+
+#define do_test(testname) \
+    if (debug) \
+        fprintf(stderr, "-> running test: " #testname "\n"); \
+    test_##testname(); \
+
+int main(int argc, char **argv)
+{
+    le = (htole16(1) == 1);
+
+    if (argc > 1 && !strcmp(argv[1], "-d")) {
+        debug = true;
+    }
+
+    do_test(lxvp);
+    do_test(stxvp);
+    do_test(plxvp);
+    do_test(plxvp_cia);
+    do_test(pstxvp);
+    do_test(lxvpx);
+    do_test(stxvpx);
+
+    dprintf("All tests passed\n");
+    return 0;
+}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 6/7] target/ppc: Add support for prefixed load/store FP instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
                   ` (4 preceding siblings ...)
  2020-12-16  9:08 ` [PATCH v2 5/7] tests/tcg: Add tests " Gustavo Romero
@ 2020-12-16  9:08 ` Gustavo Romero
  2020-12-16  9:08 ` [PATCH v2 7/7] tests/tcg: Add tests " Gustavo Romero
  2020-12-16  9:27 ` [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions no-reply
  7 siblings, 0 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:08 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

This commit adds support for the following load/store instructions for FP
registers:

  plf, plfd
  pstfs, pstfd

Signed-off-by: Michael Roth <mroth@lamentation.net>
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 target/ppc/translate/fp-impl.c.inc | 48 ++++++++++++++++++++++++++++++
 target/ppc/translate/fp-ops.c.inc  |  6 ++++
 2 files changed, 54 insertions(+)

diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc
index 9f7868ee28..1eec98de0f 100644
--- a/target/ppc/translate/fp-impl.c.inc
+++ b/target/ppc/translate/fp-impl.c.inc
@@ -874,6 +874,28 @@ static void glue(gen_, name)(DisasContext *ctx)                               \
     tcg_temp_free_i64(t0);                                                    \
 }
 
+#define GEN_PLDF(name, ldop, opc, type)                                       \
+static void glue(gen_, p##name)(DisasContext *ctx)                            \
+{                                                                             \
+    TCGv EA;                                                                  \
+    TCGv_i64 t0;                                                              \
+    if (unlikely(!ctx->fpu_enabled)) {                                        \
+        gen_exception(ctx, POWERPC_EXCP_FPU);                                 \
+        return;                                                               \
+    }                                                                         \
+    gen_set_access_type(ctx, ACCESS_FLOAT);                                   \
+    EA = tcg_temp_new();                                                      \
+    t0 = tcg_temp_new_i64();                                                  \
+    if (gen_addr_imm34_index(ctx, EA)) {                                      \
+        goto out;                                                             \
+    }                                                                         \
+    gen_qemu_##ldop(ctx, t0, EA);                                             \
+    set_fpr(rD(ctx->opcode), t0);                                             \
+out:                                                                          \
+    tcg_temp_free(EA);                                                        \
+    tcg_temp_free_i64(t0);                                                    \
+}
+
 #define GEN_LDUF(name, ldop, opc, type)                                       \
 static void glue(gen_, name##u)(DisasContext *ctx)                            \
 {                                                                             \
@@ -943,6 +965,7 @@ static void glue(gen_, name##x)(DisasContext *ctx)                            \
 
 #define GEN_LDFS(name, ldop, op, type)                                        \
 GEN_LDF(name, ldop, op | 0x20, type);                                         \
+GEN_PLDF(name, ldop, op | 0x20, type);                                        \
 GEN_LDUF(name, ldop, op | 0x21, type);                                        \
 GEN_LDUXF(name, ldop, op | 0x01, type);                                       \
 GEN_LDXF(name, ldop, 0x17, op | 0x00, type)
@@ -1109,6 +1132,28 @@ static void glue(gen_, name)(DisasContext *ctx)                               \
     tcg_temp_free_i64(t0);                                                    \
 }
 
+#define GEN_PSTF(name, stop, opc, type)                                       \
+static void glue(gen_, p##name)(DisasContext *ctx)                               \
+{                                                                             \
+    TCGv EA;                                                                  \
+    TCGv_i64 t0;                                                              \
+    if (unlikely(!ctx->fpu_enabled)) {                                        \
+        gen_exception(ctx, POWERPC_EXCP_FPU);                                 \
+        return;                                                               \
+    }                                                                         \
+    gen_set_access_type(ctx, ACCESS_FLOAT);                                   \
+    EA = tcg_temp_new();                                                      \
+    t0 = tcg_temp_new_i64();                                                  \
+    if (gen_addr_imm34_index(ctx, EA)) {                                      \
+        goto out;                                                             \
+    }                                                                         \
+    get_fpr(t0, rS(ctx->opcode));                                             \
+    gen_qemu_##stop(ctx, t0, EA);                                             \
+out:                                                                          \
+    tcg_temp_free(EA);                                                        \
+    tcg_temp_free_i64(t0);                                                    \
+}
+
 #define GEN_STUF(name, stop, opc, type)                                       \
 static void glue(gen_, name##u)(DisasContext *ctx)                            \
 {                                                                             \
@@ -1178,6 +1223,7 @@ static void glue(gen_, name##x)(DisasContext *ctx)                            \
 
 #define GEN_STFS(name, stop, op, type)                                        \
 GEN_STF(name, stop, op | 0x20, type);                                         \
+GEN_PSTF(name, stop, op | 0x20, type);                                        \
 GEN_STUF(name, stop, op | 0x21, type);                                        \
 GEN_STUXF(name, stop, op | 0x01, type);                                       \
 GEN_STXF(name, stop, 0x17, op | 0x00, type)
@@ -1483,12 +1529,14 @@ static void gen_stfqx(DisasContext *ctx)
 #undef GEN_FLOAT_BS
 
 #undef GEN_LDF
+#undef GEN_PLDF
 #undef GEN_LDUF
 #undef GEN_LDUXF
 #undef GEN_LDXF
 #undef GEN_LDFS
 
 #undef GEN_STF
+#undef GEN_PSTF
 #undef GEN_STUF
 #undef GEN_STUXF
 #undef GEN_STXF
diff --git a/target/ppc/translate/fp-ops.c.inc b/target/ppc/translate/fp-ops.c.inc
index 88fab65628..fed0db8f65 100644
--- a/target/ppc/translate/fp-ops.c.inc
+++ b/target/ppc/translate/fp-ops.c.inc
@@ -52,6 +52,8 @@ GEN_FLOAT_B(rim, 0x08, 0x0F, 1, PPC_FLOAT_EXT),
 
 #define GEN_LDF(name, ldop, opc, type)                                        \
 GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
+#define GEN_PLDF(name, ldop, opc, type)                                       \
+GEN_HANDLER_E_PREFIXED_M(p##name, opc, 0xFF, 0xFF, 0x0, PPC_64B, PPC2_ISA310),
 #define GEN_LDUF(name, ldop, opc, type)                                       \
 GEN_HANDLER(name##u, opc, 0xFF, 0xFF, 0x00000000, type),
 #define GEN_LDUXF(name, ldop, opc, type)                                      \
@@ -60,6 +62,7 @@ GEN_HANDLER(name##ux, 0x1F, 0x17, opc, 0x00000001, type),
 GEN_HANDLER(name##x, 0x1F, opc2, opc3, 0x00000001, type),
 #define GEN_LDFS(name, ldop, op, type)                                        \
 GEN_LDF(name, ldop, op | 0x20, type)                                          \
+GEN_PLDF(name, ldop, op | 0x20, type)                                         \
 GEN_LDUF(name, ldop, op | 0x21, type)                                         \
 GEN_LDUXF(name, ldop, op | 0x01, type)                                        \
 GEN_LDXF(name, ldop, 0x17, op | 0x00, type)
@@ -73,6 +76,8 @@ GEN_HANDLER_E(lfdpx, 0x1F, 0x17, 0x18, 0x00200001, PPC_NONE, PPC2_ISA205),
 
 #define GEN_STF(name, stop, opc, type)                                        \
 GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
+#define GEN_PSTF(name, ldop, opc, type)                                       \
+GEN_HANDLER_E_PREFIXED_M(p##name, opc, 0xFF, 0xFF, 0x0, PPC_64B, PPC2_ISA310),
 #define GEN_STUF(name, stop, opc, type)                                       \
 GEN_HANDLER(name##u, opc, 0xFF, 0xFF, 0x00000000, type),
 #define GEN_STUXF(name, stop, opc, type)                                      \
@@ -81,6 +86,7 @@ GEN_HANDLER(name##ux, 0x1F, 0x17, opc, 0x00000001, type),
 GEN_HANDLER(name##x, 0x1F, opc2, opc3, 0x00000001, type),
 #define GEN_STFS(name, stop, op, type)                                        \
 GEN_STF(name, stop, op | 0x20, type)                                          \
+GEN_PSTF(name, stop, op | 0x20, type)                                         \
 GEN_STUF(name, stop, op | 0x21, type)                                         \
 GEN_STUXF(name, stop, op | 0x01, type)                                        \
 GEN_STXF(name, stop, 0x17, op | 0x00, type)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 7/7] tests/tcg: Add tests for prefixed load/store FP instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
                   ` (5 preceding siblings ...)
  2020-12-16  9:08 ` [PATCH v2 6/7] target/ppc: Add support for prefixed load/store FP instructions Gustavo Romero
@ 2020-12-16  9:08 ` Gustavo Romero
  2020-12-16  9:27 ` [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions no-reply
  7 siblings, 0 replies; 10+ messages in thread
From: Gustavo Romero @ 2020-12-16  9:08 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: peter.maydell, gromero, gustavo.romero, Michael Roth, mroth, clg,
	david, alex.bennee, rth

From: Michael Roth <mdroth@linux.vnet.ibm.com>

Signed-off-by: Michael Roth <mroth@lamentation.net>
[ gromero: - tweaks in debug output ]
Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
---
 tests/tcg/ppc64le/Makefile.target             |   4 +
 .../test-prefixed-load-store-fp.c             | 270 ++++++++++++++++++
 2 files changed, 274 insertions(+)
 create mode 100644 tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c

diff --git a/tests/tcg/ppc64le/Makefile.target b/tests/tcg/ppc64le/Makefile.target
index aaa43a83a7..c6eac40b74 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -9,6 +9,7 @@ VPATH += $(SRC_PATH)/tests/tcg/$(TARGET_NAME)/instruction-tests
 QEMU_OPTS += -cpu power10
 
 PPC_TESTS = test-prefixed-load-store
+PPC_TESTS += test-prefixed-load-store-fp
 
 TESTS += $(PPC_TESTS)
 PPC_VSX_TESTS = test-paired-load-store-vsx
@@ -23,3 +24,6 @@ test-prefixed-load-store: test-prefixed-load-store.c
 
 test-paired-load-store-vsx: test-paired-load-store-vsx.c
 	gcc $< -o test-paired-load-store-vsx
+
+test-prefixed-load-store-fp: test-prefixed-load-store-fp.c
+	gcc $< -o test-prefixed-load-store-fp
diff --git a/tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c b/tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c
new file mode 100644
index 0000000000..baf8c3febf
--- /dev/null
+++ b/tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c
@@ -0,0 +1,270 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <endian.h>
+#include <string.h>
+#include <float.h>
+
+bool debug = false;
+
+#define dprintf(...) \
+    do { \
+        if (debug == true) { \
+            fprintf(stderr, "%s: ", __func__); \
+            fprintf(stderr, __VA_ARGS__); \
+        } \
+    } while (0);
+
+bool le;
+
+#define PLFS(_FRT, _RA, _d0, _d1, _R) \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 48<<26 | (" #_FRT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PSTFS(_FRS, _RA, _d0, _d1, _R) \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 52<<26 | (" #_FRS ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PLFD(_FRT, _RA, _d0, _d1, _R) \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 50<<26 | (" #_FRT ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+#define PSTFD(_FRS, _RA, _d0, _d1, _R) \
+    ".long 1<<26 | 2<<24 | (" #_R ")<<20 | (" #_d0 ")\n" \
+    ".long 54<<26 | (" #_FRS ")<<21 | (" #_RA ")<<16 | (" #_d1 ")\n"
+
+void test_plfs(void) {
+    float dest = 0;
+    float dest_copy = 0;
+    float src = FLT_MAX;
+    void *src_ptr = &src;
+
+    /* sanity check against lfs */
+    asm(
+        "lfs %0, 0(%2)"
+        : "+f" (dest_copy)
+        : "f" (src), "r" (src_ptr));
+
+    asm(
+        PLFS(%0, %2, 0, 0, 0)
+        : "+f" (dest)
+        : "f" (src), "r" (src_ptr));
+
+    assert(dest == src);
+    assert(dest_copy == dest);
+}
+
+void test_pstfs(void) {
+    float dest = 0;
+    float dest_copy = 0;
+    float src = FLT_MAX;
+    void *dest_ptr = &dest;
+    void *dest_copy_ptr = &dest_copy;
+
+    /* sanity check against stfs */
+    asm(
+        "stfs %1, 0(%0)"
+        : "+r" (dest_copy_ptr)
+        : "f" (src));
+
+    asm(
+        PSTFS(%1, %0, 0, 0, 0)
+        : "+r" (dest_ptr)
+        : "f" (src));
+
+    assert(dest == src);
+    assert(dest_copy == dest);
+}
+
+void test_plfd(void) {
+    double dest = 0;
+    double dest_copy = 0;
+    double src = DBL_MAX;
+    void *src_ptr = &src;
+
+    /* sanity check against lfd */
+    asm(
+        "lfd %0, 0(%2)"
+        : "+d" (dest_copy)
+        : "d" (src), "r" (src_ptr));
+
+    asm(
+        PLFD(%0, %2, 0, 0, 0)
+        : "+d" (dest)
+        : "d" (src), "r" (src_ptr));
+
+    assert(dest == src);
+    assert(dest_copy == dest);
+}
+
+void test_pstfd(void) {
+    double dest = 0;
+    double dest_copy = 0;
+    double src = DBL_MAX;
+    void *dest_ptr = &dest;
+    void *dest_copy_ptr = &dest_copy;
+
+    /* sanity check against stfs */
+    asm(
+        "stfd %1, 0(%0)"
+        : "+r" (dest_copy_ptr)
+        : "f" (src));
+
+    asm(
+        PSTFD(%1, %0, 0, 0, 0)
+        : "+r" (dest_ptr)
+        : "f" (src));
+
+    assert(dest == src);
+    assert(dest_copy == dest);
+}
+
+void test_plfs_offset(void) {
+    float dest;
+    float src = FLT_MAX;
+    void *src_ptr = &src;
+    void *src_ptr_offset;
+
+    src_ptr_offset = src_ptr - 1;
+    dest = 0;
+    asm(
+        PLFS(%0, %2, 0, 0x1, 0)
+        : "=f" (dest)
+        : "f" (src), "r" (src_ptr_offset));
+    assert(dest == src);
+
+    src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+    dest = 0;
+    asm(
+        PLFS(%0, %2, 0x1FFFF, 0xFFFF, 0)
+        : "=f" (dest)
+        : "f" (src), "r" (src_ptr_offset));
+    assert(dest == src);
+
+    src_ptr_offset = src_ptr + 1;
+    dest = 0;
+    asm(
+        PLFS(%0, %2, 0x3FFFF, 0xFFFF, 0)
+        : "=f" (dest)
+        : "f" (src), "r" (src_ptr_offset));
+    assert(dest == src);
+}
+
+void test_pstfs_offset(void) {
+    float dest;
+    float src = FLT_MAX;
+    void *dest_ptr = &dest;
+    void *dest_ptr_offset;
+
+    dest_ptr_offset = dest_ptr - 1;
+    dest = 0;
+    asm(
+        PSTFS(%1, %0, 0x0, 0x1, 0)
+        : "+r" (dest_ptr_offset)
+        : "f" (src));
+    assert(dest == src);
+
+    dest_ptr_offset = dest_ptr - 0x1FFFFFFFF;
+    dest = 0;
+    asm(
+        PSTFS(%1, %0, 0x1FFFF, 0xFFFF, 0)
+        : "+r" (dest_ptr_offset)
+        : "f" (src));
+    assert(dest == src);
+
+    dest_ptr_offset = dest_ptr + 1;
+    dest = 0;
+    asm(
+        PSTFS(%1, %0, 0x3FFFF, 0xFFFF, 0)
+        : "+r" (dest_ptr_offset)
+        : "f" (src));
+    assert(dest == src);
+}
+
+void test_plfd_offset(void) {
+    double dest;
+    double src = DBL_MAX;
+    void *src_ptr = &src;
+    void *src_ptr_offset;
+
+    src_ptr_offset = src_ptr - 1;
+    dest = 0;
+    asm(
+        PLFD(%0, %2, 0, 0x1, 0)
+        : "+f" (dest)
+        : "f" (src), "r" (src_ptr_offset));
+    assert(dest == src);
+
+    src_ptr_offset = src_ptr - 0x1FFFFFFFF;
+    dest = 0;
+    asm(
+        PLFD(%0, %2, 0x1FFFF, 0xFFFF, 0)
+        : "+f" (dest)
+        : "f" (src), "r" (src_ptr_offset));
+    assert(dest == src);
+
+    src_ptr_offset = src_ptr + 1;
+    dest = 0;
+    asm(
+        PLFD(%0, %2, 0x3FFFF, 0xFFFF, 0)
+        : "+f" (dest)
+        : "f" (src), "r" (src_ptr_offset));
+    assert(dest == src);
+}
+
+void test_pstfd_offset(void) {
+    double dest;
+    double src = DBL_MAX;
+    void *dest_ptr = &dest;
+    void *dest_ptr_offset;
+
+    dest_ptr_offset = dest_ptr - 1;
+    dest = 0;
+    asm(
+        PSTFD(%1, %0, 0x0, 0x1, 0)
+        : "+r" (dest_ptr_offset)
+        : "f" (src));
+    assert(dest == src);
+
+    dest_ptr_offset = dest_ptr - 0x1FFFFFFFF;
+    dest = 0;
+    asm(
+        PSTFD(%1, %0, 0x1FFFF, 0xFFFF, 0)
+        : "+r" (dest_ptr_offset)
+        : "f" (src));
+    assert(dest == src);
+
+    dest_ptr_offset = dest_ptr + 1;
+    dest = 0;
+    asm(
+        PSTFD(%1, %0, 0x3FFFF, 0xFFFF, 0)
+        : "+r" (dest_ptr_offset)
+        : "f" (src));
+    assert(dest == src);
+}
+
+#define do_test(testname) \
+    if (debug) \
+        fprintf(stderr, "-> running test: " #testname "\n"); \
+    test_##testname(); \
+
+int main(int argc, char **argv)
+{
+    le = (htole16(1) == 1);
+
+    if (argc > 1 && !strcmp(argv[1], "-d")) {
+        debug = true;
+    }
+
+    do_test(plfs);
+    do_test(pstfs);
+    do_test(plfd);
+    do_test(pstfd);
+
+    do_test(plfs_offset);
+    do_test(pstfs_offset);
+    do_test(plfd_offset);
+    do_test(pstfd_offset);
+
+    dprintf("All tests passed\n");
+    return 0;
+}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions
  2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
                   ` (6 preceding siblings ...)
  2020-12-16  9:08 ` [PATCH v2 7/7] tests/tcg: Add tests " Gustavo Romero
@ 2020-12-16  9:27 ` no-reply
  7 siblings, 0 replies; 10+ messages in thread
From: no-reply @ 2020-12-16  9:27 UTC (permalink / raw)
  To: gromero
  Cc: peter.maydell, gromero, gustavo.romero, qemu-devel, mroth,
	qemu-ppc, clg, rth, alex.bennee, david

Patchew URL: https://patchew.org/QEMU/20201216090804.58640-1-gromero@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201216090804.58640-1-gromero@linux.ibm.com
Subject: [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20201215174815.51520-1-drjones@redhat.com -> patchew/20201215174815.51520-1-drjones@redhat.com
 - [tag update]      patchew/20201215224133.3545901-1-ehabkost@redhat.com -> patchew/20201215224133.3545901-1-ehabkost@redhat.com
 * [new tag]         patchew/20201216090804.58640-1-gromero@linux.ibm.com -> patchew/20201216090804.58640-1-gromero@linux.ibm.com
Switched to a new branch 'test'
4afd921 tests/tcg: Add tests for prefixed load/store FP instructions
2f4db24 target/ppc: Add support for prefixed load/store FP instructions
09b1856 tests/tcg: Add tests for paired vector load/store instructions
84275cf target/ppc: Add support for paired vector load/store instructions
4ef6a3a tests/tcg: Add tests for prefixed load/store instructions
660dbc6 target/ppc: Add support for prefixed load/store instructions
2f816dd target/ppc: Add infrastructure for prefixed instructions

=== OUTPUT BEGIN ===
1/7 Checking commit 2f816dd856fd (target/ppc: Add infrastructure for prefixed instructions)
2/7 Checking commit 660dbc6b7475 (target/ppc: Add support for prefixed load/store instructions)
ERROR: open brace '{' following function declarations go on the next line
#262: FILE: target/ppc/translate.c:2980:
+static void gen_pstd(DisasContext *ctx) {

ERROR: open brace '{' following function declarations go on the next line
#274: FILE: target/ppc/translate.c:2992:
+static void gen_pstq(DisasContext *ctx) {

WARNING: line over 80 characters
#318: FILE: target/ppc/translate.c:7320:
+GEN_HANDLER_E_PREFIXED_M(paddi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#327: FILE: target/ppc/translate.c:7383:
+GEN_HANDLER_E_PREFIXED_M(plbz, 0x22, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#328: FILE: target/ppc/translate.c:7384:
+GEN_HANDLER_E_PREFIXED_M(plhz, 0x28, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#329: FILE: target/ppc/translate.c:7385:
+GEN_HANDLER_E_PREFIXED_M(plha, 0x2a, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#330: FILE: target/ppc/translate.c:7386:
+GEN_HANDLER_E_PREFIXED_M(plwz, 0x20, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#331: FILE: target/ppc/translate.c:7387:
+GEN_HANDLER_E_PREFIXED(plwa, 0x29, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#334: FILE: target/ppc/translate.c:7390:
+GEN_HANDLER_E_PREFIXED_M(pstb, 0x26, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#335: FILE: target/ppc/translate.c:7391:
+GEN_HANDLER_E_PREFIXED_M(psth, 0x2c, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#336: FILE: target/ppc/translate.c:7392:
+GEN_HANDLER_E_PREFIXED_M(pstw, 0x24, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#337: FILE: target/ppc/translate.c:7393:
+GEN_HANDLER_E_PREFIXED(pstd, 0x3d, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

WARNING: line over 80 characters
#338: FILE: target/ppc/translate.c:7394:
+GEN_HANDLER_E_PREFIXED(pstq, 0x3c, 0xFF, 0xFF, 0x00000000, PPC_64B, PPC2_ISA310),

total: 2 errors, 11 warnings, 306 lines checked

Patch 2/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/7 Checking commit 4ef6a3a5b704 (tests/tcg: Add tests for prefixed load/store instructions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#19: 
new file mode 120000

ERROR: do not initialise globals to 0 or NULL
#62: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:9:
+bool debug = false;

ERROR: suspicious ; after while (0)
#70: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:17:
+    } while (0);

ERROR: spaces required around that '%' (ctx:BxV)
#94: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:41:
+            op(%1, %0, offset_upper18, offset_lower16, r)              \
                ^

ERROR: spaces required around that '%' (ctx:WxV)
#94: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:41:
+            op(%1, %0, offset_upper18, offset_lower16, r)              \
                    ^

ERROR: suspicious ; after while (0)
#97: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:44:
+    } while (0);

ERROR: open brace '{' following function declarations go on the next line
#99: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:46:
+void check_pst(uint64_t src, uint64_t dest, uint64_t dest_orig, int width) {

ERROR: spaces required around that '*' (ctx:VxV)
#101: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:48:
+    uint64_t src_mask = (width == 8) ? -1UL : (1UL << (8*width)) - 1;
                                                         ^

ERROR: spaces required around that '*' (ctx:VxV)
#104: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:51:
+        dest_orig_mask = -1UL << (8*width);
                                    ^

ERROR: spaces required around that '*' (ctx:VxV)
#107: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:54:
+        dest_orig_mask = (-1UL << (8*width)) >> (8*width);
                                     ^

ERROR: spaces required around that '*' (ctx:VxV)
#107: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:54:
+        dest_orig_mask = (-1UL << (8*width)) >> (8*width);
                                                   ^

ERROR: line over 90 characters
#108: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:55:
+        assert(dest == ((dest_orig & dest_orig_mask) | ((src & src_mask) << (8*(8-width)))));

ERROR: spaces required around that '*' (ctx:VxV)
#108: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:55:
+        assert(dest == ((dest_orig & dest_orig_mask) | ((src & src_mask) << (8*(8-width)))));
                                                                               ^

ERROR: spaces required around that '-' (ctx:VxV)
#108: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:55:
+        assert(dest == ((dest_orig & dest_orig_mask) | ((src & src_mask) << (8*(8-width)))));
                                                                                  ^

ERROR: open brace '{' following function declarations go on the next line
#112: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:59:
+void test_pst_offset(int width) {

ERROR: open brace '{' following function declarations go on the next line
#182: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:129:
+void test_pst(int width) {

ERROR: open brace '{' following function declarations go on the next line
#242: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:189:
+void test_pstb(void) {

ERROR: open brace '{' following function declarations go on the next line
#247: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:194:
+void test_psth(void) {

ERROR: open brace '{' following function declarations go on the next line
#252: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:199:
+void test_pstw(void) {

ERROR: open brace '{' following function declarations go on the next line
#257: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:204:
+void test_pstd(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#290: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:237:
+            op(%0, %2, offset_upper18, offset_lower16, r)                   \
                ^

ERROR: spaces required around that '%' (ctx:WxV)
#290: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:237:
+            op(%0, %2, offset_upper18, offset_lower16, r)                   \
                    ^

ERROR: suspicious ; after while (0)
#293: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:240:
+    } while (0);

ERROR: open brace '{' following function declarations go on the next line
#295: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:242:
+void check_pl_z(uint64_t src, uint64_t dest, int width) {

ERROR: spaces required around that '*' (ctx:VxV)
#299: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:246:
+        src_mask = (width == 8) ? -1UL : (1UL << (8*width)) - 1;
                                                    ^

ERROR: spaces required around that '*' (ctx:VxV)
#302: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:249:
+        src_mask = (width == 8) ? -1UL : -1UL << (8*(8-width));
                                                    ^

ERROR: spaces required around that '-' (ctx:VxV)
#302: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:249:
+        src_mask = (width == 8) ? -1UL : -1UL << (8*(8-width));
                                                       ^

ERROR: spaces required around that '*' (ctx:VxV)
#303: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:250:
+        assert(dest == (src & src_mask) >> (8*(8-width)));
                                              ^

ERROR: spaces required around that '-' (ctx:VxV)
#303: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:250:
+        assert(dest == (src & src_mask) >> (8*(8-width)));
                                                 ^

ERROR: open brace '{' following function declarations go on the next line
#307: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:254:
+void check_pl_a(uint64_t src, uint64_t dest, int width) {

WARNING: Block comments use a leading /* on a separate line
#310: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:257:
+    /* TODO: docs suggest testing high-order bit of src byte/halfword/etc, but

ERROR: spaces required around that '*' (ctx:VxV)
#316: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:263:
+        sign_mask = (src & (1UL << (width*8-1))) ? -1UL << (8*width) : 0;
                                          ^

ERROR: spaces required around that '-' (ctx:VxV)
#316: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:263:
+        sign_mask = (src & (1UL << (width*8-1))) ? -1UL << (8*width) : 0;
                                            ^

ERROR: spaces required around that '*' (ctx:VxV)
#316: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:263:
+        sign_mask = (src & (1UL << (width*8-1))) ? -1UL << (8*width) : 0;
                                                              ^

ERROR: spaces required around that '*' (ctx:VxV)
#318: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:265:
+        sign_mask = (src & (1UL << 63)) ? -1UL << (8*width) : 0;
                                                     ^

ERROR: spaces required around that '*' (ctx:VxV)
#322: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:269:
+        src_mask = (width == 8) ? -1UL : (1UL << (8*width)) - 1;
                                                    ^

ERROR: spaces required around that '*' (ctx:VxV)
#325: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:272:
+        src_mask = (width == 8) ? -1UL : -1UL << (8*(8-width));
                                                    ^

ERROR: spaces required around that '-' (ctx:VxV)
#325: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:272:
+        src_mask = (width == 8) ? -1UL : -1UL << (8*(8-width));
                                                       ^

ERROR: spaces required around that '*' (ctx:VxV)
#326: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:273:
+        assert(dest == (((src & src_mask) >> (8*(8-width))) | sign_mask));
                                                ^

ERROR: spaces required around that '-' (ctx:VxV)
#326: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:273:
+        assert(dest == (((src & src_mask) >> (8*(8-width))) | sign_mask));
                                                   ^

ERROR: open brace '{' following function declarations go on the next line
#330: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:277:
+void test_pl_a(int width, uint64_t src, uint64_t dest_orig) {

ERROR: open brace '{' following function declarations go on the next line
#435: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:382:
+void test_pl_z(int width, uint64_t src, uint64_t dest_orig) {

ERROR: open brace '{' following function declarations go on the next line
#569: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:516:
+void test_plbz(void) {

ERROR: open brace '{' following function declarations go on the next line
#575: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:522:
+void test_plhz(void) {

ERROR: open brace '{' following function declarations go on the next line
#581: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:528:
+void test_plha(void) {

ERROR: open brace '{' following function declarations go on the next line
#587: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:534:
+void test_plwz(void) {

ERROR: open brace '{' following function declarations go on the next line
#593: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:540:
+void test_plwa(void) {

ERROR: open brace '{' following function declarations go on the next line
#599: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:546:
+void test_pld(void) {

ERROR: open brace '{' following function declarations go on the next line
#611: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:558:
+void test_pstq(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#634: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:581:
+        PSTQ(22, %0, 0, 0, 0)
                  ^

ERROR: open brace '{' following function declarations go on the next line
#665: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:612:
+void test_plq(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#720: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:667:
+        PLQ(20, %2, 0, 0, 0)
                 ^

ERROR: "(foo*)" should be "(foo *)"
#737: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:684:
+        assert(dest0a == ((uint64_t*)src)[0]);

ERROR: "(foo*)" should be "(foo *)"
#738: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:685:
+        assert(dest1a == ((uint64_t*)src)[1]);

ERROR: "(foo*)" should be "(foo *)"
#742: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:689:
+        assert(dest0a == ((uint64_t*)src)[0]);

ERROR: "(foo*)" should be "(foo *)"
#743: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:690:
+        assert(dest1a == ((uint64_t*)src)[1]);

ERROR: open brace '{' following function declarations go on the next line
#751: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:698:
+void test_plq2(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#807: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:754:
+        PLQ(20, %2, 0, 0, 0)
                 ^

ERROR: open brace '{' following function declarations go on the next line
#834: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:781:
+void test_plbz_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#838: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:785:
+        PLBZ(%0, 0, 0, 8 /* skip plbz */ + 4 /* skip b */, 1)

ERROR: spaces required around that '%' (ctx:BxV)
#838: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:785:
+        PLBZ(%0, 0, 0, 8 /* skip plbz */ + 4 /* skip b */, 1)
              ^

ERROR: open brace '{' following function declarations go on the next line
#850: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:797:
+void test_plhz_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#854: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:801:
+        PLHZ(%0, 0, 0, 8 /* skip plhz */ + 4 /* skip b */, 1)

ERROR: spaces required around that '%' (ctx:BxV)
#854: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:801:
+        PLHZ(%0, 0, 0, 8 /* skip plhz */ + 4 /* skip b */, 1)
              ^

ERROR: open brace '{' following function declarations go on the next line
#870: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:817:
+void test_plha_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#874: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:821:
+        PLHA(%0, 0, 0, 8 /* skip plha */ + 4 /* skip b */, 1)

ERROR: spaces required around that '%' (ctx:BxV)
#874: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:821:
+        PLHA(%0, 0, 0, 8 /* skip plha */ + 4 /* skip b */, 1)
              ^

ERROR: open brace '{' following function declarations go on the next line
#894: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:841:
+void test_plwz_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#898: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:845:
+        PLWZ(%0, 0, 0, 8 /* skip plwz */ + 4 /* skip b */, 1)

ERROR: spaces required around that '%' (ctx:BxV)
#898: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:845:
+        PLWZ(%0, 0, 0, 8 /* skip plwz */ + 4 /* skip b */, 1)
              ^

ERROR: open brace '{' following function declarations go on the next line
#914: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:861:
+void test_plwa_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#918: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:865:
+        PLWA(%0, 0, 0, 8 /* skip plwa */ + 4 /* skip b */, 1)

ERROR: spaces required around that '%' (ctx:BxV)
#918: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:865:
+        PLWA(%0, 0, 0, 8 /* skip plwa */ + 4 /* skip b */, 1)
              ^

ERROR: open brace '{' following function declarations go on the next line
#938: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:885:
+void test_pld_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#942: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:889:
+        PLD(%0, 0, 0, 8 /* skip pld */ + 4 /* skip b */, 1)

ERROR: spaces required around that '%' (ctx:BxV)
#942: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:889:
+        PLD(%0, 0, 0, 8 /* skip pld */ + 4 /* skip b */, 1)
             ^

ERROR: Macros with multiple statements should be enclosed in a do - while loop
#962: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store.c:909:
+#define do_test(testname) \
+    if (debug) \
+        fprintf(stderr, "-> running test: " #testname "\n"); \
+    test_##testname(); \
+

total: 69 errors, 8 warnings, 962 lines checked

Patch 3/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/7 Checking commit 84275cf80953 (target/ppc: Add support for paired vector load/store instructions)
ERROR: do not use C99 // comments
#85: FILE: target/ppc/mem_helper.c:554:
+    } else { // TODO: check if it's correct for BE.

ERROR: do not use C99 // comments
#114: FILE: target/ppc/mem_helper.c:583:
+    } else { // TODO: check if it's correct for BE.

WARNING: line over 80 characters
#140: FILE: target/ppc/translate.c:7401:
+GEN_HANDLER_E_PREFIXED(plxvp, 0x3A, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA310),

WARNING: line over 80 characters
#141: FILE: target/ppc/translate.c:7402:
+GEN_HANDLER_E_PREFIXED(pstxvp, 0x3E, 0xFF, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA310),

total: 2 errors, 2 warnings, 181 lines checked

Patch 4/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/7 Checking commit 09b18567aad4 (tests/tcg: Add tests for paired vector load/store instructions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

ERROR: do not initialise globals to 0 or NULL
#50: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:10:
+bool debug = false;

ERROR: suspicious ; after while (0)
#58: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:18:
+    } while (0);

ERROR: line over 90 characters
#65: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:25:
+    ".long 58 << 26 | (" #_Tp ") << 22 | (" #_TX ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"

ERROR: open brace '{' following function declarations go on the next line
#67: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:27:
+void test_plxvp_cia(void) {

WARNING: Block comments use a leading /* on a separate line
#74: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:34:
+        PLXVP(4, 0, 0, 8 /* skip plxvp */ + 4 /* skip b */, 1, 0)

ERROR: braces {} are necessary for all arms of this statement
#112: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:72:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#115: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:75:
+            assert(v0[i] == (31 - i)); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#119: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:79:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#122: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:82:
+            assert(v1[i] == 15 - i); // FIXME

ERROR: open brace '{' following function declarations go on the next line
#126: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:86:
+void test_plxvp(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#138: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:98:
+    asm(PLXVP(3, %2, 0, 0, 0, 0)
                  ^

ERROR: braces {} are necessary for all arms of this statement
#143: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:103:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#146: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:106:
+            assert(v0[i] == buf[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#150: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:110:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#153: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:113:
+            assert(v1[i] == buf[16 + i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#158: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:118:
+    asm(PLXVP(3, %2, 0, 0x1000, 0, 0)
                  ^

ERROR: braces {} are necessary for all arms of this statement
#163: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:123:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#166: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:126:
+            assert(v0[i] == buf[32 + i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#170: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:130:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#173: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:133:
+            assert(v1[i] == buf[48 + i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#179: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:139:
+    asm(PLXVP(3, %2, 0x1000, 0x1000, 0, 0)
                  ^

ERROR: braces {} are necessary for all arms of this statement
#184: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:144:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#187: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:147:
+            assert(v0[i] == buf[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#191: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:151:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#194: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:154:
+            assert(v1[i] == buf[16 + i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#202: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:162:
+    asm(PLXVP(3, %2, 0x3ffff, 0xf000, 0, 0)
                  ^

ERROR: braces {} are necessary for all arms of this statement
#207: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:167:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#210: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:170:
+            assert(v0[i] == buf[32 + i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#214: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:174:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#217: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:177:
+            assert(v1[i] == buf[48 + i]); // FIXME

ERROR: line over 90 characters
#224: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:184:
+    ".long 62 << 26 | (" #_Sp ") << 22 | (" #_SX ") << 21 | (" #_RA ") << 16 | (" #_d1 ");"

ERROR: open brace '{' following function declarations go on the next line
#226: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:186:
+void test_pstxvp(void) {

ERROR: do not use C99 // comments
#228: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:188:
+// FIXME: reorder values for readability

ERROR: do not use C99 // comments
#235: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:195:
+// FIXME: reorder values for readability

ERROR: spaces required around that '%' (ctx:WxV)
#250: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:210:
+    asm(PSTXVP(3, %0, 0, 0, 0, 0)
                   ^

ERROR: braces {} are necessary for all arms of this statement
#255: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:215:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#258: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:218:
+            assert(v0[i] == buf[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#262: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:222:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#265: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:225:
+            assert(v1[i] == buf[16 + i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#270: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:230:
+    asm(PSTXVP(3, %0, 0, 0x1000, 0, 0)
                   ^

ERROR: braces {} are necessary for all arms of this statement
#275: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:235:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#278: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:238:
+            assert(v0[i] == buf[32 + i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#282: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:242:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#285: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:245:
+            assert(v1[i] == buf[48 + i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#291: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:251:
+    asm(PSTXVP(3, %0, 0x1000, 0x1000, 0, 0)
                   ^

ERROR: braces {} are necessary for all arms of this statement
#296: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:256:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#299: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:259:
+            assert(v0[i] == buf[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#303: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:263:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#306: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:266:
+            assert(v1[i] == buf[16 + i]); // FIXME

WARNING: Block comments use a leading /* on a separate line
#313: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:273:
+/* TODO: we force 2 instead of 1 in opc2 currently to hack around

ERROR: line over 90 characters
#318: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:278:
+    ".long 6 << 26 | (" #_Sp ") << 22 | (" #_SX ") << 21 | (" #_RA ") << 16 | (" #_DQ ") << 4 | 1;"

ERROR: open brace '{' following function declarations go on the next line
#320: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:280:
+void test_stxvp(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#342: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:302:
+    asm(STXVP(2, %0, 0, 0)
                  ^

ERROR: braces {} are necessary for all arms of this statement
#348: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:308:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#351: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:311:
+            assert(buf[i] == v0[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#355: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:315:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#358: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:318:
+            assert(buf[16 + i] == v1[i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#363: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:323:
+    asm(STXVP(2, %0, 4, 0)
                  ^

ERROR: braces {} are necessary for all arms of this statement
#369: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:329:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#372: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:332:
+            assert(buf[32 + i] == v0[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#376: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:336:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#379: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:339:
+            assert(buf[48 + i] == v1[i]); // FIXME

ERROR: line over 90 characters
#387: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:347:
+    ".long 6 << 26 | (" #_Tp ") << 22 | (" #_TX ") << 21 | (" #_RA ") << 16 | (" #_DQ ") << 4;"

ERROR: open brace '{' following function declarations go on the next line
#389: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:349:
+void test_lxvp(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#401: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:361:
+    asm(LXVP(2, %2, 0, 0)
                 ^

ERROR: braces {} are necessary for all arms of this statement
#407: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:367:
+        if (le) {
[...]
+         } else
[...]

ERROR: do not use C99 // comments
#411: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:371:
+            assert(v0[i] == buf[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#415: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:375:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#418: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:378:
+            assert(v1[i] == buf[16+i]); // FIXME

ERROR: spaces required around that '+' (ctx:VxV)
#418: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:378:
+            assert(v1[i] == buf[16+i]); // FIXME
                                   ^

ERROR: spaces required around that '%' (ctx:WxV)
#423: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:383:
+    asm(LXVP(2, %2, 4, 0)
                 ^

ERROR: braces {} are necessary for all arms of this statement
#429: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:389:
+        if (le)
[...]
+        else
[...]

ERROR: spaces required around that '+' (ctx:VxV)
#430: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:390:
+            assert(v0[i] == buf[32+16+i]);
                                   ^

ERROR: spaces required around that '+' (ctx:VxV)
#430: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:390:
+            assert(v0[i] == buf[32+16+i]);
                                      ^

ERROR: do not use C99 // comments
#432: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:392:
+            assert(v0[i] == buf[32+i]); // FIXME

ERROR: spaces required around that '+' (ctx:VxV)
#432: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:392:
+            assert(v0[i] == buf[32+i]); // FIXME
                                   ^

ERROR: braces {} are necessary for all arms of this statement
#437: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:397:
+        if (le)
[...]
+        else
[...]

ERROR: spaces required around that '+' (ctx:VxV)
#438: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:398:
+            assert(v1[i] == buf[32+i]);
                                   ^

ERROR: do not use C99 // comments
#440: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:400:
+            assert(v1[i] == buf[48+i]); // FIXME

ERROR: spaces required around that '+' (ctx:VxV)
#440: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:400:
+            assert(v1[i] == buf[48+i]); // FIXME
                                   ^

ERROR: line over 90 characters
#448: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:408:
+    ".long 31 << 26 | (" #_Tp ") << 22 | (" #_TX ") << 21 | (" #_RA ") << 16 | (" #_RB ") << 11 | 333 << 1;"

ERROR: open brace '{' following function declarations go on the next line
#450: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:410:
+void test_lxvpx(void) {

ERROR: spaces required around that '%' (ctx:WxV)
#464: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:424:
+    asm(LXVPX(4, %2, %3, 0)
                  ^

ERROR: spaces required around that '%' (ctx:WxV)
#464: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:424:
+    asm(LXVPX(4, %2, %3, 0)
                      ^

ERROR: braces {} are necessary for all arms of this statement
#470: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:430:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#473: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:433:
+            assert(v0[i] == buf[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#478: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:438:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#481: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:441:
+            assert(v1[i] == buf[16+i]); // FIXME

ERROR: spaces required around that '+' (ctx:VxV)
#481: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:441:
+            assert(v1[i] == buf[16+i]); // FIXME
                                   ^

ERROR: spaces required around that '%' (ctx:WxV)
#487: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:447:
+    asm(LXVPX(4, %2, %3, 0)
                  ^

ERROR: spaces required around that '%' (ctx:WxV)
#487: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:447:
+    asm(LXVPX(4, %2, %3, 0)
                      ^

ERROR: braces {} are necessary for all arms of this statement
#493: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:453:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#496: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:456:
+            assert(v0[i] == buf[32+i]); // FIXME

ERROR: spaces required around that '+' (ctx:VxV)
#496: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:456:
+            assert(v0[i] == buf[32+i]); // FIXME
                                   ^

ERROR: braces {} are necessary for all arms of this statement
#500: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:460:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#503: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:463:
+            assert(v1[i] == buf[48+i]); // FIXME

ERROR: spaces required around that '+' (ctx:VxV)
#503: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:463:
+            assert(v1[i] == buf[48+i]); // FIXME
                                   ^

ERROR: line over 90 characters
#511: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:471:
+    ".long 31 << 26 | (" #_Sp ") << 22 | (" #_SX ") << 21 | (" #_RA ") << 16 | (" #_RB ") << 11 | 461 << 1;"

ERROR: open brace '{' following function declarations go on the next line
#513: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:473:
+void test_stxvpx(void) {

ERROR: do not use C99 // comments
#515: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:475:
+// FIXME: reorder for readability

ERROR: do not use C99 // comments
#522: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:482:
+// FIXME: ditto

ERROR: spaces required around that '%' (ctx:WxV)
#539: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:499:
+    asm(STXVPX(5, %0, %1, 0)
                   ^

ERROR: spaces required around that '%' (ctx:WxV)
#539: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:499:
+    asm(STXVPX(5, %0, %1, 0)
                       ^

ERROR: braces {} are necessary for all arms of this statement
#545: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:505:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#548: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:508:
+            assert(buf[i] == v0[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#552: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:512:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#555: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:515:
+            assert(buf[16 + i] == v1[i]); // FIXME

ERROR: spaces required around that '%' (ctx:WxV)
#561: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:521:
+    asm(STXVPX(5, %0, %1, 0)
                   ^

ERROR: spaces required around that '%' (ctx:WxV)
#561: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:521:
+    asm(STXVPX(5, %0, %1, 0)
                       ^

ERROR: braces {} are necessary for all arms of this statement
#567: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:527:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#570: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:530:
+            assert(buf[48 + i] == v1[i]); // FIXME

ERROR: braces {} are necessary for all arms of this statement
#574: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:534:
+        if (le)
[...]
+        else
[...]

ERROR: do not use C99 // comments
#577: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:537:
+            assert(buf[32 + i] == v0[i]); // FIXME

ERROR: Macros with multiple statements should be enclosed in a do - while loop
#584: FILE: tests/tcg/ppc64le/instruction-tests/test-paired-load-store-vsx.c:544:
+#define do_test(testname) \
+    if (debug) \
+        fprintf(stderr, "-> running test: " #testname "\n"); \
+    test_##testname(); \
+

total: 112 errors, 3 warnings, 582 lines checked

Patch 5/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/7 Checking commit 2f4db24b2a05 (target/ppc: Add support for prefixed load/store FP instructions)
7/7 Checking commit 4afd9214dccd (tests/tcg: Add tests for prefixed load/store FP instructions)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: do not initialise globals to 0 or NULL
#46: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:10:
+bool debug = false;

ERROR: suspicious ; after while (0)
#54: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:18:
+    } while (0);

ERROR: open brace '{' following function declarations go on the next line
#71: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:35:
+void test_plfs(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#84: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:48:
+        PLFS(%0, %2, 0, 0, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#84: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:48:
+        PLFS(%0, %2, 0, 0, 0)
                  ^

ERROR: open brace '{' following function declarations go on the next line
#92: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:56:
+void test_pstfs(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#106: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:70:
+        PSTFS(%1, %0, 0, 0, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#106: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:70:
+        PSTFS(%1, %0, 0, 0, 0)
                   ^

ERROR: open brace '{' following function declarations go on the next line
#114: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:78:
+void test_plfd(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#127: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:91:
+        PLFD(%0, %2, 0, 0, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#127: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:91:
+        PLFD(%0, %2, 0, 0, 0)
                  ^

ERROR: open brace '{' following function declarations go on the next line
#135: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:99:
+void test_pstfd(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#149: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:113:
+        PSTFD(%1, %0, 0, 0, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#149: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:113:
+        PSTFD(%1, %0, 0, 0, 0)
                   ^

ERROR: open brace '{' following function declarations go on the next line
#157: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:121:
+void test_plfs_offset(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#166: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:130:
+        PLFS(%0, %2, 0, 0x1, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#166: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:130:
+        PLFS(%0, %2, 0, 0x1, 0)
                  ^

ERROR: spaces required around that '%' (ctx:BxV)
#174: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:138:
+        PLFS(%0, %2, 0x1FFFF, 0xFFFF, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#174: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:138:
+        PLFS(%0, %2, 0x1FFFF, 0xFFFF, 0)
                  ^

ERROR: spaces required around that '%' (ctx:BxV)
#182: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:146:
+        PLFS(%0, %2, 0x3FFFF, 0xFFFF, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#182: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:146:
+        PLFS(%0, %2, 0x3FFFF, 0xFFFF, 0)
                  ^

ERROR: open brace '{' following function declarations go on the next line
#188: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:152:
+void test_pstfs_offset(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#197: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:161:
+        PSTFS(%1, %0, 0x0, 0x1, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#197: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:161:
+        PSTFS(%1, %0, 0x0, 0x1, 0)
                   ^

ERROR: spaces required around that '%' (ctx:BxV)
#205: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:169:
+        PSTFS(%1, %0, 0x1FFFF, 0xFFFF, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#205: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:169:
+        PSTFS(%1, %0, 0x1FFFF, 0xFFFF, 0)
                   ^

ERROR: spaces required around that '%' (ctx:BxV)
#213: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:177:
+        PSTFS(%1, %0, 0x3FFFF, 0xFFFF, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#213: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:177:
+        PSTFS(%1, %0, 0x3FFFF, 0xFFFF, 0)
                   ^

ERROR: open brace '{' following function declarations go on the next line
#219: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:183:
+void test_plfd_offset(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#228: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:192:
+        PLFD(%0, %2, 0, 0x1, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#228: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:192:
+        PLFD(%0, %2, 0, 0x1, 0)
                  ^

ERROR: spaces required around that '%' (ctx:BxV)
#236: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:200:
+        PLFD(%0, %2, 0x1FFFF, 0xFFFF, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#236: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:200:
+        PLFD(%0, %2, 0x1FFFF, 0xFFFF, 0)
                  ^

ERROR: spaces required around that '%' (ctx:BxV)
#244: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:208:
+        PLFD(%0, %2, 0x3FFFF, 0xFFFF, 0)
              ^

ERROR: spaces required around that '%' (ctx:WxV)
#244: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:208:
+        PLFD(%0, %2, 0x3FFFF, 0xFFFF, 0)
                  ^

ERROR: open brace '{' following function declarations go on the next line
#250: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:214:
+void test_pstfd_offset(void) {

ERROR: spaces required around that '%' (ctx:BxV)
#259: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:223:
+        PSTFD(%1, %0, 0x0, 0x1, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#259: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:223:
+        PSTFD(%1, %0, 0x0, 0x1, 0)
                   ^

ERROR: spaces required around that '%' (ctx:BxV)
#267: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:231:
+        PSTFD(%1, %0, 0x1FFFF, 0xFFFF, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#267: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:231:
+        PSTFD(%1, %0, 0x1FFFF, 0xFFFF, 0)
                   ^

ERROR: spaces required around that '%' (ctx:BxV)
#275: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:239:
+        PSTFD(%1, %0, 0x3FFFF, 0xFFFF, 0)
               ^

ERROR: spaces required around that '%' (ctx:WxV)
#275: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:239:
+        PSTFD(%1, %0, 0x3FFFF, 0xFFFF, 0)
                   ^

ERROR: Macros with multiple statements should be enclosed in a do - while loop
#281: FILE: tests/tcg/ppc64le/instruction-tests/test-prefixed-load-store-fp.c:245:
+#define do_test(testname) \
+    if (debug) \
+        fprintf(stderr, "-> running test: " #testname "\n"); \
+    test_##testname(); \
+

total: 43 errors, 1 warnings, 283 lines checked

Patch 7/7 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201216090804.58640-1-gromero@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/7] target/ppc: Add infrastructure for prefixed instructions
  2020-12-16  9:07 ` [PATCH v2 1/7] target/ppc: Add infrastructure for " Gustavo Romero
@ 2021-01-05  4:39   ` David Gibson
  0 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2021-01-05  4:39 UTC (permalink / raw)
  To: Gustavo Romero
  Cc: peter.maydell, gustavo.romero, qemu-devel, mroth, Michael Roth,
	qemu-ppc, clg, alex.bennee, rth

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

On Wed, Dec 16, 2020 at 06:07:58AM -0300, Gustavo Romero wrote:
> From: Michael Roth <mdroth@linux.vnet.ibm.com>
> 
> Some prefixed instructions (Type 0 and 1, e.g. 8-byte Load/Store or 8LS),
> have a completely seperate namespace for their primary opcodes.
> 
> Other prefixed instructions (Type 2 and 3, e.g. Modified Load/Store or MLS)
> actually re-use existing opcodes to provide a modified variant. We could
> handle these by extending the existing opcode handlers to check for the
> prefix and handle accordingly, but in some cases it is cleaner to define
> seperate handlers for these in their own table entry, and avoids the need
> to add error-handling to existing handlers for cases where they are called
> for a prefixed Type 2/3 instruction but don't implement the handling for
> them. In the future we can re-work things to support both approaches if
> cases arise where that seems warranted.
> 
> Then we have the normal non-prefixed instructions.
> 
> To handle all 3 of these cases we extend the table size 3x, with normal
> instructions indexed directly by their primary opcodes, Type 0/1 indexed by
> primary opcode + PPC_CPU_PREFIXED_OPCODE_OFFSET, and Type 2/3 indexed by
> primary opcode + PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET.
> 
> Various exception/SRR handling changes related to prefixed instructions are
> yet to be implemented. For instance, no alignment interrupt is generated if
> a prefixed instruction crosses the 64-byte boundary; no SRR bit related to
> prefixed instructions is set on any interrupt, like for FP unavailable
> interrupt, data storage interrupt, etc.
> 
> For details, please see PowerISA v3.1, particularly the following sections:
> 
> 1.6 Instruction Formats, p. 11
> 1.6.3  Instruction Prefix Formats, p. 22
> 3.3.2  Fixed-Point Load Instructions, p. 51
> 4.6.2  Floating-Point Load Instructions, p. 149
> Chapter 7 Interrupts, p. 1247
> 
> Signed-off-by: Michael Roth <mroth@lamentation.net>
> [ gromero: - comments clean up and updates
>            - additional comments in the commit log ]
> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>

I'm still not seeing any advantage to putting both the plain and
prefixed opcodes into a single table.  Essentially you're taking the
prefix and base opcode and encoding them into a single index.  Which
isn't inherently wrong, except that the only thing you do with that
single index is effectively decode it back into the original two
parts.

More specifically

[snip]
> +static uint32_t opc1_idx(DisasContext *ctx)
> +{
> +    uint32_t table_offset = 0;
> +
> +    switch (ctx->prefix_subtype) {
> +    case PREFIX_ST_8LS:
> +    case PREFIX_ST_8MLS:
> +    case PREFIX_ST_8RR:
> +    case PREFIX_ST_8MRR:
> +        table_offset = PPC_CPU_PREFIXED_OPCODE_OFFSET;
> +        break;
> +    case PREFIX_ST_MLS:
> +    case PREFIX_ST_MMLS:
> +    case PREFIX_ST_MRR:
> +    case PREFIX_ST_MMRR:
> +    case PREFIX_ST_MMIRR:
> +        table_offset = PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET;
> +        break;
> +    }
> +
> +    return table_offset + opc1(ctx->opcode);
> +}

Here, you translate the prefix type into a table offset, but...

> +
>  static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
>  {
>      DisasContext *ctx = container_of(dcbase, DisasContext, base);
> @@ -8004,14 +8142,40 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
>  
>      ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
>                                        need_byteswap(ctx));
> +    /* check for prefix */
> +    ctx->prefix_subtype = parse_prefix_subtype(ctx->opcode);
> +    if (ctx->prefix_subtype == PREFIX_ST_INVALID) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported prefix: "
> +                      "%08x " TARGET_FMT_lx "\n",
> +                      ctx->prefix, ctx->base.pc_next);
> +    } else if (ctx->prefix_subtype != PREFIX_ST_NONE) {
> +        /*
> +         * this is the 4-byte prefix of an instruction, read the
> +         * next 4 and advance the PC
> +         *
> +         * TODO: we can optimize this to do a single load since we
> +         * read in target_long anyways already
> +         *
> +         * double-check endianess cases.
> +         *
> +         * engineering note about endianess changing based on rfid
> +         * or interrupt. does this need to be accounted for here?
> +         */
> +        ctx->prefix = ctx->opcode;
> +        ctx->base.pc_next += 4;
> +        ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
> +                                          need_byteswap(ctx));
> +    } else {
> +        ctx->prefix = 0;
> +    }
>  
> -    LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
> +    LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) prefix %08x (%s)\n",
>                ctx->opcode, opc1(ctx->opcode), opc2(ctx->opcode),
> -              opc3(ctx->opcode), opc4(ctx->opcode),
> +              opc3(ctx->opcode), opc4(ctx->opcode), ctx->prefix,
>                ctx->le_mode ? "little" : "big");
>      ctx->base.pc_next += 4;
>      table = cpu->opcodes;
> -    handler = table[opc1(ctx->opcode)];
> +    handler = table[opc1_idx(ctx)];

Here, at the only caller, you already have some conditionals on prefix
type, so you could equally well just use a different table.

>      if (is_indirect_opcode(handler)) {
>          table = ind_table(handler);
>          handler = table[opc2(ctx->opcode)];
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index bb66526280..0ea8c2c5c1 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -9563,8 +9563,13 @@ static int insert_in_table(opc_handler_t **table, unsigned char idx,
>  }
>  
>  static int register_direct_insn(opc_handler_t **ppc_opcodes,
> -                                unsigned char idx, opc_handler_t *handler)
> +                                unsigned char idx, opc_handler_t *handler,
> +                                bool prefixed, bool modified)
>  {
> +    if (prefixed) {
> +        idx = modified ? idx + PPC_CPU_PREFIXED_MODIFIED_OPCODE_OFFSET
> +                       : idx + PPC_CPU_PREFIXED_OPCODE_OFFSET;
> +    }
>      if (insert_in_table(ppc_opcodes, idx, handler) < 0) {
>          printf("*** ERROR: opcode %02x already assigned in main "
>                 "opcode table\n", idx);
> @@ -9688,7 +9693,8 @@ static int register_insn(opc_handler_t **ppc_opcodes, opcode_t *insn)
>              }
>          }
>      } else {
> -        if (register_direct_insn(ppc_opcodes, insn->opc1, &insn->handler) < 0) {
> +        if (register_direct_insn(ppc_opcodes, insn->opc1, &insn->handler,
> +                                 insn->prefixed, insn->modified) < 0) {
>              return -1;
>          }
>      }
> @@ -9766,6 +9772,7 @@ static void dump_ppc_insns(CPUPPCState *env)
>      for (opc1 = 0x00; opc1 < PPC_CPU_OPCODES_LEN; opc1++) {
>          table = env->opcodes;
>          handler = table[opc1];
> +        /* TODO: need to update opcode dump to account for prefixed space */
>          if (is_indirect_opcode(handler)) {
>              /* opc2 is 5 bits long */
>              for (opc2 = 0; opc2 < PPC_CPU_INDIRECT_OPCODES_LEN; opc2++) {

-- 
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: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-01-05  4:40 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-16  9:07 [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions Gustavo Romero
2020-12-16  9:07 ` [PATCH v2 1/7] target/ppc: Add infrastructure for " Gustavo Romero
2021-01-05  4:39   ` David Gibson
2020-12-16  9:07 ` [PATCH v2 2/7] target/ppc: Add support for prefixed load/store instructions Gustavo Romero
2020-12-16  9:08 ` [PATCH v2 3/7] tests/tcg: Add tests " Gustavo Romero
2020-12-16  9:08 ` [PATCH v2 4/7] target/ppc: Add support for paired vector " Gustavo Romero
2020-12-16  9:08 ` [PATCH v2 5/7] tests/tcg: Add tests " Gustavo Romero
2020-12-16  9:08 ` [PATCH v2 6/7] target/ppc: Add support for prefixed load/store FP instructions Gustavo Romero
2020-12-16  9:08 ` [PATCH v2 7/7] tests/tcg: Add tests " Gustavo Romero
2020-12-16  9:27 ` [PATCH v2 0/7] PPC64: Add support for the new prefixed instructions no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.