Re: [Qemu-devel] [PATCH v14 11/33] target-tilegx: Framework for decoding bundles

[Richard Henderson]

On 08/29/2015 02:08 PM, Peter Maydell wrote:

On 24 August 2015 at 17:17, Richard Henderson  wrote:

Signed-off-by: Richard Henderson 
+if (dc->jmp.cond != TCG_COND_NEVER) {
+if (dc->jmp.cond == TCG_COND_ALWAYS) {
+tcg_gen_mov_i64(cpu_pc, dc->jmp.dest);
+} else {
+TCGv next = tcg_const_i64(dc->pc + TILEGX_BUNDLE_SIZE_IN_BYTES);
+tcg_gen_movcond_i64(dc->jmp.cond, cpu_pc,
+dc->jmp.val1, load_zero(dc),
+dc->jmp.dest, next);
+tcg_temp_free_i64(dc->jmp.val1);
+tcg_temp_free_i64(next);
+}
+tcg_temp_free_i64(dc->jmp.dest);
+tcg_gen_exit_tb(0);
+dc->exit_tb = true;
+}


Doing conditional branches with movcond to pc means we'll
never be able to link TBs which end with conditional branches,
right?


The structure of the code is such that we could use goto_tb.
I just didn't want to complicate the initial implementation.


r~

Re: [Qemu-devel] [PATCH v14 11/33] target-tilegx: Framework for decoding bundles

[Chen Gang]

On 8/29/15 22:50, Peter Maydell wrote:
> On 24 August 2015 at 17:17, Richard Henderson  wrote:
>
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, " %" PRIx64 ": { ", dc->pc);
>> + if (get_Mode(bundle)) {
>> + notice_excp(dc, bundle, "y0", decode_y0(dc, bundle));
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, " ; ");
>> + notice_excp(dc, bundle, "y1", decode_y1(dc, bundle));
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, " ; ");
>> + notice_excp(dc, bundle, "y2", decode_y2(dc, bundle));
>> + } else {
>> + notice_excp(dc, bundle, "x0", decode_x0(dc, bundle));
>> + qemu_log_mask(CPU_LOG_TB_IN_ASM, " ; ");
>> + notice_excp(dc, bundle, "x1", decode_x1(dc, bundle));
>> + }
>
> I notice that even if the first insn in a bundle generates an
> exception we'll go ahead and generate unreachable code for
> the rest.
>
> I haven't tried to find and compare against any instruction
> set documentation, but structurally it looks good, so
>

For me, I am not quite sure it is really safe, more explanations are
welcomed.

And for each bundle, I still suggest to:

- buffering dest regs in temp regs for each pipe.

- buffering 1st exception information in env for the related pipe.

- flush temp regs to dest regs.

- gen exception based on the env information.

I assumed all exceptions which can still let working flow continue are
all in pipe x1 (at present, it should be corret), so it is enough for us
to only process 1st exception of bundle.


> Reviewed-by: Peter Maydell 


Thanks.
--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed
  

Re: [Qemu-devel] [PATCH v14 11/33] target-tilegx: Framework for decoding bundles

[Peter Maydell]

On 24 August 2015 at 17:17, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 
> +if (dc->jmp.cond != TCG_COND_NEVER) {
> +if (dc->jmp.cond == TCG_COND_ALWAYS) {
> +tcg_gen_mov_i64(cpu_pc, dc->jmp.dest);
> +} else {
> +TCGv next = tcg_const_i64(dc->pc + TILEGX_BUNDLE_SIZE_IN_BYTES);
> +tcg_gen_movcond_i64(dc->jmp.cond, cpu_pc,
> +dc->jmp.val1, load_zero(dc),
> +dc->jmp.dest, next);
> +tcg_temp_free_i64(dc->jmp.val1);
> +tcg_temp_free_i64(next);
> +}
> +tcg_temp_free_i64(dc->jmp.dest);
> +tcg_gen_exit_tb(0);
> +dc->exit_tb = true;
> +}

Doing conditional branches with movcond to pc means we'll
never be able to link TBs which end with conditional branches,
right?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v14 11/33] target-tilegx: Framework for decoding bundles

[Peter Maydell]

On 24 August 2015 at 17:17, Richard Henderson  wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target-tilegx/translate.c | 1145 
> +
>  1 file changed, 1145 insertions(+)
>  create mode 100644 target-tilegx/translate.c
>
> diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
> new file mode 100644
> index 000..a2d597d
> --- /dev/null
> +++ b/target-tilegx/translate.c
> @@ -0,0 +1,1145 @@
> +/*
> + * QEMU TILE-Gx CPU
> + *
> + *  Copyright (c) 2015 Chen Gang
> + *
> + * 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
> + * 
> + */
> +
> +#include "cpu.h"
> +#include "qemu/log.h"
> +#include "disas/disas.h"
> +#include "tcg-op.h"
> +#include "exec/cpu_ldst.h"
> +#include "opcode_tilegx.h"
> +
> +#define FMT64X  "%016" PRIx64
> +
> +static TCGv_ptr cpu_env;
> +static TCGv cpu_pc;
> +static TCGv cpu_regs[TILEGX_R_COUNT];
> +
> +static const char * const reg_names[64] = {
> + "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
> + "r8",  "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> +"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
> +"r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
> +"r32", "r33", "r34", "r35", "r36", "r37", "r38", "r39",
> +"r40", "r41", "r42", "r43", "r44", "r45", "r46", "r47",
> +"r48", "r49", "r50", "r51",  "bp",  "tp",  "sp",  "lr",
> +"sn", "idn0", "idn1", "udn0", "udn1", "udn2", "udn2", "zero"
> +};
> +
> +/* Modified registers are cached in temporaries until the end of the bundle. 
> */
> +typedef struct {
> +unsigned reg;
> +TCGv val;
> +} DisasContextTemp;
> +
> +#define MAX_WRITEBACK 4
> +
> +/* This is the state at translation time.  */
> +typedef struct {
> +uint64_t pc;   /* Current pc */
> +
> +TCGv zero;  /* For zero register */
> +
> +DisasContextTemp wb[MAX_WRITEBACK];
> +int num_wb;
> +int mmuidx;
> +bool exit_tb;
> +
> +struct {
> +TCGCond cond;/* branch condition */
> +TCGv dest;   /* branch destination */
> +TCGv val1;   /* value to be compared against zero, for cond */
> +} jmp;   /* Jump object, only once in each TB block */
> +} DisasContext;
> +
> +#include "exec/gen-icount.h"
> +
> +/* Differentiate the various pipe encodings.  */
> +#define TY_X0  0
> +#define TY_X1  1
> +#define TY_Y0  2
> +#define TY_Y1  3
> +
> +/* Remerge the base opcode and extension fields for switching.
> +   The X opcode fields are 3 bits; Y0/Y1 opcode fields are 4 bits;
> +   Y2 opcode field is 2 bits.  */
> +#define OE(OP, EXT, XY) (TY_##XY + OP * 4 + EXT * 64)

Slightly odd to assemble bitfields with multiplies and adds
rather than shifts and logical-or.

> +qemu_log_mask(CPU_LOG_TB_IN_ASM, "  %" PRIx64 ":  { ", dc->pc);
> +if (get_Mode(bundle)) {
> +notice_excp(dc, bundle, "y0", decode_y0(dc, bundle));
> +qemu_log_mask(CPU_LOG_TB_IN_ASM, " ; ");
> +notice_excp(dc, bundle, "y1", decode_y1(dc, bundle));
> +qemu_log_mask(CPU_LOG_TB_IN_ASM, " ; ");
> +notice_excp(dc, bundle, "y2", decode_y2(dc, bundle));
> +} else {
> +notice_excp(dc, bundle, "x0", decode_x0(dc, bundle));
> +qemu_log_mask(CPU_LOG_TB_IN_ASM, " ; ");
> +notice_excp(dc, bundle, "x1", decode_x1(dc, bundle));
> +}

I notice that even if the first insn in a bundle generates an
exception we'll go ahead and generate unreachable code for
the rest.

I haven't tried to find and compare against any instruction
set documentation, but structurally it looks good, so

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v14 11/33] target-tilegx: Framework for decoding bundles

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-tilegx/translate.c | 1145 +
 1 file changed, 1145 insertions(+)
 create mode 100644 target-tilegx/translate.c

diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
new file mode 100644
index 000..a2d597d
--- /dev/null
+++ b/target-tilegx/translate.c
@@ -0,0 +1,1145 @@
+/*
+ * QEMU TILE-Gx CPU
+ *
+ *  Copyright (c) 2015 Chen Gang
+ *
+ * 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
+ * 
+ */
+
+#include "cpu.h"
+#include "qemu/log.h"
+#include "disas/disas.h"
+#include "tcg-op.h"
+#include "exec/cpu_ldst.h"
+#include "opcode_tilegx.h"
+
+#define FMT64X  "%016" PRIx64
+
+static TCGv_ptr cpu_env;
+static TCGv cpu_pc;
+static TCGv cpu_regs[TILEGX_R_COUNT];
+
+static const char * const reg_names[64] = {
+ "r0",  "r1",  "r2",  "r3",  "r4",  "r5",  "r6",  "r7",
+ "r8",  "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+"r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
+"r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+"r32", "r33", "r34", "r35", "r36", "r37", "r38", "r39",
+"r40", "r41", "r42", "r43", "r44", "r45", "r46", "r47",
+"r48", "r49", "r50", "r51",  "bp",  "tp",  "sp",  "lr",
+"sn", "idn0", "idn1", "udn0", "udn1", "udn2", "udn2", "zero"
+};
+
+/* Modified registers are cached in temporaries until the end of the bundle. */
+typedef struct {
+unsigned reg;
+TCGv val;
+} DisasContextTemp;
+
+#define MAX_WRITEBACK 4
+
+/* This is the state at translation time.  */
+typedef struct {
+uint64_t pc;   /* Current pc */
+
+TCGv zero;  /* For zero register */
+
+DisasContextTemp wb[MAX_WRITEBACK];
+int num_wb;
+int mmuidx;
+bool exit_tb;
+
+struct {
+TCGCond cond;/* branch condition */
+TCGv dest;   /* branch destination */
+TCGv val1;   /* value to be compared against zero, for cond */
+} jmp;   /* Jump object, only once in each TB block */
+} DisasContext;
+
+#include "exec/gen-icount.h"
+
+/* Differentiate the various pipe encodings.  */
+#define TY_X0  0
+#define TY_X1  1
+#define TY_Y0  2
+#define TY_Y1  3
+
+/* Remerge the base opcode and extension fields for switching.
+   The X opcode fields are 3 bits; Y0/Y1 opcode fields are 4 bits;
+   Y2 opcode field is 2 bits.  */
+#define OE(OP, EXT, XY) (TY_##XY + OP * 4 + EXT * 64)
+
+/* Similar, but for Y2 only.  */
+#define OEY2(OP, MODE) (OP + MODE * 4)
+
+/* Similar, but make sure opcode names match up.  */
+#define OE_RR_X0(E)OE(RRR_0_OPCODE_X0, E##_UNARY_OPCODE_X0, X0)
+#define OE_RR_X1(E)OE(RRR_0_OPCODE_X1, E##_UNARY_OPCODE_X1, X1)
+#define OE_RR_Y0(E)OE(RRR_1_OPCODE_Y0, E##_UNARY_OPCODE_Y0, Y0)
+#define OE_RR_Y1(E)OE(RRR_1_OPCODE_Y1, E##_UNARY_OPCODE_Y1, Y1)
+#define OE_RRR(E,N,XY) OE(RRR_##N##_OPCODE_##XY, E##_RRR_##N##_OPCODE_##XY, XY)
+#define OE_IM(E,XY)OE(IMM8_OPCODE_##XY, E##_IMM8_OPCODE_##XY, XY)
+#define OE_SH(E,XY)OE(SHIFT_OPCODE_##XY, E##_SHIFT_OPCODE_##XY, XY)
+
+
+static void gen_exception(DisasContext *dc, TileExcp num)
+{
+TCGv_i32 tmp;
+
+tcg_gen_movi_tl(cpu_pc, dc->pc + TILEGX_BUNDLE_SIZE_IN_BYTES);
+
+tmp = tcg_const_i32(num);
+gen_helper_exception(cpu_env, tmp);
+tcg_temp_free_i32(tmp);
+dc->exit_tb = true;
+}
+
+static TileExcp gen_rr_opcode(DisasContext *dc, unsigned opext,
+  unsigned dest, unsigned srca)
+{
+const char *mnemonic;
+
+/* Eliminate nops before doing anything else.  */
+switch (opext) {
+case OE_RR_Y0(NOP):
+case OE_RR_Y1(NOP):
+case OE_RR_X0(NOP):
+case OE_RR_X1(NOP):
+mnemonic = "nop";
+goto do_nop;
+case OE_RR_Y0(FNOP):
+case OE_RR_Y1(FNOP):
+case OE_RR_X0(FNOP):
+case OE_RR_X1(FNOP):
+mnemonic = "fnop";
+do_nop:
+if (srca || dest) {
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+}
+qemu_log_mask(CPU_LOG_TB_IN_ASM, "%s", mnemonic);
+return TILEGX_EXCP_NONE;
+}
+
+switch (opext) {
+case OE_RR_X0(CNTLZ):
+case OE_RR_Y0(CNTLZ):
+case OE_RR_X0(CNTTZ):
+case OE_RR_Y0(CNTTZ):
+case OE_RR_X1(DRAIN):
+case OE_RR_X1(DTLBPR):
+case OE_RR_X1(FINV):
+case OE_RR_X1(FLUSHWB):
+case OE_RR_X1