qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions
@ 2021-04-27 17:16 Luis Pires
  2021-04-27 17:16 ` [PATCH v2 01/15] decodetree: Add support for " Luis Pires
                   ` (14 more replies)
  0 siblings, 15 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, Luis Pires, lagarcia, bruno.larsen,
	matheus.ferst, david

This series provides the basic infrastructure for adding the new 32/64-bit
instructions in Power ISA 3.1 to target/ppc.

It starts by changing decodetree.py to support 64-bit instructions,
then changes the target/ppc code to allow 32- and 64-bit instructions
to be decoded using decodetree, and finishes by adding the implementation
for an initial group of instructions to demonstrate the new approach:
- addis/addis/paddi
- pnop
- integer loads/stores (both prefixed and non-prefixed)

Link to the changes in Github:
https://github.com/PPC64/qemu/tree/lffpires-ppc-isa31-1

v2:
- Store current pc in ctx instead of insn_size
- Use separate decode files for 32- and 64-bit instructions
- Improvements to the exception/is_jmp logic
- Use translator_loop_temp_check()
- Moved logic to prevent translation from crossing page boundaries
- Additional instructions using decodetree: addis, pnop, loads/stores
- Added check for prefixed insn support in cpu flags

This code contains contributions from Richard Henderson, Matheus Ferst
and myself.

Luis Pires (2):
  decodetree: Add support for 64-bit instructions
  target/ppc: Check cpu flags for prefixed insn support

Richard Henderson (13):
  target/ppc: Add cia field to DisasContext
  target/ppc: Split out decode_legacy
  target/ppc: Move DISAS_NORETURN setting into gen_exception*
  target/ppc: Tidy exception vs exit_tb
  target/ppc: Mark helper_raise_exception* as noreturn
  target/ppc: Use translator_loop_temp_check
  target/ppc: Add infrastructure for prefixed insns
  target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
  target/ppc: Implement PNOP
  target/ppc: Move D/DS/X-form integer loads to decodetree
  target/ppc: Implement prefixed integer load instructions
  target/ppc: Move D/DS/X-form integer stores to decodetree
  target/ppc: Implement prefixed integer store instructions

 docs/devel/decodetree.rst                  |   5 +-
 scripts/decodetree.py                      |  26 +-
 target/ppc/cpu.h                           |   1 +
 target/ppc/helper.h                        |   4 +-
 target/ppc/insn32.decode                   |  85 ++++
 target/ppc/insn64.decode                   |  64 +++
 target/ppc/meson.build                     |   9 +
 target/ppc/translate.c                     | 513 +++++++--------------
 target/ppc/translate/fixedpoint-impl.c.inc | 424 +++++++++++++++++
 target/ppc/translate_init.c.inc            |  42 +-
 10 files changed, 798 insertions(+), 375 deletions(-)
 create mode 100644 target/ppc/insn32.decode
 create mode 100644 target/ppc/insn64.decode
 create mode 100644 target/ppc/translate/fixedpoint-impl.c.inc

-- 
2.25.1



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

* [PATCH v2 01/15] decodetree: Add support for 64-bit instructions
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 02/15] target/ppc: Add cia field to DisasContext Luis Pires
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, Luis Pires, lagarcia, bruno.larsen,
	matheus.ferst, david

Allow '64' to be specified for the instruction width command line params
and use the appropriate insn/field data types, mask, extract and deposit
functions in that case.

This will be used to implement the new 64-bit Power ISA 3.1 instructions.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
Message-Id: <CP2PR80MB3668E123E2EFDB0ACD3A46F1DA759@CP2PR80MB3668.lamprd80.prod.outlook.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 docs/devel/decodetree.rst |  5 +++--
 scripts/decodetree.py     | 26 +++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst
index 74f66bf46e..d776dae14f 100644
--- a/docs/devel/decodetree.rst
+++ b/docs/devel/decodetree.rst
@@ -40,8 +40,9 @@ and returns an integral value extracted from there.
 
 A field with no ``unnamed_fields`` and no ``!function`` is in error.
 
-FIXME: the fields of the structure into which this result will be stored
-is restricted to ``int``.  Which means that we cannot expand 64-bit items.
+The fields of the structure into which this result will be stored are
+defined as ``int`` when the instruction size is set to 16 or 32 bits
+and as ``int64_t`` when the instruction size is set to 64 bits.
 
 Field examples:
 
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 4637b633e7..26156dfc36 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -42,6 +42,10 @@
 output_fd = None
 insntype = 'uint32_t'
 decode_function = 'decode'
+field_data_type = 'int'
+extract_function = 'extract32'
+sextract_function = 'sextract32'
+deposit_function = 'deposit32'
 
 # An identifier for C.
 re_C_ident = '[a-zA-Z][a-zA-Z0-9_]*'
@@ -185,9 +189,9 @@ def __str__(self):
 
     def str_extract(self):
         if self.sign:
-            extr = 'sextract32'
+            extr = sextract_function
         else:
-            extr = 'extract32'
+            extr = extract_function
         return '{0}(insn, {1}, {2})'.format(extr, self.pos, self.len)
 
     def __eq__(self, other):
@@ -215,8 +219,9 @@ def str_extract(self):
             if pos == 0:
                 ret = f.str_extract()
             else:
-                ret = 'deposit32({0}, {1}, {2}, {3})' \
-                      .format(ret, pos, 32 - pos, f.str_extract())
+                ret = '{4}({0}, {1}, {2}, {3})' \
+                      .format(ret, pos, insnwidth - pos,
+                              f.str_extract(), deposit_function)
             pos += f.len
         return ret
 
@@ -311,7 +316,7 @@ def output_def(self):
         if not self.extern:
             output('typedef struct {\n')
             for n in self.fields:
-                output('    int ', n, ';\n')
+                output('    ', field_data_type, ' ', n, ';\n')
             output('} ', self.struct_name(), ';\n\n')
 # end Arguments
 
@@ -1264,6 +1269,10 @@ def main():
     global insntype
     global insnmask
     global decode_function
+    global extract_function
+    global sextract_function
+    global deposit_function
+    global field_data_type
     global variablewidth
     global anyextern
 
@@ -1293,6 +1302,13 @@ def main():
             if insnwidth == 16:
                 insntype = 'uint16_t'
                 insnmask = 0xffff
+            elif insnwidth == 64:
+                insntype = 'uint64_t'
+                insnmask = 0xffffffffffffffff
+                field_data_type = 'int64_t'
+                extract_function = 'extract64'
+                sextract_function = 'sextract64'
+                deposit_function = 'deposit64'
             elif insnwidth != 32:
                 error(0, 'cannot handle insns of width', insnwidth)
         else:
-- 
2.25.1



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

* [PATCH v2 02/15] target/ppc: Add cia field to DisasContext
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
  2021-04-27 17:16 ` [PATCH v2 01/15] decodetree: Add support for " Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-28 14:55   ` Richard Henderson
  2021-04-27 17:16 ` [PATCH v2 03/15] target/ppc: Split out decode_legacy Luis Pires
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0984ce637b..ee25badba2 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -154,6 +154,7 @@ void ppc_translate_init(void)
 /* internal defines */
 struct DisasContext {
     DisasContextBase base;
+    target_ulong cia;  /* current instruction address */
     uint32_t opcode;
     uint32_t exception;
     /* Routine used to access memory */
@@ -254,7 +255,7 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
      * faulting instruction
      */
     if (ctx->exception == POWERPC_EXCP_NONE) {
-        gen_update_nip(ctx, ctx->base.pc_next - 4);
+        gen_update_nip(ctx, ctx->cia);
     }
     t0 = tcg_const_i32(excp);
     t1 = tcg_const_i32(error);
@@ -273,7 +274,7 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
      * faulting instruction
      */
     if (ctx->exception == POWERPC_EXCP_NONE) {
-        gen_update_nip(ctx, ctx->base.pc_next - 4);
+        gen_update_nip(ctx, ctx->cia);
     }
     t0 = tcg_const_i32(excp);
     gen_helper_raise_exception(cpu_env, t0);
@@ -3113,7 +3114,7 @@ static void gen_eieio(DisasContext *ctx)
          */
         if (!(ctx->insns_flags2 & PPC2_ISA300)) {
             qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
-                          TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
+                          TARGET_FMT_lx "\n", ctx->cia);
         } else {
             bar = TCG_MO_ST_LD;
         }
@@ -3782,14 +3783,14 @@ static void gen_b(DisasContext *ctx)
     li = LI(ctx->opcode);
     li = (li ^ 0x02000000) - 0x02000000;
     if (likely(AA(ctx->opcode) == 0)) {
-        target = ctx->base.pc_next + li - 4;
+        target = ctx->cia + li;
     } else {
         target = li;
     }
     if (LK(ctx->opcode)) {
         gen_setlr(ctx, ctx->base.pc_next);
     }
-    gen_update_cfar(ctx, ctx->base.pc_next - 4);
+    gen_update_cfar(ctx, ctx->cia);
     gen_goto_tb(ctx, 0, target);
 }
 
@@ -3888,11 +3889,11 @@ static void gen_bcond(DisasContext *ctx, int type)
         }
         tcg_temp_free_i32(temp);
     }
-    gen_update_cfar(ctx, ctx->base.pc_next - 4);
+    gen_update_cfar(ctx, ctx->cia);
     if (type == BCOND_IM) {
         target_ulong li = (target_long)((int16_t)(BD(ctx->opcode)));
         if (likely(AA(ctx->opcode) == 0)) {
-            gen_goto_tb(ctx, 0, ctx->base.pc_next + li - 4);
+            gen_goto_tb(ctx, 0, ctx->cia + li);
         } else {
             gen_goto_tb(ctx, 0, li);
         }
@@ -4008,7 +4009,7 @@ static void gen_rfi(DisasContext *ctx)
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_start();
     }
-    gen_update_cfar(ctx, ctx->base.pc_next - 4);
+    gen_update_cfar(ctx, ctx->cia);
     gen_helper_rfi(cpu_env);
     gen_sync_exception(ctx);
 #endif
@@ -4025,7 +4026,7 @@ static void gen_rfid(DisasContext *ctx)
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_start();
     }
-    gen_update_cfar(ctx, ctx->base.pc_next - 4);
+    gen_update_cfar(ctx, ctx->cia);
     gen_helper_rfid(cpu_env);
     gen_sync_exception(ctx);
 #endif
@@ -4042,7 +4043,7 @@ static void gen_rfscv(DisasContext *ctx)
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_start();
     }
-    gen_update_cfar(ctx, ctx->base.pc_next - 4);
+    gen_update_cfar(ctx, ctx->cia);
     gen_helper_rfscv(cpu_env);
     gen_sync_exception(ctx);
 #endif
@@ -4338,7 +4339,7 @@ static inline void gen_op_mfspr(DisasContext *ctx)
             if (sprn != SPR_PVR) {
                 qemu_log_mask(LOG_GUEST_ERROR, "Trying to read privileged spr "
                               "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn,
-                              ctx->base.pc_next - 4);
+                              ctx->cia);
             }
             gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
         }
@@ -4352,7 +4353,7 @@ static inline void gen_op_mfspr(DisasContext *ctx)
         /* Not defined */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "Trying to read invalid spr %d (0x%03x) at "
-                      TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
+                      TARGET_FMT_lx "\n", sprn, sprn, ctx->cia);
 
         /*
          * The behaviour depends on MSR:PR and SPR# bit 0x10, it can
@@ -4516,7 +4517,7 @@ static void gen_mtspr(DisasContext *ctx)
             /* Privilege exception */
             qemu_log_mask(LOG_GUEST_ERROR, "Trying to write privileged spr "
                           "%d (0x%03x) at " TARGET_FMT_lx "\n", sprn, sprn,
-                          ctx->base.pc_next - 4);
+                          ctx->cia);
             gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
         }
     } else {
@@ -4530,7 +4531,7 @@ static void gen_mtspr(DisasContext *ctx)
         /* Not defined */
         qemu_log_mask(LOG_GUEST_ERROR,
                       "Trying to write invalid spr %d (0x%03x) at "
-                      TARGET_FMT_lx "\n", sprn, sprn, ctx->base.pc_next - 4);
+                      TARGET_FMT_lx "\n", sprn, sprn, ctx->cia);
 
 
         /*
@@ -8002,6 +8003,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     LOG_DISAS("nip=" TARGET_FMT_lx " super=%d ir=%d\n",
               ctx->base.pc_next, ctx->mem_idx, (int)msr_ir);
 
+    ctx->cia = ctx->base.pc_next;
     ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
                                       need_byteswap(ctx));
 
@@ -8031,7 +8033,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
                       TARGET_FMT_lx " %d\n",
                       opc1(ctx->opcode), opc2(ctx->opcode),
                       opc3(ctx->opcode), opc4(ctx->opcode),
-                      ctx->opcode, ctx->base.pc_next - 4, (int)msr_ir);
+                      ctx->opcode, ctx->cia, (int)msr_ir);
     } else {
         uint32_t inval;
 
@@ -8048,7 +8050,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
                           TARGET_FMT_lx "\n", ctx->opcode & inval,
                           opc1(ctx->opcode), opc2(ctx->opcode),
                           opc3(ctx->opcode), opc4(ctx->opcode),
-                          ctx->opcode, ctx->base.pc_next - 4);
+                          ctx->opcode, ctx->cia);
             gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
             ctx->base.is_jmp = DISAS_NORETURN;
             return;
-- 
2.25.1



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

* [PATCH v2 03/15] target/ppc: Split out decode_legacy
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
  2021-04-27 17:16 ` [PATCH v2 01/15] decodetree: Add support for " Luis Pires
  2021-04-27 17:16 ` [PATCH v2 02/15] target/ppc: Add cia field to DisasContext Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception* Luis Pires
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate.c | 115 +++++++++++++++++++++++------------------
 1 file changed, 64 insertions(+), 51 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ee25badba2..ebe5afe7ae 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7876,6 +7876,62 @@ void ppc_cpu_dump_statistics(CPUState *cs, int flags)
 #endif
 }
 
+static bool decode_legacy(PowerPCCPU *cpu, DisasContext *ctx, uint32_t insn)
+{
+    opc_handler_t **table, *handler;
+    uint32_t inval;
+
+    ctx->opcode = insn;
+
+    LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
+              insn, opc1(insn), opc2(insn), opc3(insn), opc4(insn),
+              ctx->le_mode ? "little" : "big");
+
+    table = cpu->opcodes;
+    handler = table[opc1(insn)];
+    if (is_indirect_opcode(handler)) {
+        table = ind_table(handler);
+        handler = table[opc2(insn)];
+        if (is_indirect_opcode(handler)) {
+            table = ind_table(handler);
+            handler = table[opc3(insn)];
+            if (is_indirect_opcode(handler)) {
+                table = ind_table(handler);
+                handler = table[opc4(insn)];
+            }
+        }
+    }
+
+    /* Is opcode *REALLY* valid ? */
+    if (unlikely(handler->handler == &gen_invalid)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported opcode: "
+                      "%02x - %02x - %02x - %02x (%08x) "
+                      TARGET_FMT_lx "\n",
+                      opc1(insn), opc2(insn), opc3(insn), opc4(insn),
+                      insn, ctx->cia);
+        return false;
+    }
+
+    if (unlikely(handler->type & (PPC_SPE | PPC_SPE_SINGLE | PPC_SPE_DOUBLE)
+                 && Rc(insn))) {
+        inval = handler->inval2;
+    } else {
+        inval = handler->inval1;
+    }
+
+    if (unlikely((insn & inval) != 0)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "invalid bits: %08x for opcode: "
+                      "%02x - %02x - %02x - %02x (%08x) "
+                      TARGET_FMT_lx "\n", insn & inval,
+                      opc1(insn), opc2(insn), opc3(insn), opc4(insn),
+                      insn, ctx->cia);
+        return false;
+    }
+
+    handler->handler(ctx);
+    return true;
+}
+
 static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
@@ -7997,66 +8053,23 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = cs->env_ptr;
-    opc_handler_t **table, *handler;
+    uint32_t insn;
+    bool ok;
 
     LOG_DISAS("----------------\n");
     LOG_DISAS("nip=" TARGET_FMT_lx " super=%d ir=%d\n",
               ctx->base.pc_next, ctx->mem_idx, (int)msr_ir);
 
     ctx->cia = ctx->base.pc_next;
-    ctx->opcode = translator_ldl_swap(env, ctx->base.pc_next,
-                                      need_byteswap(ctx));
-
-    LOG_DISAS("translate opcode %08x (%02x %02x %02x %02x) (%s)\n",
-              ctx->opcode, opc1(ctx->opcode), opc2(ctx->opcode),
-              opc3(ctx->opcode), opc4(ctx->opcode),
-              ctx->le_mode ? "little" : "big");
+    insn = translator_ldl_swap(env, ctx->base.pc_next, need_byteswap(ctx));
     ctx->base.pc_next += 4;
-    table = cpu->opcodes;
-    handler = table[opc1(ctx->opcode)];
-    if (is_indirect_opcode(handler)) {
-        table = ind_table(handler);
-        handler = table[opc2(ctx->opcode)];
-        if (is_indirect_opcode(handler)) {
-            table = ind_table(handler);
-            handler = table[opc3(ctx->opcode)];
-            if (is_indirect_opcode(handler)) {
-                table = ind_table(handler);
-                handler = table[opc4(ctx->opcode)];
-            }
-        }
-    }
-    /* Is opcode *REALLY* valid ? */
-    if (unlikely(handler->handler == &gen_invalid)) {
-        qemu_log_mask(LOG_GUEST_ERROR, "invalid/unsupported opcode: "
-                      "%02x - %02x - %02x - %02x (%08x) "
-                      TARGET_FMT_lx " %d\n",
-                      opc1(ctx->opcode), opc2(ctx->opcode),
-                      opc3(ctx->opcode), opc4(ctx->opcode),
-                      ctx->opcode, ctx->cia, (int)msr_ir);
-    } else {
-        uint32_t inval;
 
-        if (unlikely(handler->type & (PPC_SPE | PPC_SPE_SINGLE | PPC_SPE_DOUBLE)
-                     && Rc(ctx->opcode))) {
-            inval = handler->inval2;
-        } else {
-            inval = handler->inval1;
-        }
-
-        if (unlikely((ctx->opcode & inval) != 0)) {
-            qemu_log_mask(LOG_GUEST_ERROR, "invalid bits: %08x for opcode: "
-                          "%02x - %02x - %02x - %02x (%08x) "
-                          TARGET_FMT_lx "\n", ctx->opcode & inval,
-                          opc1(ctx->opcode), opc2(ctx->opcode),
-                          opc3(ctx->opcode), opc4(ctx->opcode),
-                          ctx->opcode, ctx->cia);
-            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
-            ctx->base.is_jmp = DISAS_NORETURN;
-            return;
-        }
+    ok = decode_legacy(cpu, ctx, insn);
+    if (!ok) {
+        gen_invalid(ctx);
+        ctx->base.is_jmp = DISAS_NORETURN;
     }
-    (*(handler->handler))(ctx);
+
 #if defined(DO_PPC_STATISTICS)
     handler->count++;
 #endif
-- 
2.25.1



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

* [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception*
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (2 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 03/15] target/ppc: Split out decode_legacy Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-28 15:05   ` Richard Henderson
  2021-04-27 17:16 ` [PATCH v2 05/15] target/ppc: Tidy exception vs exit_tb Luis Pires
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, Luis Pires, lagarcia, bruno.larsen,
	matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

There are other valid settings for is_jmp besides
DISAS_NEXT and DISAS_NORETURN, so eliminating that
dichotomy from ppc_tr_translate_insn is helpful.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
---
 target/ppc/translate.c          | 65 ++++++++++++++++-----------------
 target/ppc/translate_init.c.inc | 42 ++++++++++-----------
 2 files changed, 53 insertions(+), 54 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index ebe5afe7ae..46de2dab27 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -263,6 +263,7 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error)
     tcg_temp_free_i32(t0);
     tcg_temp_free_i32(t1);
     ctx->exception = (excp);
+    ctx->base.is_jmp = DISAS_NORETURN;
 }
 
 static void gen_exception(DisasContext *ctx, uint32_t excp)
@@ -280,6 +281,7 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
     gen_helper_raise_exception(cpu_env, t0);
     tcg_temp_free_i32(t0);
     ctx->exception = (excp);
+    ctx->base.is_jmp = DISAS_NORETURN;
 }
 
 static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
@@ -292,6 +294,7 @@ static void gen_exception_nip(DisasContext *ctx, uint32_t excp,
     gen_helper_raise_exception(cpu_env, t0);
     tcg_temp_free_i32(t0);
     ctx->exception = (excp);
+    ctx->base.is_jmp = DISAS_NORETURN;
 }
 
 /*
@@ -337,6 +340,7 @@ static void gen_debug_exception(DisasContext *ctx)
     t0 = tcg_const_i32(EXCP_DEBUG);
     gen_helper_raise_exception(cpu_env, t0);
     tcg_temp_free_i32(t0);
+    ctx->base.is_jmp = DISAS_NORETURN;
 }
 
 static inline void gen_inval_exception(DisasContext *ctx, uint32_t error)
@@ -357,20 +361,18 @@ static inline void gen_hvpriv_exception(DisasContext *ctx, uint32_t error)
 }
 
 /* Stop translation */
-static inline void gen_stop_exception(DisasContext *ctx)
+static inline void gen_end_tb_exception(DisasContext *ctx, uint32_t excp)
 {
-    gen_update_nip(ctx, ctx->base.pc_next);
-    ctx->exception = POWERPC_EXCP_STOP;
+    /* No need to update nip for SYNC/BRANCH, as execution flow will change */
+    if ((excp != POWERPC_EXCP_SYNC) &&
+        (excp != POWERPC_EXCP_BRANCH))
+    {
+        gen_update_nip(ctx, ctx->base.pc_next);
+    }
+    ctx->exception = excp;
+    ctx->base.is_jmp = DISAS_NORETURN;
 }
 
-#ifndef CONFIG_USER_ONLY
-/* No need to update nip here, as execution flow will change */
-static inline void gen_sync_exception(DisasContext *ctx)
-{
-    ctx->exception = POWERPC_EXCP_SYNC;
-}
-#endif
-
 #define GEN_HANDLER(name, opc1, opc2, opc3, inval, type)                      \
 GEN_OPCODE(name, opc1, opc2, opc3, inval, type, PPC_NONE)
 
@@ -1863,7 +1865,7 @@ static void gen_darn(DisasContext *ctx)
             gen_helper_darn64(cpu_gpr[rD(ctx->opcode)]);
         }
         if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-            gen_stop_exception(ctx);
+            gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
         }
     }
 }
@@ -3159,7 +3161,7 @@ static void gen_isync(DisasContext *ctx)
         gen_check_tlb_flush(ctx, false);
     }
     tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
-    gen_stop_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
 }
 
 #define MEMOP_GET_SIZE(x)  (1 << ((x) & MO_SIZE))
@@ -3778,7 +3780,8 @@ static void gen_b(DisasContext *ctx)
 {
     target_ulong li, target;
 
-    ctx->exception = POWERPC_EXCP_BRANCH;
+    gen_end_tb_exception(ctx, POWERPC_EXCP_BRANCH);
+
     /* sign extend LI */
     li = LI(ctx->opcode);
     li = (li ^ 0x02000000) - 0x02000000;
@@ -3804,7 +3807,8 @@ static void gen_bcond(DisasContext *ctx, int type)
     uint32_t bo = BO(ctx->opcode);
     TCGLabel *l1;
     TCGv target;
-    ctx->exception = POWERPC_EXCP_BRANCH;
+
+    gen_end_tb_exception(ctx, POWERPC_EXCP_BRANCH);
 
     if (type == BCOND_LR || type == BCOND_CTR || type == BCOND_TAR) {
         target = tcg_temp_local_new();
@@ -4011,7 +4015,7 @@ static void gen_rfi(DisasContext *ctx)
     }
     gen_update_cfar(ctx, ctx->cia);
     gen_helper_rfi(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif
 }
 
@@ -4028,7 +4032,7 @@ static void gen_rfid(DisasContext *ctx)
     }
     gen_update_cfar(ctx, ctx->cia);
     gen_helper_rfid(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif
 }
 
@@ -4045,7 +4049,7 @@ static void gen_rfscv(DisasContext *ctx)
     }
     gen_update_cfar(ctx, ctx->cia);
     gen_helper_rfscv(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif
 }
 #endif
@@ -4058,7 +4062,7 @@ static void gen_hrfid(DisasContext *ctx)
     /* Restore CPU state */
     CHK_HV;
     gen_helper_hrfid(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif
 }
 #endif
@@ -4444,7 +4448,7 @@ static void gen_mtmsrd(DisasContext *ctx)
         gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
     }
     /* Must stop the translation as machine state (may have) changed */
-    gen_stop_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
 #endif /* !defined(CONFIG_USER_ONLY) */
 }
 #endif /* defined(TARGET_PPC64) */
@@ -4489,7 +4493,7 @@ static void gen_mtmsr(DisasContext *ctx)
         tcg_temp_free(msr);
     }
     /* Must stop the translation as machine state (may have) changed */
-    gen_stop_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
 #endif
 }
 
@@ -5944,7 +5948,7 @@ static void gen_rfsvc(DisasContext *ctx)
     CHK_SV;
 
     gen_helper_rfsvc(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
@@ -6324,7 +6328,7 @@ static void gen_rfci_40x(DisasContext *ctx)
     CHK_SV;
     /* Restore CPU state */
     gen_helper_40x_rfci(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
@@ -6336,7 +6340,7 @@ static void gen_rfci(DisasContext *ctx)
     CHK_SV;
     /* Restore CPU state */
     gen_helper_rfci(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
@@ -6351,7 +6355,7 @@ static void gen_rfdi(DisasContext *ctx)
     CHK_SV;
     /* Restore CPU state */
     gen_helper_rfdi(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
@@ -6364,7 +6368,7 @@ static void gen_rfmci(DisasContext *ctx)
     CHK_SV;
     /* Restore CPU state */
     gen_helper_rfmci(cpu_env);
-    gen_sync_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_SYNC);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
@@ -6626,7 +6630,7 @@ static void gen_wrtee(DisasContext *ctx)
      * Stop translation to have a chance to raise an exception if we
      * just set msr_ee to 1
      */
-    gen_stop_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
@@ -6640,7 +6644,7 @@ static void gen_wrteei(DisasContext *ctx)
     if (ctx->opcode & 0x00008000) {
         tcg_gen_ori_tl(cpu_msr, cpu_msr, (1 << MSR_EE));
         /* Stop translation to have a chance to raise an exception */
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     } else {
         tcg_gen_andi_tl(cpu_msr, cpu_msr, ~(1 << MSR_EE));
     }
@@ -8037,7 +8041,6 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
     gen_debug_exception(ctx);
-    dcbase->is_jmp = DISAS_NORETURN;
     /*
      * The address covered by the breakpoint must be included in
      * [tb->pc, tb->pc + tb->size) in order to for it to be properly
@@ -8067,7 +8070,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     ok = decode_legacy(cpu, ctx, insn);
     if (!ok) {
         gen_invalid(ctx);
-        ctx->base.is_jmp = DISAS_NORETURN;
     }
 
 #if defined(DO_PPC_STATISTICS)
@@ -8088,9 +8090,6 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
                  "temporaries\n", opc1(ctx->opcode), opc2(ctx->opcode),
                  opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
     }
-
-    ctx->base.is_jmp = ctx->exception == POWERPC_EXCP_NONE ?
-        DISAS_NEXT : DISAS_NORETURN;
 }
 
 static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index c03a7c4f52..48c713be7e 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -188,7 +188,7 @@ static void spr_read_decr(DisasContext *ctx, int gprn, int sprn)
     }
     gen_helper_load_decr(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -199,7 +199,7 @@ static void spr_write_decr(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_decr(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 #endif
@@ -214,7 +214,7 @@ static void spr_read_tbl(DisasContext *ctx, int gprn, int sprn)
     gen_helper_load_tbl(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_end();
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -226,7 +226,7 @@ static void spr_read_tbu(DisasContext *ctx, int gprn, int sprn)
     gen_helper_load_tbu(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_end();
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -251,7 +251,7 @@ static void spr_write_tbl(DisasContext *ctx, int sprn, int gprn)
     gen_helper_store_tbl(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_end();
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -263,7 +263,7 @@ static void spr_write_tbu(DisasContext *ctx, int sprn, int gprn)
     gen_helper_store_tbu(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_end();
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -288,7 +288,7 @@ static void spr_read_purr(DisasContext *ctx, int gprn, int sprn)
     }
     gen_helper_load_purr(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -299,7 +299,7 @@ static void spr_write_purr(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_purr(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -312,7 +312,7 @@ static void spr_read_hdecr(DisasContext *ctx, int gprn, int sprn)
     gen_helper_load_hdecr(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_end();
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -324,7 +324,7 @@ static void spr_write_hdecr(DisasContext *ctx, int sprn, int gprn)
     gen_helper_store_hdecr(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
         gen_io_end();
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -335,7 +335,7 @@ static void spr_read_vtb(DisasContext *ctx, int gprn, int sprn)
     }
     gen_helper_load_vtb(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -346,7 +346,7 @@ static void spr_write_vtb(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_vtb(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -357,7 +357,7 @@ static void spr_write_tbu40(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_tbu40(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -534,7 +534,7 @@ static void spr_write_hid0_601(DisasContext *ctx, int sprn, int gprn)
 {
     gen_helper_store_hid0_601(cpu_env, cpu_gpr[gprn]);
     /* Must stop the translation as endianness may have changed */
-    gen_stop_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
 }
 #endif
 
@@ -571,7 +571,7 @@ static void spr_read_40x_pit(DisasContext *ctx, int gprn, int sprn)
     }
     gen_helper_load_40x_pit(cpu_gpr[gprn], cpu_env);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -582,7 +582,7 @@ static void spr_write_40x_pit(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_40x_pit(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -594,9 +594,9 @@ static void spr_write_40x_dbcr0(DisasContext *ctx, int sprn, int gprn)
     gen_store_spr(sprn, cpu_gpr[gprn]);
     gen_helper_store_40x_dbcr0(cpu_env, cpu_gpr[gprn]);
     /* We must stop translation as we may have rebooted */
-    gen_stop_exception(ctx);
+    gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -607,7 +607,7 @@ static void spr_write_40x_sler(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_40x_sler(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -618,7 +618,7 @@ static void spr_write_booke_tcr(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_booke_tcr(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 
@@ -629,7 +629,7 @@ static void spr_write_booke_tsr(DisasContext *ctx, int sprn, int gprn)
     }
     gen_helper_store_booke_tsr(cpu_env, cpu_gpr[gprn]);
     if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
-        gen_stop_exception(ctx);
+        gen_end_tb_exception(ctx, POWERPC_EXCP_STOP);
     }
 }
 #endif
-- 
2.25.1



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

* [PATCH v2 05/15] target/ppc: Tidy exception vs exit_tb
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (3 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception* Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 06/15] target/ppc: Mark helper_raise_exception* as noreturn Luis Pires
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

We do not need to emit an exit_tb after an exception,
as the latter will exit via longjmp.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 46de2dab27..b18ad8ec2c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -3744,8 +3744,9 @@ static void gen_lookup_and_goto_ptr(DisasContext *ctx)
         } else if (sse & (CPU_SINGLE_STEP | CPU_BRANCH_STEP)) {
             uint32_t excp = gen_prep_dbgex(ctx);
             gen_exception(ctx, excp);
+        } else {
+            tcg_gen_exit_tb(NULL, 0);
         }
-        tcg_gen_exit_tb(NULL, 0);
     } else {
         tcg_gen_lookup_and_goto_ptr();
     }
@@ -8101,9 +8102,10 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
     } else if (ctx->exception != POWERPC_EXCP_BRANCH) {
         if (unlikely(ctx->base.singlestep_enabled)) {
             gen_debug_exception(ctx);
+        } else {
+            /* Generate the return instruction */
+            tcg_gen_exit_tb(NULL, 0);
         }
-        /* Generate the return instruction */
-        tcg_gen_exit_tb(NULL, 0);
     }
 }
 
-- 
2.25.1



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

* [PATCH v2 06/15] target/ppc: Mark helper_raise_exception* as noreturn
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (4 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 05/15] target/ppc: Tidy exception vs exit_tb Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 07/15] target/ppc: Use translator_loop_temp_check Luis Pires
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/helper.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6a4dccf70c..af5b3586d1 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -1,5 +1,5 @@
-DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, void, env, i32, i32)
-DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, void, env, i32)
+DEF_HELPER_FLAGS_3(raise_exception_err, TCG_CALL_NO_WG, noreturn, env, i32, i32)
+DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, i32)
 DEF_HELPER_FLAGS_4(tw, TCG_CALL_NO_WG, void, env, tl, tl, i32)
 #if defined(TARGET_PPC64)
 DEF_HELPER_FLAGS_4(td, TCG_CALL_NO_WG, void, env, tl, tl, i32)
-- 
2.25.1



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

* [PATCH v2 07/15] target/ppc: Use translator_loop_temp_check
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (5 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 06/15] target/ppc: Mark helper_raise_exception* as noreturn Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 08/15] target/ppc: Add infrastructure for prefixed insns Luis Pires
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

The special logging is unnecessary.  It will have been done
immediately before in the log file.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b18ad8ec2c..dd34f22704 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -8086,11 +8086,7 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
         gen_exception_nip(ctx, excp, ctx->base.pc_next);
     }
 
-    if (tcg_check_temp_count()) {
-        qemu_log("Opcode %02x %02x %02x %02x (%08x) leaked "
-                 "temporaries\n", opc1(ctx->opcode), opc2(ctx->opcode),
-                 opc3(ctx->opcode), opc4(ctx->opcode), ctx->opcode);
-    }
+    translator_loop_temp_check(&ctx->base);
 }
 
 static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
-- 
2.25.1



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

* [PATCH v2 08/15] target/ppc: Add infrastructure for prefixed insns
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (6 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 07/15] target/ppc: Use translator_loop_temp_check Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Luis Pires
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, Luis Pires, lagarcia, bruno.larsen,
	matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
---
 target/ppc/cpu.h                           |  1 +
 target/ppc/insn32.decode                   | 18 +++++++++
 target/ppc/insn64.decode                   | 18 +++++++++
 target/ppc/meson.build                     |  9 +++++
 target/ppc/translate.c                     | 45 +++++++++++++++++-----
 target/ppc/translate/fixedpoint-impl.c.inc | 18 +++++++++
 6 files changed, 99 insertions(+), 10 deletions(-)
 create mode 100644 target/ppc/insn32.decode
 create mode 100644 target/ppc/insn64.decode
 create mode 100644 target/ppc/translate/fixedpoint-impl.c.inc

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..9bb2805a22 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -148,6 +148,7 @@ enum {
     POWERPC_EXCP_ALIGN_PROT    = 0x04,  /* Access cross protection boundary  */
     POWERPC_EXCP_ALIGN_BAT     = 0x05,  /* Access cross a BAT/seg boundary   */
     POWERPC_EXCP_ALIGN_CACHE   = 0x06,  /* Impossible dcbz access            */
+    POWERPC_EXCP_ALIGN_INSN    = 0x07,  /* Pref. insn x-ing 64-byte boundary */
     /* Exception subtypes for POWERPC_EXCP_PROGRAM                           */
     /* FP exceptions                                                         */
     POWERPC_EXCP_FP            = 0x10,
diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
new file mode 100644
index 0000000000..b175441209
--- /dev/null
+++ b/target/ppc/insn32.decode
@@ -0,0 +1,18 @@
+#
+# Power ISA decode for 32-bit insns (opcode space 0)
+#
+# Copyright (c) 2021 Luis Pires <luis.pires@eldorado.org.br>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see <http://www.gnu.org/licenses/>.
+#
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
new file mode 100644
index 0000000000..9fc45d0614
--- /dev/null
+++ b/target/ppc/insn64.decode
@@ -0,0 +1,18 @@
+#
+# Power ISA decode for 64-bit prefixed insns (opcode space 0 and 1)
+#
+# Copyright (c) 2021 Luis Pires <luis.pires@eldorado.org.br>
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, see <http://www.gnu.org/licenses/>.
+#
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index bbfef90e08..e604e56c6a 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -15,6 +15,15 @@ ppc_ss.add(files(
 
 ppc_ss.add(libdecnumber)
 
+gen = [
+  decodetree.process('insn32.decode',
+                     extra_args: '--static-decode=decode_insn32'),
+  decodetree.process('insn64.decode',
+                     extra_args: ['--static-decode=decode_insn64',
+                                  '--insnwidth=64']),
+]
+ppc_ss.add(gen)
+
 ppc_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c'))
 ppc_ss.add(when: 'CONFIG_USER_ONLY', if_true: files('user_only_helper.c'))
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index dd34f22704..83f08950b4 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6906,6 +6906,11 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
     tcg_gen_st_i64(src, cpu_env, avr64_offset(regno, high));
 }
 
+#include "decode-insn64.c.inc"
+#include "decode-insn32.c.inc"
+
+#include "translate/fixedpoint-impl.c.inc"
+
 #include "translate/fp-impl.c.inc"
 
 #include "translate/vmx-impl.c.inc"
@@ -7941,7 +7946,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPUPPCState *env = cs->env_ptr;
-    int bound;
 
     ctx->exception = POWERPC_EXCP_NONE;
     ctx->spr_cb = env->spr_cb;
@@ -8022,9 +8026,6 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     /* Single step trace mode */
     msr_se = 1;
 #endif
-
-    bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
-    ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
 }
 
 static void ppc_tr_tb_start(DisasContextBase *db, CPUState *cs)
@@ -8052,11 +8053,18 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
     return true;
 }
 
+static bool is_prefix_insn(DisasContext *ctx, uint32_t insn)
+{
+    /* TODO: Check ctx->insns_flags* for whether prefixes are supported. */
+    return opc1(insn) == 1;
+}
+
 static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = cs->env_ptr;
+    target_ulong pc;
     uint32_t insn;
     bool ok;
 
@@ -8064,11 +8072,26 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     LOG_DISAS("nip=" TARGET_FMT_lx " super=%d ir=%d\n",
               ctx->base.pc_next, ctx->mem_idx, (int)msr_ir);
 
-    ctx->cia = ctx->base.pc_next;
-    insn = translator_ldl_swap(env, ctx->base.pc_next, need_byteswap(ctx));
-    ctx->base.pc_next += 4;
+    ctx->cia = pc = ctx->base.pc_next;
+    insn = translator_ldl_swap(env, pc, need_byteswap(ctx));
+    ctx->base.pc_next = pc += 4;
 
-    ok = decode_legacy(cpu, ctx, insn);
+    if (!is_prefix_insn(ctx, insn)) {
+        ok = (decode_insn32(ctx, insn) ||
+              decode_legacy(cpu, ctx, insn));
+    } else if ((pc & 63) == 0) {
+        /*
+         * Power v3.1, section 1.9 Exceptions:
+         * attempt to execute a prefixed instruction that crosses a
+         * 64-byte address boundary (system alignment error).
+         */
+        gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_INSN);
+        ok = true;
+    } else {
+        uint32_t insn2 = translator_ldl_swap(env, pc, need_byteswap(ctx));
+        ctx->base.pc_next = pc += 4;
+        ok = decode_insn64(ctx, deposit64(insn2, 32, 32, insn));
+    }
     if (!ok) {
         gen_invalid(ctx);
     }
@@ -8078,12 +8101,14 @@ static void ppc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 #endif
     /* Check trace mode exceptions */
     if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP &&
-                 (ctx->base.pc_next <= 0x100 || ctx->base.pc_next > 0xF00) &&
+                 (pc <= 0x100 || pc > 0xF00) &&
                  ctx->exception != POWERPC_SYSCALL &&
                  ctx->exception != POWERPC_EXCP_TRAP &&
                  ctx->exception != POWERPC_EXCP_BRANCH)) {
         uint32_t excp = gen_prep_dbgex(ctx);
-        gen_exception_nip(ctx, excp, ctx->base.pc_next);
+        gen_exception_nip(ctx, excp, pc);
+    } else if (ctx->base.is_jmp == DISAS_NEXT && !(pc & ~TARGET_PAGE_MASK)) {
+        ctx->base.is_jmp = DISAS_TOO_MANY;
     }
 
     translator_loop_temp_check(&ctx->base);
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
new file mode 100644
index 0000000000..b740083605
--- /dev/null
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -0,0 +1,18 @@
+/*
+ * Power ISA decode for Fixed-Point Facility instructions
+ *
+ * Copyright (c) 2021 Luis Pires <luis.pires@eldorado.org.br>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
-- 
2.25.1



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

* [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (7 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 08/15] target/ppc: Add infrastructure for prefixed insns Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-28 14:10   ` Matheus K. Ferst
  2021-04-27 17:16 ` [PATCH v2 10/15] target/ppc: Implement PNOP Luis Pires
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/insn32.decode                   |  8 +++++
 target/ppc/insn64.decode                   | 14 ++++++++
 target/ppc/translate.c                     | 29 ---------------
 target/ppc/translate/fixedpoint-impl.c.inc | 42 ++++++++++++++++++++++
 4 files changed, 64 insertions(+), 29 deletions(-)

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index b175441209..878d2f2f66 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -16,3 +16,11 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library; if not, see <http://www.gnu.org/licenses/>.
 #
+
+&D              rt ra si
+@D              ...... rt:5 ra:5 si:s16                 &D
+
+### Fixed-Point Arithmetic Instructions
+
+ADDI            001110 ..... ..... ................     @D
+ADDIS           001111 ..... ..... ................     @D
diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 9fc45d0614..68ed2cbff8 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -16,3 +16,17 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library; if not, see <http://www.gnu.org/licenses/>.
 #
+
+# Format MLS:D and 8LS:D
+&PLS_D          rt ra si r
+
+%pls_si         32:s18 0:16
+
+@PLS_D          ...... .. ... r:1 .. .................. \
+                ...... rt:5 ra:5 ................       \
+                &PLS_D si=%pls_si
+
+### Fixed-Point Arithmetic Instructions
+
+PADDI           000001 10 0--.-- ..................     \
+                001110 ..... ..... ................     @PLS_D
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 83f08950b4..6edde6a53d 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -937,19 +937,6 @@ GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0);
 /* addze  addze.  addzeo  addzeo.*/
 GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0)
 GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1)
-/* addi */
-static void gen_addi(DisasContext *ctx)
-{
-    target_long simm = SIMM(ctx->opcode);
-
-    if (rA(ctx->opcode) == 0) {
-        /* li case */
-        tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm);
-    } else {
-        tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)],
-                        cpu_gpr[rA(ctx->opcode)], simm);
-    }
-}
 /* addic  addic.*/
 static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0)
 {
@@ -969,20 +956,6 @@ static void gen_addic_(DisasContext *ctx)
     gen_op_addic(ctx, 1);
 }
 
-/* addis */
-static void gen_addis(DisasContext *ctx)
-{
-    target_long simm = SIMM(ctx->opcode);
-
-    if (rA(ctx->opcode) == 0) {
-        /* lis case */
-        tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm << 16);
-    } else {
-        tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)],
-                        cpu_gpr[rA(ctx->opcode)], simm << 16);
-    }
-}
-
 /* addpcis */
 static void gen_addpcis(DisasContext *ctx)
 {
@@ -7034,10 +7007,8 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x00600000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x00000001, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x00400001, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x00000001, PPC_ISEL),
-GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
-GEN_HANDLER(addis, 0x0F, 0xFF, 0xFF, 0x00000000, PPC_INTEGER),
 GEN_HANDLER_E(addpcis, 0x13, 0x2, 0xFF, 0x00000000, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(mulhw, 0x1F, 0x0B, 0x02, 0x00000400, PPC_INTEGER),
 GEN_HANDLER(mulhwu, 0x1F, 0x0B, 0x00, 0x00000400, PPC_INTEGER),
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index b740083605..76e1832297 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -16,3 +16,45 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
+
+/*
+ * Incorporate CIA into the constant when R=1.
+ * Validate that when R=1, RA=0.
+ */
+static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a)
+{
+    if (a->r) {
+        a->si += ctx->cia;
+        return a->ra == 0;
+    }
+    return true;
+}
+
+static bool trans_ADDI(DisasContext *ctx, arg_D *a)
+{
+    if (a->ra) {
+        tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
+    } else {
+        tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
+    }
+    return true;
+}
+
+static bool trans_ADDIS(DisasContext *ctx, arg_D *a)
+{
+    a->si <<= 16;
+    return trans_ADDI(ctx, a);
+}
+
+static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
+{
+    if (!resolve_PLS_D(ctx, a)) {
+        return false;
+    }
+    if (a->ra) {
+        tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
+    } else {
+        tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
+    }
+    return true;
+}
-- 
2.25.1



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

* [PATCH v2 10/15] target/ppc: Implement PNOP
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (8 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree Luis Pires
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/insn64.decode                   |  5 +++++
 target/ppc/translate/fixedpoint-impl.c.inc | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 68ed2cbff8..9bef32a845 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -30,3 +30,8 @@
 
 PADDI           000001 10 0--.-- ..................     \
                 001110 ..... ..... ................     @PLS_D
+
+### Prefixed No-operation Instruction
+
+PNOP            000001 11 0000-- 000000000000000000     \
+                --------------------------------
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index 76e1832297..7d80e3c002 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -58,3 +58,15 @@ static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
     }
     return true;
 }
+
+static bool trans_PNOP(DisasContext *ctx, arg_PNOP *a)
+{
+    /*
+     * TODO: diagnose the set of patterns that are illegal:
+     * branches, rfebb, sync other than isync, or a
+     * service processor attention.
+     * The Engineering Note allows us to either diagnose
+     * these as illegal, or treat them all as no-op.
+     */
+    return true;
+}
-- 
2.25.1



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

* [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (9 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 10/15] target/ppc: Implement PNOP Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-28 13:31   ` Matheus K. Ferst
  2021-04-27 17:16 ` [PATCH v2 12/15] target/ppc: Implement prefixed integer load instructions Luis Pires
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

These are all connected by macros in the legacy decoding.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/insn32.decode                   |  37 ++++
 target/ppc/translate.c                     | 136 ++-------------
 target/ppc/translate/fixedpoint-impl.c.inc | 188 +++++++++++++++++++++
 3 files changed, 238 insertions(+), 123 deletions(-)

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index 878d2f2f66..bf39ce5c15 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -20,6 +20,43 @@
 &D              rt ra si
 @D              ...... rt:5 ra:5 si:s16                 &D
 
+%ds_si          2:s14  !function=times_4
+@DS             ...... rt:5 ra:5 .............. ..      &D si=%ds_si
+
+&X              rt ra rb
+@X              ...... rt:5 ra:5 rb:5 .......... .      &X
+
+### Fixed-Point Load Instructions
+
+LBZ             100010 ..... ..... ................     @D
+LBZU            100011 ..... ..... ................     @D
+LBZX            011111 ..... ..... ..... 0001010111 -   @X
+LBZUX           011111 ..... ..... ..... 0001110111 -   @X
+
+LHZ             101000 ..... ..... ................     @D
+LHZU            101001 ..... ..... ................     @D
+LHZX            011111 ..... ..... ..... 0100010111 -   @X
+LHZUX           011111 ..... ..... ..... 0100110111 -   @X
+
+LHA             101010 ..... ..... ................     @D
+LHAU            101011 ..... ..... ................     @D
+LHAX            011111 ..... ..... ..... 0101010111 -   @X
+LHAXU           011111 ..... ..... ..... 0101110111 -   @X
+
+LWZ             100000 ..... ..... ................     @D
+LWZU            100001 ..... ..... ................     @D
+LWZX            011111 ..... ..... ..... 0000010111 -   @X
+LWZUX           011111 ..... ..... ..... 0000110111 -   @X
+
+LWA             111010 ..... ..... ..............10     @DS
+LWAX            011111 ..... ..... ..... 0101010101 -   @X
+LWAUX           011111 ..... ..... ..... 0101110101 -   @X
+
+LD              111010 ..... ..... ..............00     @DS
+LDU             111010 ..... ..... ..............01     @DS
+LDX             011111 ..... ..... ..... 0000010101 -   @X
+LDUX            011111 ..... ..... ..... 0000110101 -   @X
+
 ### Fixed-Point Arithmetic Instructions
 
 ADDI            001110 ..... ..... ................     @D
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 6edde6a53d..a1f0e59afd 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2505,54 +2505,6 @@ GEN_QEMU_STORE_64(st64, DEF_MEMOP(MO_Q))
 GEN_QEMU_STORE_64(st64r, BSWAP_MEMOP(MO_Q))
 #endif
 
-#define GEN_LD(name, ldop, opc, type)                                         \
-static void glue(gen_, name)(DisasContext *ctx)                               \
-{                                                                             \
-    TCGv EA;                                                                  \
-    gen_set_access_type(ctx, ACCESS_INT);                                     \
-    EA = tcg_temp_new();                                                      \
-    gen_addr_imm_index(ctx, EA, 0);                                           \
-    gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA);                       \
-    tcg_temp_free(EA);                                                        \
-}
-
-#define GEN_LDU(name, ldop, opc, type)                                        \
-static void glue(gen_, name##u)(DisasContext *ctx)                            \
-{                                                                             \
-    TCGv EA;                                                                  \
-    if (unlikely(rA(ctx->opcode) == 0 ||                                      \
-                 rA(ctx->opcode) == rD(ctx->opcode))) {                       \
-        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);                   \
-        return;                                                               \
-    }                                                                         \
-    gen_set_access_type(ctx, ACCESS_INT);                                     \
-    EA = tcg_temp_new();                                                      \
-    if (type == PPC_64B)                                                      \
-        gen_addr_imm_index(ctx, EA, 0x03);                                    \
-    else                                                                      \
-        gen_addr_imm_index(ctx, EA, 0);                                       \
-    gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA);                       \
-    tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);                             \
-    tcg_temp_free(EA);                                                        \
-}
-
-#define GEN_LDUX(name, ldop, opc2, opc3, type)                                \
-static void glue(gen_, name##ux)(DisasContext *ctx)                           \
-{                                                                             \
-    TCGv EA;                                                                  \
-    if (unlikely(rA(ctx->opcode) == 0 ||                                      \
-                 rA(ctx->opcode) == rD(ctx->opcode))) {                       \
-        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);                   \
-        return;                                                               \
-    }                                                                         \
-    gen_set_access_type(ctx, ACCESS_INT);                                     \
-    EA = tcg_temp_new();                                                      \
-    gen_addr_reg_index(ctx, EA);                                              \
-    gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA);                       \
-    tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);                             \
-    tcg_temp_free(EA);                                                        \
-}
-
 #define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk)                   \
 static void glue(gen_, name##x)(DisasContext *ctx)                            \
 {                                                                             \
@@ -2571,21 +2523,6 @@ static void glue(gen_, name##x)(DisasContext *ctx)                            \
 #define GEN_LDX_HVRM(name, ldop, opc2, opc3, type)                            \
     GEN_LDX_E(name, ldop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
 
-#define GEN_LDS(name, ldop, op, type)                                         \
-GEN_LD(name, ldop, op | 0x20, type);                                          \
-GEN_LDU(name, ldop, op | 0x21, type);                                         \
-GEN_LDUX(name, ldop, 0x17, op | 0x01, type);                                  \
-GEN_LDX(name, ldop, 0x17, op | 0x00, type)
-
-/* lbz lbzu lbzux lbzx */
-GEN_LDS(lbz, ld8u, 0x02, PPC_INTEGER);
-/* lha lhau lhaux lhax */
-GEN_LDS(lha, ld16s, 0x0A, PPC_INTEGER);
-/* lhz lhzu lhzux lhzx */
-GEN_LDS(lhz, ld16u, 0x08, PPC_INTEGER);
-/* lwz lwzu lwzux lwzx */
-GEN_LDS(lwz, ld32u, 0x00, PPC_INTEGER);
-
 #define GEN_LDEPX(name, ldop, opc2, opc3)                                     \
 static void glue(gen_, name##epx)(DisasContext *ctx)                          \
 {                                                                             \
@@ -2606,47 +2543,12 @@ GEN_LDEPX(ld, DEF_MEMOP(MO_Q), 0x1D, 0x00)
 #endif
 
 #if defined(TARGET_PPC64)
-/* lwaux */
-GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B);
-/* lwax */
-GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B);
-/* ldux */
-GEN_LDUX(ld, ld64_i64, 0x15, 0x01, PPC_64B);
-/* ldx */
-GEN_LDX(ld, ld64_i64, 0x15, 0x00, PPC_64B);
-
 /* CI load/store variants */
 GEN_LDX_HVRM(ldcix, ld64_i64, 0x15, 0x1b, PPC_CILDST)
 GEN_LDX_HVRM(lwzcix, ld32u, 0x15, 0x15, PPC_CILDST)
 GEN_LDX_HVRM(lhzcix, ld16u, 0x15, 0x19, PPC_CILDST)
 GEN_LDX_HVRM(lbzcix, ld8u, 0x15, 0x1a, PPC_CILDST)
 
-static void gen_ld(DisasContext *ctx)
-{
-    TCGv EA;
-    if (Rc(ctx->opcode)) {
-        if (unlikely(rA(ctx->opcode) == 0 ||
-                     rA(ctx->opcode) == rD(ctx->opcode))) {
-            gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
-            return;
-        }
-    }
-    gen_set_access_type(ctx, ACCESS_INT);
-    EA = tcg_temp_new();
-    gen_addr_imm_index(ctx, EA, 0x03);
-    if (ctx->opcode & 0x02) {
-        /* lwa (lwau is undefined) */
-        gen_qemu_ld32s(ctx, cpu_gpr[rD(ctx->opcode)], EA);
-    } else {
-        /* ld - ldu */
-        gen_qemu_ld64_i64(ctx, cpu_gpr[rD(ctx->opcode)], EA);
-    }
-    if (Rc(ctx->opcode)) {
-        tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);
-    }
-    tcg_temp_free(EA);
-}
-
 /* lq */
 static void gen_lq(DisasContext *ctx)
 {
@@ -6879,6 +6781,18 @@ static inline void set_avr64(int regno, TCGv_i64 src, bool high)
     tcg_gen_st_i64(src, cpu_env, avr64_offset(regno, high));
 }
 
+static inline int times_4(DisasContext *ctx, int x)
+{
+    return x * 4;
+}
+
+#define REQUIRE_INSNS_FLAGS(CTX, NAME) \
+    do {                                                \
+        if (((CTX)->insns_flags & PPC_##NAME) == 0) {   \
+            return false;                               \
+        }                                               \
+    } while (0)
+
 #include "decode-insn64.c.inc"
 #include "decode-insn32.c.inc"
 
@@ -7064,7 +6978,6 @@ GEN_HANDLER2_E(extswsli1, "extswsli", 0x1F, 0x1B, 0x1B, 0x00000000,
                PPC_NONE, PPC2_ISA300),
 #endif
 #if defined(TARGET_PPC64)
-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),
 #endif
@@ -7430,34 +7343,11 @@ GEN_PPC64_R2(rldcr, 0x1E, 0x09),
 GEN_PPC64_R4(rldimi, 0x1E, 0x06),
 #endif
 
-#undef GEN_LD
-#undef GEN_LDU
-#undef GEN_LDUX
 #undef GEN_LDX_E
-#undef GEN_LDS
-#define GEN_LD(name, ldop, opc, type)                                         \
-GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_LDU(name, ldop, opc, type)                                        \
-GEN_HANDLER(name##u, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_LDUX(name, ldop, opc2, opc3, type)                                \
-GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
 #define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk)                   \
 GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
-#define GEN_LDS(name, ldop, op, type)                                         \
-GEN_LD(name, ldop, op | 0x20, type)                                           \
-GEN_LDU(name, ldop, op | 0x21, type)                                          \
-GEN_LDUX(name, ldop, 0x17, op | 0x01, type)                                   \
-GEN_LDX(name, ldop, 0x17, op | 0x00, type)
-
-GEN_LDS(lbz, ld8u, 0x02, PPC_INTEGER)
-GEN_LDS(lha, ld16s, 0x0A, PPC_INTEGER)
-GEN_LDS(lhz, ld16u, 0x08, PPC_INTEGER)
-GEN_LDS(lwz, ld32u, 0x00, PPC_INTEGER)
+
 #if defined(TARGET_PPC64)
-GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B)
-GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B)
-GEN_LDUX(ld, ld64_i64, 0x15, 0x01, PPC_64B)
-GEN_LDX(ld, ld64_i64, 0x15, 0x00, PPC_64B)
 GEN_LDX_E(ldbr, ld64ur_i64, 0x14, 0x10, PPC_NONE, PPC2_DBRX, CHK_NONE)
 
 /* HV/P7 and later only */
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index 7d80e3c002..e15e379931 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -30,6 +30,194 @@ static bool resolve_PLS_D(DisasContext *ctx, arg_PLS_D *a)
     return true;
 }
 
+static bool do_ldst_D(DisasContext *ctx, arg_D *a, bool update,
+                      bool store, MemOp mop)
+{
+    TCGv ea;
+
+    if (update && (a->ra == 0 || (!store && a->ra == a->rt))) {
+        return false;
+    }
+    gen_set_access_type(ctx, ACCESS_INT);
+
+    ea = tcg_temp_new();
+    if (a->ra) {
+        tcg_gen_addi_tl(ea, cpu_gpr[a->ra], a->si);
+    } else {
+        tcg_gen_movi_tl(ea, a->si);
+    }
+    if (NARROW_MODE(ctx)) {
+        tcg_gen_ext32u_tl(ea, ea);
+    }
+    mop ^= ctx->default_tcg_memop_mask;
+    if (store) {
+        tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+    } else {
+        tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+    }
+    if (update) {
+        tcg_gen_mov_tl(cpu_gpr[a->ra], ea);
+    }
+    tcg_temp_free(ea);
+
+    return true;
+}
+
+static bool trans_LBZ(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, false, MO_UB);
+}
+
+static bool trans_LBZU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, false, MO_UB);
+}
+
+static bool trans_LHZ(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, false, MO_UW);
+}
+
+static bool trans_LHZU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, false, MO_UW);
+}
+
+static bool trans_LHA(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, false, MO_SW);
+}
+
+static bool trans_LHAU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, false, MO_SW);
+}
+
+static bool trans_LWZ(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, false, MO_UL);
+}
+
+static bool trans_LWZU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, false, MO_UL);
+}
+
+static bool trans_LWA(DisasContext *ctx, arg_D *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_D(ctx, a, false, false, MO_SL);
+}
+
+static bool trans_LD(DisasContext *ctx, arg_D *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_D(ctx, a, false, false, MO_Q);
+}
+
+static bool trans_LDU(DisasContext *ctx, arg_D *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_D(ctx, a, true, false, MO_Q);
+}
+
+static bool do_ldst_X(DisasContext *ctx, arg_X *a, bool update,
+                      bool store, MemOp mop)
+{
+    TCGv ea;
+
+    if (update && (a->ra == 0 || (!store && a->ra == a->rt))) {
+        return false;
+    }
+    gen_set_access_type(ctx, ACCESS_INT);
+
+    ea = tcg_temp_new();
+    if (a->ra) {
+        tcg_gen_add_tl(ea, cpu_gpr[a->ra], cpu_gpr[a->rb]);
+    } else {
+        tcg_gen_mov_tl(ea, cpu_gpr[a->rb]);
+    }
+    if (NARROW_MODE(ctx)) {
+        tcg_gen_ext32u_tl(ea, ea);
+    }
+    mop ^= ctx->default_tcg_memop_mask;
+    if (store) {
+        tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+    } else {
+        tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+    }
+    if (update) {
+        tcg_gen_mov_tl(cpu_gpr[a->ra], ea);
+    }
+    tcg_temp_free(ea);
+
+    return true;
+}
+
+static bool trans_LBZX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, false, MO_UB);
+}
+
+static bool trans_LBZUX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, false, MO_UB);
+}
+
+static bool trans_LHZX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, false, MO_UW);
+}
+
+static bool trans_LHZUX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, false, MO_UW);
+}
+
+static bool trans_LHAX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, false, MO_SW);
+}
+
+static bool trans_LHAXU(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, false, MO_SW);
+}
+
+static bool trans_LWZX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, false, MO_UL);
+}
+
+static bool trans_LWZUX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, false, MO_UL);
+}
+
+static bool trans_LWAX(DisasContext *ctx, arg_X *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_X(ctx, a, false, false, MO_SL);
+}
+
+static bool trans_LWAUX(DisasContext *ctx, arg_X *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_X(ctx, a, true, false, MO_SL);
+}
+
+static bool trans_LDX(DisasContext *ctx, arg_X *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_X(ctx, a, false, false, MO_Q);
+}
+
+static bool trans_LDUX(DisasContext *ctx, arg_X *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_X(ctx, a, true, false, MO_Q);
+}
+
 static bool trans_ADDI(DisasContext *ctx, arg_D *a)
 {
     if (a->ra) {
-- 
2.25.1



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

* [PATCH v2 12/15] target/ppc: Implement prefixed integer load instructions
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (10 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 13/15] target/ppc: Move D/DS/X-form integer stores to decodetree Luis Pires
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/insn64.decode                   | 15 ++++++
 target/ppc/translate/fixedpoint-impl.c.inc | 60 ++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 9bef32a845..2e08d89e62 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -26,6 +26,21 @@
                 ...... rt:5 ra:5 ................       \
                 &PLS_D si=%pls_si
 
+### Fixed-Point Load Instructions
+
+PLBZ            000001 10 0--.-- .................. \
+                100010 ..... ..... ................     @PLS_D
+PLHZ            000001 10 0--.-- .................. \
+                101000 ..... ..... ................     @PLS_D
+PLHA            000001 10 0--.-- .................. \
+                101010 ..... ..... ................     @PLS_D
+PLWZ            000001 10 0--.-- .................. \
+                100000 ..... ..... ................     @PLS_D
+PLWA            000001 00 0--.-- .................. \
+                101001 ..... ..... ................     @PLS_D
+PLD             000001 00 0--.-- .................. \
+                111001 ..... ..... ................     @PLS_D
+
 ### Fixed-Point Arithmetic Instructions
 
 PADDI           000001 10 0--.-- ..................     \
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index e15e379931..80f849fc4a 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -218,6 +218,66 @@ static bool trans_LDUX(DisasContext *ctx, arg_X *a)
     return do_ldst_X(ctx, a, true, false, MO_Q);
 }
 
+static bool do_ldst_PLS_D(DisasContext *ctx, arg_PLS_D *a,
+                          bool store, MemOp mop)
+{
+    TCGv ea;
+
+    if (!resolve_PLS_D(ctx, a)) {
+        return false;
+    }
+    gen_set_access_type(ctx, ACCESS_INT);
+
+    ea = tcg_temp_new();
+    if (a->ra) {
+        tcg_gen_addi_tl(ea, cpu_gpr[a->ra], a->si);
+    } else {
+        tcg_gen_movi_tl(ea, a->si);
+    }
+    if (NARROW_MODE(ctx)) {
+        tcg_gen_ext32u_tl(ea, ea);
+    }
+    mop ^= ctx->default_tcg_memop_mask;
+    if (store) {
+        tcg_gen_qemu_st_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+    } else {
+        tcg_gen_qemu_ld_tl(cpu_gpr[a->rt], ea, ctx->mem_idx, mop);
+    }
+    tcg_temp_free(ea);
+
+    return true;
+}
+
+static bool trans_PLBZ(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, false, MO_UB);
+}
+
+static bool trans_PLHZ(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, false, MO_UW);
+}
+
+static bool trans_PLHA(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, false, MO_SW);
+}
+
+static bool trans_PLWZ(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, false, MO_UL);
+}
+
+static bool trans_PLWA(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, false, MO_SL);
+}
+
+static bool trans_PLD(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, false, MO_Q);
+}
+
 static bool trans_ADDI(DisasContext *ctx, arg_D *a)
 {
     if (a->ra) {
-- 
2.25.1



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

* [PATCH v2 13/15] target/ppc: Move D/DS/X-form integer stores to decodetree
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (11 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 12/15] target/ppc: Implement prefixed integer load instructions Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 14/15] target/ppc: Implement prefixed integer store instructions Luis Pires
  2021-04-27 17:16 ` [PATCH v2 15/15] target/ppc: Check cpu flags for prefixed insn support Luis Pires
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

These are all connected by macros in the legacy decoding.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/insn32.decode                   | 22 ++++++
 target/ppc/translate.c                     | 85 +---------------------
 target/ppc/translate/fixedpoint-impl.c.inc | 84 +++++++++++++++++++++
 3 files changed, 109 insertions(+), 82 deletions(-)

diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
index bf39ce5c15..df92f11558 100644
--- a/target/ppc/insn32.decode
+++ b/target/ppc/insn32.decode
@@ -57,6 +57,28 @@ LDU             111010 ..... ..... ..............01     @DS
 LDX             011111 ..... ..... ..... 0000010101 -   @X
 LDUX            011111 ..... ..... ..... 0000110101 -   @X
 
+### Fixed-Point Store Instructions
+
+STB             100110 ..... ..... ................     @D
+STBU            100111 ..... ..... ................     @D
+STBX            011111 ..... ..... ..... 0011010111 -   @X
+STBUX           011111 ..... ..... ..... 0011110111 -   @X
+
+STH             101100 ..... ..... ................     @D
+STHU            101101 ..... ..... ................     @D
+STHX            011111 ..... ..... ..... 0110110111 -   @X
+STHUX           011111 ..... ..... ..... 0110010111 -   @X
+
+STW             100100 ..... ..... ................     @D
+STWU            100101 ..... ..... ................     @D
+STWX            011111 ..... ..... ..... 0010010111 -   @X
+STWUX           011111 ..... ..... ..... 0010110111 -   @X
+
+STD             111110 ..... ..... ..............00     @DS
+STDU            111110 ..... ..... ..............01     @DS
+STDX            011111 ..... ..... ..... 0010010101 -   @X
+STDUX           011111 ..... ..... ..... 0010110101 -   @X
+
 ### Fixed-Point Arithmetic Instructions
 
 ADDI            001110 ..... ..... ................     @D
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index a1f0e59afd..7422ea4e13 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2481,7 +2481,9 @@ static void glue(gen_qemu_, stop)(DisasContext *ctx,                    \
     tcg_gen_qemu_st_tl(val, addr, ctx->mem_idx, op);                    \
 }
 
+#if defined(TARGET_PPC64) || !defined(CONFIG_USER_ONLY)
 GEN_QEMU_STORE_TL(st8,  DEF_MEMOP(MO_UB))
+#endif
 GEN_QEMU_STORE_TL(st16, DEF_MEMOP(MO_UW))
 GEN_QEMU_STORE_TL(st32, DEF_MEMOP(MO_UL))
 
@@ -2614,52 +2616,6 @@ static void gen_lq(DisasContext *ctx)
 #endif
 
 /***                              Integer store                            ***/
-#define GEN_ST(name, stop, opc, type)                                         \
-static void glue(gen_, name)(DisasContext *ctx)                               \
-{                                                                             \
-    TCGv EA;                                                                  \
-    gen_set_access_type(ctx, ACCESS_INT);                                     \
-    EA = tcg_temp_new();                                                      \
-    gen_addr_imm_index(ctx, EA, 0);                                           \
-    gen_qemu_##stop(ctx, cpu_gpr[rS(ctx->opcode)], EA);                       \
-    tcg_temp_free(EA);                                                        \
-}
-
-#define GEN_STU(name, stop, opc, type)                                        \
-static void glue(gen_, stop##u)(DisasContext *ctx)                            \
-{                                                                             \
-    TCGv EA;                                                                  \
-    if (unlikely(rA(ctx->opcode) == 0)) {                                     \
-        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);                   \
-        return;                                                               \
-    }                                                                         \
-    gen_set_access_type(ctx, ACCESS_INT);                                     \
-    EA = tcg_temp_new();                                                      \
-    if (type == PPC_64B)                                                      \
-        gen_addr_imm_index(ctx, EA, 0x03);                                    \
-    else                                                                      \
-        gen_addr_imm_index(ctx, EA, 0);                                       \
-    gen_qemu_##stop(ctx, cpu_gpr[rS(ctx->opcode)], EA);                       \
-    tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);                             \
-    tcg_temp_free(EA);                                                        \
-}
-
-#define GEN_STUX(name, stop, opc2, opc3, type)                                \
-static void glue(gen_, name##ux)(DisasContext *ctx)                           \
-{                                                                             \
-    TCGv EA;                                                                  \
-    if (unlikely(rA(ctx->opcode) == 0)) {                                     \
-        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);                   \
-        return;                                                               \
-    }                                                                         \
-    gen_set_access_type(ctx, ACCESS_INT);                                     \
-    EA = tcg_temp_new();                                                      \
-    gen_addr_reg_index(ctx, EA);                                              \
-    gen_qemu_##stop(ctx, cpu_gpr[rS(ctx->opcode)], EA);                       \
-    tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], EA);                             \
-    tcg_temp_free(EA);                                                        \
-}
-
 #define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk)                   \
 static void glue(gen_, name##x)(DisasContext *ctx)                            \
 {                                                                             \
@@ -2677,19 +2633,6 @@ static void glue(gen_, name##x)(DisasContext *ctx)                            \
 #define GEN_STX_HVRM(name, stop, opc2, opc3, type)                            \
     GEN_STX_E(name, stop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
 
-#define GEN_STS(name, stop, op, type)                                         \
-GEN_ST(name, stop, op | 0x20, type);                                          \
-GEN_STU(name, stop, op | 0x21, type);                                         \
-GEN_STUX(name, stop, 0x17, op | 0x01, type);                                  \
-GEN_STX(name, stop, 0x17, op | 0x00, type)
-
-/* stb stbu stbux stbx */
-GEN_STS(stb, st8, 0x06, PPC_INTEGER);
-/* sth sthu sthux sthx */
-GEN_STS(sth, st16, 0x0C, PPC_INTEGER);
-/* stw stwu stwux stwx */
-GEN_STS(stw, st32, 0x04, PPC_INTEGER);
-
 #define GEN_STEPX(name, stop, opc2, opc3)                                     \
 static void glue(gen_, name##epx)(DisasContext *ctx)                          \
 {                                                                             \
@@ -2711,8 +2654,6 @@ GEN_STEPX(std, DEF_MEMOP(MO_Q), 0x1d, 0x04)
 #endif
 
 #if defined(TARGET_PPC64)
-GEN_STUX(std, st64_i64, 0x15, 0x05, PPC_64B);
-GEN_STX(std, st64_i64, 0x15, 0x04, PPC_64B);
 GEN_STX_HVRM(stdcix, st64_i64, 0x15, 0x1f, PPC_CILDST)
 GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
 GEN_STX_HVRM(sthcix, st16, 0x15, 0x1d, PPC_CILDST)
@@ -7372,31 +7313,11 @@ GEN_LDEPX(lw, DEF_MEMOP(MO_UL), 0x1F, 0x00)
 GEN_LDEPX(ld, DEF_MEMOP(MO_Q), 0x1D, 0x00)
 #endif
 
-#undef GEN_ST
-#undef GEN_STU
-#undef GEN_STUX
 #undef GEN_STX_E
-#undef GEN_STS
-#define GEN_ST(name, stop, opc, type)                                         \
-GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_STU(name, stop, opc, type)                                        \
-GEN_HANDLER(stop##u, opc, 0xFF, 0xFF, 0x00000000, type),
-#define GEN_STUX(name, stop, opc2, opc3, type)                                \
-GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
 #define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk)                   \
 GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000000, type, type2),
-#define GEN_STS(name, stop, op, type)                                         \
-GEN_ST(name, stop, op | 0x20, type)                                           \
-GEN_STU(name, stop, op | 0x21, type)                                          \
-GEN_STUX(name, stop, 0x17, op | 0x01, type)                                   \
-GEN_STX(name, stop, 0x17, op | 0x00, type)
-
-GEN_STS(stb, st8, 0x06, PPC_INTEGER)
-GEN_STS(sth, st16, 0x0C, PPC_INTEGER)
-GEN_STS(stw, st32, 0x04, PPC_INTEGER)
+
 #if defined(TARGET_PPC64)
-GEN_STUX(std, st64_i64, 0x15, 0x05, PPC_64B)
-GEN_STX(std, st64_i64, 0x15, 0x04, PPC_64B)
 GEN_STX_E(stdbr, st64r_i64, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE)
 GEN_STX_HVRM(stdcix, st64_i64, 0x15, 0x1f, PPC_CILDST)
 GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index 80f849fc4a..b36011a539 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -121,6 +121,48 @@ static bool trans_LDU(DisasContext *ctx, arg_D *a)
     return do_ldst_D(ctx, a, true, false, MO_Q);
 }
 
+static bool trans_STB(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, true, MO_UB);
+}
+
+static bool trans_STBU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, true, MO_UB);
+}
+
+static bool trans_STH(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, true, MO_UW);
+}
+
+static bool trans_STHU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, true, MO_UW);
+}
+
+static bool trans_STW(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, false, true, MO_UL);
+}
+
+static bool trans_STWU(DisasContext *ctx, arg_D *a)
+{
+    return do_ldst_D(ctx, a, true, true, MO_UL);
+}
+
+static bool trans_STD(DisasContext *ctx, arg_D *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_D(ctx, a, false, true, MO_Q);
+}
+
+static bool trans_STDU(DisasContext *ctx, arg_D *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_D(ctx, a, true, true, MO_Q);
+}
+
 static bool do_ldst_X(DisasContext *ctx, arg_X *a, bool update,
                       bool store, MemOp mop)
 {
@@ -218,6 +260,48 @@ static bool trans_LDUX(DisasContext *ctx, arg_X *a)
     return do_ldst_X(ctx, a, true, false, MO_Q);
 }
 
+static bool trans_STBX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, true, MO_UB);
+}
+
+static bool trans_STBUX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, true, MO_UB);
+}
+
+static bool trans_STHX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, true, MO_UW);
+}
+
+static bool trans_STHUX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, true, MO_UW);
+}
+
+static bool trans_STWX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, false, true, MO_UL);
+}
+
+static bool trans_STWUX(DisasContext *ctx, arg_X *a)
+{
+    return do_ldst_X(ctx, a, true, true, MO_UL);
+}
+
+static bool trans_STDX(DisasContext *ctx, arg_X *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_X(ctx, a, false, true, MO_Q);
+}
+
+static bool trans_STDUX(DisasContext *ctx, arg_X *a)
+{
+    REQUIRE_INSNS_FLAGS(ctx, 64B);
+    return do_ldst_X(ctx, a, true, true, MO_Q);
+}
+
 static bool do_ldst_PLS_D(DisasContext *ctx, arg_PLS_D *a,
                           bool store, MemOp mop)
 {
-- 
2.25.1



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

* [PATCH v2 14/15] target/ppc: Implement prefixed integer store instructions
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (12 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 13/15] target/ppc: Move D/DS/X-form integer stores to decodetree Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-27 17:16 ` [PATCH v2 15/15] target/ppc: Check cpu flags for prefixed insn support Luis Pires
  14 siblings, 0 replies; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, lagarcia, bruno.larsen, matheus.ferst, david

From: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/insn64.decode                   | 12 ++++++++++++
 target/ppc/translate/fixedpoint-impl.c.inc | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
index 2e08d89e62..0f3b0b2725 100644
--- a/target/ppc/insn64.decode
+++ b/target/ppc/insn64.decode
@@ -41,6 +41,18 @@ PLWA            000001 00 0--.-- .................. \
 PLD             000001 00 0--.-- .................. \
                 111001 ..... ..... ................     @PLS_D
 
+### Fixed-Point Store Instructions
+
+PSTW            000001 10 0--.-- .................. \
+                100100 ..... ..... ................     @PLS_D
+PSTB            000001 10 0--.-- .................. \
+                100110 ..... ..... ................     @PLS_D
+PSTH            000001 10 0--.-- .................. \
+                101100 ..... ..... ................     @PLS_D
+
+PSTD            000001 00 0--.-- .................. \
+                111101 ..... ..... ................     @PLS_D
+
 ### Fixed-Point Arithmetic Instructions
 
 PADDI           000001 10 0--.-- ..................     \
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc b/target/ppc/translate/fixedpoint-impl.c.inc
index b36011a539..4ba477eb93 100644
--- a/target/ppc/translate/fixedpoint-impl.c.inc
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -362,6 +362,26 @@ static bool trans_PLD(DisasContext *ctx, arg_PLS_D *a)
     return do_ldst_PLS_D(ctx, a, false, MO_Q);
 }
 
+static bool trans_PSTB(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, true, MO_UB);
+}
+
+static bool trans_PSTH(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, true, MO_UW);
+}
+
+static bool trans_PSTW(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, true, MO_UL);
+}
+
+static bool trans_PSTD(DisasContext *ctx, arg_PLS_D *a)
+{
+    return do_ldst_PLS_D(ctx, a, true, MO_Q);
+}
+
 static bool trans_ADDI(DisasContext *ctx, arg_D *a)
 {
     if (a->ra) {
-- 
2.25.1



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

* [PATCH v2 15/15] target/ppc: Check cpu flags for prefixed insn support
  2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
                   ` (13 preceding siblings ...)
  2021-04-27 17:16 ` [PATCH v2 14/15] target/ppc: Implement prefixed integer store instructions Luis Pires
@ 2021-04-27 17:16 ` Luis Pires
  2021-04-28 15:37   ` Richard Henderson
  14 siblings, 1 reply; 26+ messages in thread
From: Luis Pires @ 2021-04-27 17:16 UTC (permalink / raw)
  To: qemu-devel, qemu-ppc
  Cc: richard.henderson, f4bug, Luis Pires, lagarcia, bruno.larsen,
	matheus.ferst, david

Prefixed instructions were introduced in Power ISA 3.1

Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
---
 target/ppc/translate.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 7422ea4e13..f4802a4f08 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7837,7 +7837,11 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
 
 static bool is_prefix_insn(DisasContext *ctx, uint32_t insn)
 {
-    /* TODO: Check ctx->insns_flags* for whether prefixes are supported. */
+    if (!(ctx->insns_flags2 & PPC2_ISA310)) {
+        /* Prefixed instructions are not supported */
+        return false;
+    }
+
     return opc1(insn) == 1;
 }
 
-- 
2.25.1



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

* Re: [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree
  2021-04-27 17:16 ` [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree Luis Pires
@ 2021-04-28 13:31   ` Matheus K. Ferst
  2021-04-28 15:34     ` Richard Henderson
  0 siblings, 1 reply; 26+ messages in thread
From: Matheus K. Ferst @ 2021-04-28 13:31 UTC (permalink / raw)
  To: Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, richard.henderson, f4bug, david

On 27/04/2021 14:16, Luis Pires wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> These are all connected by macros in the legacy decoding.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
>   target/ppc/insn32.decode                   |  37 ++++
>   target/ppc/translate.c                     | 136 ++-------------
>   target/ppc/translate/fixedpoint-impl.c.inc | 188 +++++++++++++++++++++
>   3 files changed, 238 insertions(+), 123 deletions(-)
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index 878d2f2f66..bf39ce5c15 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -20,6 +20,43 @@
>   &D              rt ra si
>   @D              ...... rt:5 ra:5 si:s16                 &D
>   
> +%ds_si          2:s14  !function=times_4
> +@DS             ...... rt:5 ra:5 .............. ..      &D si=%ds_si
> +
> +&X              rt ra rb
> +@X              ...... rt:5 ra:5 rb:5 .......... .      &X
> +

This is a bit problematic, the instruction form isn't enough to decide its
fields. Eg. setb is X-form, but the fields are rt:5 bfa:3, setbc is also 
X-form
and the fields are rt:5 ba:5. In fact, for the X-form, there is a whole 
page of
field designations in PowerISA v3.1.

I would break this into three cases:
  - Some forms have single field designations. Eg. B, XLS, XX4;
  - Others have multiple designations but are just alternative names for the
    fields. Eg. DQ, DS, M;
  - And there are forms with multiple designations, with a variable 
number of
    fields that may overlap each other. Eg. X, XFX, XX2.

The first is a non-issue, just use the form name as done here. The 
second seems
tractable, we could pick one field name for each part of the insn and 
still use
the form name as the identifier for args_def/fmt_def. The last case will 
likely
require multiple fmt_defs for each form, in which case we would need to 
come up
with a pattern to name them.

Looking at what Binutils did when they added Power10 support, it seems 
that the
insn form is just a hint for opcode positions, and fields are specified 
for each
insn. The sad part of this kind of approach is that it would leave us 
with, eg.
arg_LBZX and arg_LBZUX instead of a single arg_X, making it harder to put
multiple insns under the same implementation.

Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>


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

* Re: [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
  2021-04-27 17:16 ` [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Luis Pires
@ 2021-04-28 14:10   ` Matheus K. Ferst
  2021-04-28 15:23     ` Richard Henderson
  0 siblings, 1 reply; 26+ messages in thread
From: Matheus K. Ferst @ 2021-04-28 14:10 UTC (permalink / raw)
  To: Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, richard.henderson, f4bug, david

On 27/04/2021 14:16, Luis Pires wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/ppc/insn32.decode                   |  8 +++++
>   target/ppc/insn64.decode                   | 14 ++++++++
>   target/ppc/translate.c                     | 29 ---------------
>   target/ppc/translate/fixedpoint-impl.c.inc | 42 ++++++++++++++++++++++
>   4 files changed, 64 insertions(+), 29 deletions(-)
> 
> diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> index b175441209..878d2f2f66 100644
> --- a/target/ppc/insn32.decode
> +++ b/target/ppc/insn32.decode
> @@ -16,3 +16,11 @@
>   # You should have received a copy of the GNU Lesser General Public
>   # License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   #
> +
> +&D              rt ra si
> +@D              ...... rt:5 ra:5 si:s16                 &D
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +ADDI            001110 ..... ..... ................     @D
> +ADDIS           001111 ..... ..... ................     @D
> diff --git a/target/ppc/insn64.decode b/target/ppc/insn64.decode
> index 9fc45d0614..68ed2cbff8 100644
> --- a/target/ppc/insn64.decode
> +++ b/target/ppc/insn64.decode
> @@ -16,3 +16,17 @@
>   # You should have received a copy of the GNU Lesser General Public
>   # License along with this library; if not, see <http://www.gnu.org/licenses/>.
>   #
> +
> +# Format MLS:D and 8LS:D
> +&PLS_D          rt ra si r
> +
> +%pls_si         32:s18 0:16
> +
> +@PLS_D          ...... .. ... r:1 .. .................. \
> +                ...... rt:5 ra:5 ................       \
> +                &PLS_D si=%pls_si
> +
> +### Fixed-Point Arithmetic Instructions
> +
> +PADDI           000001 10 0--.-- ..................     \
> +                001110 ..... ..... ................     @PLS_D

<snip>

> +
> +static bool trans_ADDI(DisasContext *ctx, arg_D *a)
> +{
> +    if (a->ra) {
> +        tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
> +    } else {
> +        tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
> +    }
> +    return true;
> +}
> +
> +static bool trans_ADDIS(DisasContext *ctx, arg_D *a)
> +{
> +    a->si <<= 16;
> +    return trans_ADDI(ctx, a);
> +}
> +
> +static bool trans_PADDI(DisasContext *ctx, arg_PLS_D *a)
> +{
> +    if (!resolve_PLS_D(ctx, a)) {
> +        return false;
> +    }
> +    if (a->ra) {
> +        tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
> +    } else {
> +        tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
> +    }
> +    return true;
> +}
> 

In our first attempt, we did some efforts to keep prefixed instructions 
type 0b10 and 0b11 under the same implementation as their word-size 
counterpart, i.e. trans_ADDI and trans_PADDI had the same signature and 
just forwarded their arguments to a third method that does the real 
work. Is this kind of approach desirable? We initially achieved this by 
using const_elt to set r=0 for addi, which is not particularly nice, but 
we can look for other solutions.

Thanks,
Matheus K. Ferst
Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/>
Analista de Software Júnior
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>


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

* Re: [PATCH v2 02/15] target/ppc: Add cia field to DisasContext
  2021-04-27 17:16 ` [PATCH v2 02/15] target/ppc: Add cia field to DisasContext Luis Pires
@ 2021-04-28 14:55   ` Richard Henderson
  2021-04-28 14:59     ` Luis Fernando Fujita Pires
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2021-04-28 14:55 UTC (permalink / raw)
  To: Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, matheus.ferst, f4bug, david

On 4/27/21 10:16 AM, Luis Pires wrote:
> From: Richard Henderson<richard.henderson@linaro.org>
> 
> Signed-off-by: Richard Henderson<richard.henderson@linaro.org>
> ---
>   target/ppc/translate.c | 34 ++++++++++++++++++----------------
>   1 file changed, 18 insertions(+), 16 deletions(-)

When a patch passes through your hands, it should contain your S-o-b.


r~


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

* RE: [PATCH v2 02/15] target/ppc: Add cia field to DisasContext
  2021-04-28 14:55   ` Richard Henderson
@ 2021-04-28 14:59     ` Luis Fernando Fujita Pires
  0 siblings, 0 replies; 26+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-28 14:59 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, qemu-ppc
  Cc: lagarcia, Bruno Piazera Larsen, Matheus Kowalczuk Ferst, f4bug, david

I'll do that, thanks!

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: quarta-feira, 28 de abril de 2021 11:55
> To: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>; qemu-
> devel@nongnu.org; qemu-ppc@nongnu.org
> Cc: david@gibson.dropbear.id.au; Matheus Kowalczuk Ferst
> <matheus.ferst@eldorado.org.br>; Bruno Piazera Larsen
> <bruno.larsen@eldorado.org.br>; lagarcia@br.ibm.com; f4bug@amsat.org
> Subject: Re: [PATCH v2 02/15] target/ppc: Add cia field to DisasContext
> 
> On 4/27/21 10:16 AM, Luis Pires wrote:
> > From: Richard Henderson<richard.henderson@linaro.org>
> >
> > Signed-off-by: Richard Henderson<richard.henderson@linaro.org>
> > ---
> >   target/ppc/translate.c | 34 ++++++++++++++++++----------------
> >   1 file changed, 18 insertions(+), 16 deletions(-)
> 
> When a patch passes through your hands, it should contain your S-o-b.
> 
> 
> r~

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

* Re: [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception*
  2021-04-27 17:16 ` [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception* Luis Pires
@ 2021-04-28 15:05   ` Richard Henderson
  2021-04-29 17:07     ` Luis Fernando Fujita Pires
  0 siblings, 1 reply; 26+ messages in thread
From: Richard Henderson @ 2021-04-28 15:05 UTC (permalink / raw)
  To: Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, matheus.ferst, f4bug, david

On 4/27/21 10:16 AM, Luis Pires wrote:
> -static inline void gen_stop_exception(DisasContext *ctx)
> +static inline void gen_end_tb_exception(DisasContext *ctx, uint32_t excp)
>   {
> -    gen_update_nip(ctx, ctx->base.pc_next);
> -    ctx->exception = POWERPC_EXCP_STOP;
> +    /* No need to update nip for SYNC/BRANCH, as execution flow will change */
> +    if ((excp != POWERPC_EXCP_SYNC) &&
> +        (excp != POWERPC_EXCP_BRANCH))
> +    {
> +        gen_update_nip(ctx, ctx->base.pc_next);
> +    }
> +    ctx->exception = excp;
> +    ctx->base.is_jmp = DISAS_NORETURN;
>   }

Hmm.  You didn't actually raise the exception, so you can't set DISAS_NORETURN 
that way.  It looks like you should be using gen_exception_nip().

And as side notes: (1) no need for extra parentheses, (2) brace is misplaced.


r~


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

* Re: [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI
  2021-04-28 14:10   ` Matheus K. Ferst
@ 2021-04-28 15:23     ` Richard Henderson
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2021-04-28 15:23 UTC (permalink / raw)
  To: Matheus K. Ferst, Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, f4bug, david

On 4/28/21 7:10 AM, Matheus K. Ferst wrote:
> In our first attempt, we did some efforts to keep prefixed instructions type 
> 0b10 and 0b11 under the same implementation as their word-size counterpart, 
> i.e. trans_ADDI and trans_PADDI had the same signature and just forwarded their 
> arguments to a third method that does the real work. Is this kind of approach 
> desirable? We initially achieved this by using const_elt to set r=0 for addi, 
> which is not particularly nice, but we can look for other solutions.

Yes, I could have tried harder to share the implementation here.  And in 
retrospect, using a &PLS_D argument set for the non-prefixed integer load/store 
insns would have been fairly easy, and reduce 30 lines of unnecessary duplication.

For the MMIRR prefixed instructions, that pain of duplication would be a lot 
higher.


r~


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

* Re: [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree
  2021-04-28 13:31   ` Matheus K. Ferst
@ 2021-04-28 15:34     ` Richard Henderson
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2021-04-28 15:34 UTC (permalink / raw)
  To: Matheus K. Ferst, Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, f4bug, david

On 4/28/21 6:31 AM, Matheus K. Ferst wrote:
> This is a bit problematic, the instruction form isn't enough to decide its
> fields. Eg. setb is X-form, but the fields are rt:5 bfa:3, setbc is also X-form
> and the fields are rt:5 ba:5. In fact, for the X-form, there is a whole page of
> field designations in PowerISA v3.1.
> 
> I would break this into three cases:
>   - Some forms have single field designations. Eg. B, XLS, XX4;
>   - Others have multiple designations but are just alternative names for the
>     fields. Eg. DQ, DS, M;
>   - And there are forms with multiple designations, with a variable number of
>     fields that may overlap each other. Eg. X, XFX, XX2.
> 
> The first is a non-issue, just use the form name as done here. The second seems
> tractable, we could pick one field name for each part of the insn and still use
> the form name as the identifier for args_def/fmt_def. The last case will likely
> require multiple fmt_defs for each form, in which case we would need to come up
> with a pattern to name them.

Yep.

> 
> Looking at what Binutils did when they added Power10 support, it seems that the
> insn form is just a hint for opcode positions, and fields are specified for each
> insn. The sad part of this kind of approach is that it would leave us with, eg.
> arg_LBZX and arg_LBZUX instead of a single arg_X, making it harder to put
> multiple insns under the same implementation.

You'll just make up names for those that are used by more than one instruction. 
  E.g. X, X_rc, X_l, X_wc_pl, etc.


r~


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

* Re: [PATCH v2 15/15] target/ppc: Check cpu flags for prefixed insn support
  2021-04-27 17:16 ` [PATCH v2 15/15] target/ppc: Check cpu flags for prefixed insn support Luis Pires
@ 2021-04-28 15:37   ` Richard Henderson
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2021-04-28 15:37 UTC (permalink / raw)
  To: Luis Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, bruno.larsen, matheus.ferst, f4bug, david

On 4/27/21 10:16 AM, Luis Pires wrote:
> Prefixed instructions were introduced in Power ISA 3.1
> 
> Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
> ---
>   target/ppc/translate.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 7422ea4e13..f4802a4f08 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7837,7 +7837,11 @@ static bool ppc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
>   
>   static bool is_prefix_insn(DisasContext *ctx, uint32_t insn)
>   {
> -    /* TODO: Check ctx->insns_flags* for whether prefixes are supported. */
> +    if (!(ctx->insns_flags2 & PPC2_ISA310)) {
> +        /* Prefixed instructions are not supported */
> +        return false;
> +    }

Patch 11 introduced REQUIRE_INSNS_FLAGS; this pattern calls for the 
introduction of REQUIRE_INSNS_FLAGS2, as you'll need it later.

Fold this back into patch 8, or move this to be patch 9, so that we don't have 
a range of patches which accept invalid instructions.


r~


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

* RE: [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception*
  2021-04-28 15:05   ` Richard Henderson
@ 2021-04-29 17:07     ` Luis Fernando Fujita Pires
  2021-04-29 17:21       ` Richard Henderson
  0 siblings, 1 reply; 26+ messages in thread
From: Luis Fernando Fujita Pires @ 2021-04-29 17:07 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, qemu-ppc
  Cc: lagarcia, Bruno Piazera Larsen, Matheus Kowalczuk Ferst, f4bug, david

> > -static inline void gen_stop_exception(DisasContext *ctx)
> > +static inline void gen_end_tb_exception(DisasContext *ctx, uint32_t
> > +excp)
> >   {
> > -    gen_update_nip(ctx, ctx->base.pc_next);
> > -    ctx->exception = POWERPC_EXCP_STOP;
> > +    /* No need to update nip for SYNC/BRANCH, as execution flow will change
> */
> > +    if ((excp != POWERPC_EXCP_SYNC) &&
> > +        (excp != POWERPC_EXCP_BRANCH))
> > +    {
> > +        gen_update_nip(ctx, ctx->base.pc_next);
> > +    }
> > +    ctx->exception = excp;
> > +    ctx->base.is_jmp = DISAS_NORETURN;
> >   }
> 
> Hmm.  You didn't actually raise the exception, so you can't set
> DISAS_NORETURN that way.  It looks like you should be using
> gen_exception_nip().

This is reproducing the behavior that was implemented before the DISAS_NORETURN changes, that caused check-tcg to fail with an assertion otherwise.

IIUC, POWERPC_EXCP_{STOP,SYNC,BRANCH} are not really exceptions and, in these cases, ctx->exception is being used just to cause  ppc_tr_translate_insn() to end the translation block. If so, we should not be using ctx->exception for that, but I believe fixing that to not use ctx->exception belongs in a separate stand-alone patch.

> 
> And as side notes: (1) no need for extra parentheses, (2) brace is misplaced.
> 
> 
> r~

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

* Re: [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception*
  2021-04-29 17:07     ` Luis Fernando Fujita Pires
@ 2021-04-29 17:21       ` Richard Henderson
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Henderson @ 2021-04-29 17:21 UTC (permalink / raw)
  To: Luis Fernando Fujita Pires, qemu-devel, qemu-ppc
  Cc: lagarcia, Bruno Piazera Larsen, Matheus Kowalczuk Ferst, f4bug, david

On 4/29/21 10:07 AM, Luis Fernando Fujita Pires wrote:
>>> -static inline void gen_stop_exception(DisasContext *ctx)
>>> +static inline void gen_end_tb_exception(DisasContext *ctx, uint32_t
>>> +excp)
>>>    {
>>> -    gen_update_nip(ctx, ctx->base.pc_next);
>>> -    ctx->exception = POWERPC_EXCP_STOP;
>>> +    /* No need to update nip for SYNC/BRANCH, as execution flow will change
>> */
>>> +    if ((excp != POWERPC_EXCP_SYNC) &&
>>> +        (excp != POWERPC_EXCP_BRANCH))
>>> +    {
>>> +        gen_update_nip(ctx, ctx->base.pc_next);
>>> +    }
>>> +    ctx->exception = excp;
>>> +    ctx->base.is_jmp = DISAS_NORETURN;
>>>    }
>>
>> Hmm.  You didn't actually raise the exception, so you can't set
>> DISAS_NORETURN that way.  It looks like you should be using
>> gen_exception_nip().
> 
> This is reproducing the behavior that was implemented before the DISAS_NORETURN changes, that caused check-tcg to fail with an assertion otherwise.
> 
> IIUC, POWERPC_EXCP_{STOP,SYNC,BRANCH} are not really exceptions and, in these cases, ctx->exception is being used just to cause  ppc_tr_translate_insn() to end the translation block. If so, we should not be using ctx->exception for that, but I believe fixing that to not use ctx->exception belongs in a separate stand-alone patch.

Hum.  Well, you can set is_jmp = DISAS_TOO_MANY to force exit from the 
translation loop.

I believe a proper fix would be to turn all of these "fake" exceptions into 
DISAS_FOO constants, to be assigned to is_jmp. There are a bunch of 
DISAS_TARGET_N enumerators which should be renamed like in target/arm/translate.h:

#define DISAS_WFI       DISAS_TARGET_2
#define DISAS_SWI       DISAS_TARGET_3

etc.  Then most of the code that is special casing these constants should get 
moved to ppc_tr_tb_stop.


r~


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

end of thread, other threads:[~2021-04-29 17:35 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27 17:16 [PATCH v2 00/15] Base for adding PowerPC 64-bit instructions Luis Pires
2021-04-27 17:16 ` [PATCH v2 01/15] decodetree: Add support for " Luis Pires
2021-04-27 17:16 ` [PATCH v2 02/15] target/ppc: Add cia field to DisasContext Luis Pires
2021-04-28 14:55   ` Richard Henderson
2021-04-28 14:59     ` Luis Fernando Fujita Pires
2021-04-27 17:16 ` [PATCH v2 03/15] target/ppc: Split out decode_legacy Luis Pires
2021-04-27 17:16 ` [PATCH v2 04/15] target/ppc: Move DISAS_NORETURN setting into gen_exception* Luis Pires
2021-04-28 15:05   ` Richard Henderson
2021-04-29 17:07     ` Luis Fernando Fujita Pires
2021-04-29 17:21       ` Richard Henderson
2021-04-27 17:16 ` [PATCH v2 05/15] target/ppc: Tidy exception vs exit_tb Luis Pires
2021-04-27 17:16 ` [PATCH v2 06/15] target/ppc: Mark helper_raise_exception* as noreturn Luis Pires
2021-04-27 17:16 ` [PATCH v2 07/15] target/ppc: Use translator_loop_temp_check Luis Pires
2021-04-27 17:16 ` [PATCH v2 08/15] target/ppc: Add infrastructure for prefixed insns Luis Pires
2021-04-27 17:16 ` [PATCH v2 09/15] target/ppc: Move ADDI, ADDIS to decodetree, implement PADDI Luis Pires
2021-04-28 14:10   ` Matheus K. Ferst
2021-04-28 15:23     ` Richard Henderson
2021-04-27 17:16 ` [PATCH v2 10/15] target/ppc: Implement PNOP Luis Pires
2021-04-27 17:16 ` [PATCH v2 11/15] target/ppc: Move D/DS/X-form integer loads to decodetree Luis Pires
2021-04-28 13:31   ` Matheus K. Ferst
2021-04-28 15:34     ` Richard Henderson
2021-04-27 17:16 ` [PATCH v2 12/15] target/ppc: Implement prefixed integer load instructions Luis Pires
2021-04-27 17:16 ` [PATCH v2 13/15] target/ppc: Move D/DS/X-form integer stores to decodetree Luis Pires
2021-04-27 17:16 ` [PATCH v2 14/15] target/ppc: Implement prefixed integer store instructions Luis Pires
2021-04-27 17:16 ` [PATCH v2 15/15] target/ppc: Check cpu flags for prefixed insn support Luis Pires
2021-04-28 15:37   ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).