Re: [Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families

2014-11-13 Thread Leon Alrae
On 12/11/2014 21:07, Paolo Bonzini wrote:
 
 
 On 08/10/2014 12:55, Leon Alrae wrote:
  case OPC_LUI:
 -tcg_gen_movi_tl(cpu_gpr[rt], imm  16);
 -MIPS_DEBUG(lui %s,  TARGET_FMT_lx, regnames[rt], uimm);
 +if (rs != 0  (ctx-insn_flags  ISA_MIPS32R6)) {
 +/* OPC_AUI */
 +tcg_gen_addi_tl(cpu_gpr[rt], cpu_gpr[rs], imm  16);
 +tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
 +MIPS_DEBUG(aui %s, %s, %04x, regnames[rt], regnames[rs], imm);
 +} else {
 +tcg_gen_movi_tl(cpu_gpr[rt], imm  16);
 +MIPS_DEBUG(lui %s,  TARGET_FMT_lx, regnames[rt], uimm);
 +}
  break;
 
 Coverity reported a
 
 gen_logic_imm(ctx, OPC_LUI, rs, -1, imm);
 
 where the -1 probably has to become zero now.

This line is from microMIPS decoder. We don't support microMIPS R6 in
QEMU so I think this isn't an issue at the moment as (ctx-insn_flags 
ISA_MIPS32R6) should always be false on a CPU with existing
implementation of microMIPS. Nevertheless, I agree this should be 0 for
safety, I'll correct it in 2.3.

Thanks for pointing this out.

Regards,
Leon




Re: [Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families

2014-11-12 Thread Paolo Bonzini


On 08/10/2014 12:55, Leon Alrae wrote:
  case OPC_LUI:
 -tcg_gen_movi_tl(cpu_gpr[rt], imm  16);
 -MIPS_DEBUG(lui %s,  TARGET_FMT_lx, regnames[rt], uimm);
 +if (rs != 0  (ctx-insn_flags  ISA_MIPS32R6)) {
 +/* OPC_AUI */
 +tcg_gen_addi_tl(cpu_gpr[rt], cpu_gpr[rs], imm  16);
 +tcg_gen_ext32s_tl(cpu_gpr[rt], cpu_gpr[rt]);
 +MIPS_DEBUG(aui %s, %s, %04x, regnames[rt], regnames[rs], imm);
 +} else {
 +tcg_gen_movi_tl(cpu_gpr[rt], imm  16);
 +MIPS_DEBUG(lui %s,  TARGET_FMT_lx, regnames[rt], uimm);
 +}
  break;

Coverity reported a

gen_logic_imm(ctx, OPC_LUI, rs, -1, imm);

where the -1 probably has to become zero now.

Paolo



Re: [Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families

2014-10-14 Thread Leon Alrae
Hi Yongbok,

On 13/10/2014 14:37, Yongbok Kim wrote:
 +OPC_PCREL= (0x3B  26),
 +};
 +
 +/* PC-relative address computation / loads  */
 +#define MASK_OPC_PCREL_TOP2BITS(op)  (MASK_OP_MAJOR(op) | (op  (3 
 19)))
 +#define MASK_OPC_PCREL_TOP5BITS(op)  (MASK_OP_MAJOR(op) | (op  (0x1f
  16)))
 
 There must be better name for this macro.
 It confused me that was looking like 31 and 30 bits.
 Just naming though...

TOP2BITS and TOP5BITS are referring to T bits. R6 PC-relative
family encoding: 111011.rs.T.imm16

Instructions:
111011.rs.00.-imm19 ADDIUPC
111011.rs.01.disp19 LWPC
111011.rs.10.disp19 LWUPC
111011.rs.110.---disp18 LDPC
111011.rs.1110.---imm17 reserved
111011.rs.0.--imm16 AUIPC
111011.rs.1.--imm16 ALUIPC

I couldn't come up with better name having reasonable length, any
suggestions are welcome.

Thanks,
Leon




Re: [Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families

2014-10-13 Thread Yongbok Kim

I have just few minor comments.

Reviewed-by: Yongbok Kim yongbok@imgtec.com

regards,
Yongbok

On 08/10/2014 11:55, Leon Alrae wrote:

Signed-off-by: Leon Alrae leon.al...@imgtec.com
---
v3:
* use sextract32 instead of open coding the bit field extraction
* replace _i64 with _tl in DAHI, DATI and DAUI
* fix misleading LDPC comment
---
  disas/mips.c|  42 ++-
  target-mips/translate.c | 197 +---
  2 files changed, 225 insertions(+), 14 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index 8234369..091f4e2 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -407,6 +407,12 @@ struct mips_opcode
 +3 UDI immediate bits 6-20
 +4 UDI immediate bits 6-25
  
+   R6 immediates/displacements :

+   (adding suffix to 'o' to avoid adding new characters)
+   +o  9 bits immediate/displacement (shift = 7)
+   +o1 18 bits immediate/displacement (shift = 0)
+   +o2 19 bits immediate/displacement (shift = 0)
+
 Other:
 () parens surrounding optional value
 ,  separates operands
@@ -1217,6 +1223,17 @@ const struct mips_opcode mips_builtin_opcodes[] =
 them first.  The assemblers uses a hash table based on the
 instruction name anyhow.  */
  /* name,args, match,  mask,   pinfo,  
membership */
+{lwpc,s,+o2,0xec08, 0xfc18, WR_d, 0, 
I32R6},
+{lwupc,   s,+o2,0xec10, 0xfc18, WR_d, 0, 
I32R6},


I64R6


+{ldpc,s,+o1,0xec18, 0xfc1c, WR_d, 0, 
I64R6},
+{addiupc, s,+o2,0xec00, 0xfc18, WR_d, 0, 
I32R6},
+{auipc,   s,u,  0xec1e, 0xfc1f, WR_d, 0, 
I32R6},
+{aluipc,  s,u,  0xec1f, 0xfc1f, WR_d, 0, 
I32R6},
+{daui,s,t,u,0x7400, 0xfc00, RD_s|WR_t,0, 
I64R6},
+{dahi,s,u,  0x0406, 0xfc1f, RD_s, 0, 
I64R6},
+{dati,s,u,  0x041e, 0xfc1f, RD_s, 0, 
I64R6},
+{lsa, d,s,t,0x0005, 0xfc00073f, WR_d|RD_s|RD_t,   0, 
I32R6},
+{dlsa,d,s,t,0x0015, 0xfc00073f, WR_d|RD_s|RD_t,   0, 
I64R6},
  {clz, U,s,  0x0050, 0xfc1f07ff, WR_d|RD_s,0, 
I32R6},
  {clo, U,s,  0x0051, 0xfc1f07ff, WR_d|RD_s,0, 
I32R6},
  {dclz,U,s,  0x0052, 0xfc1f07ff, WR_d|RD_s,0, 
I64R6},
@@ -1822,6 +1839,7 @@ const struct mips_opcode mips_builtin_opcodes[] =
  {lld, t,o(b), 0xd000, 0xfc00, LDD|RD_b|WR_t,  0,  
I3  },
  {lld, t,A(b), 0,(int) M_LLD_AB,   INSN_MACRO, 0,  
I3  },
  {lui, t,u,0x3c00, 0xffe0, WR_t,   0,  
I1  },
+{aui, s,t,u,0x3c00, 0xfc00, RD_s|WR_t,0, 
I32R6},
  {luxc1,   D,t(b), 0x4c05, 0xfc00f83f, LDD|WR_D|RD_t|RD_b|FP_D, 0, 
I5|I33|N55},
  {lw,  t,o(b), 0x8c00, 0xfc00, LDD|RD_b|WR_t,  0,  
I1  },
  {lw,  t,A(b), 0,(int) M_LW_AB,INSN_MACRO, 0,  
I1  },
@@ -3645,10 +3663,28 @@ print_insn_args (const char *d,
  break;
  
  case 'o':

-delta = (l  OP_SH_DELTA_R6)  OP_MASK_DELTA_R6;
-if (delta  0x8000) {
-delta |= ~0x;
+switch (*(d+1)) {
+case '1':
+d++;
+delta = l  ((1  18) - 1);
+if (delta  0x2) {
+delta |= ~0x1;
+}
+break;
+case '2':
+d++;
+delta = l  ((1  19) - 1);
+if (delta  0x4) {
+delta |= ~0x3;
+}
+break;
+default:
+delta = (l  OP_SH_DELTA_R6)  OP_MASK_DELTA_R6;
+if (delta  0x8000) {
+delta |= ~0x;
+}
  }
+
  (*info-fprintf_func) (info-stream, %d, delta);
  break;
  
diff --git a/target-mips/translate.c b/target-mips/translate.c

index 06ececb..6f64c47 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -75,6 +75,7 @@ enum {
  OPC_BGTZ = (0x07  26),
  OPC_BGTZL= (0x17  26),
  OPC_JALX = (0x1D  26),  /* MIPS 16 only */
+OPC_DAUI = (0x1D  26),
  OPC_JALXS= OPC_JALX | 0x5,
  /* Load and stores */
  OPC_LDL  = (0x1A  26),
@@ -141,8 +142,25 @@ enum {
  /* Cache and prefetch */
  OPC_CACHE= (0x2F  26),
  OPC_PREF = (0x33  26),
-/* Reserved major opcode */
-OPC_MAJOR3B_RESERVED = (0x3B  26),
+/* PC-relative address computation / loads */
+OPC_PCREL= (0x3B  26),
+};
+
+/* 

[Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families

2014-10-08 Thread Leon Alrae
Signed-off-by: Leon Alrae leon.al...@imgtec.com
---
v3:
* use sextract32 instead of open coding the bit field extraction
* replace _i64 with _tl in DAHI, DATI and DAUI
* fix misleading LDPC comment
---
 disas/mips.c|  42 ++-
 target-mips/translate.c | 197 +---
 2 files changed, 225 insertions(+), 14 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index 8234369..091f4e2 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -407,6 +407,12 @@ struct mips_opcode
+3 UDI immediate bits 6-20
+4 UDI immediate bits 6-25
 
+   R6 immediates/displacements :
+   (adding suffix to 'o' to avoid adding new characters)
+   +o  9 bits immediate/displacement (shift = 7)
+   +o1 18 bits immediate/displacement (shift = 0)
+   +o2 19 bits immediate/displacement (shift = 0)
+
Other:
() parens surrounding optional value
,  separates operands
@@ -1217,6 +1223,17 @@ const struct mips_opcode mips_builtin_opcodes[] =
them first.  The assemblers uses a hash table based on the
instruction name anyhow.  */
 /* name,args,  match,  mask,   pinfo,  
membership */
+{lwpc,s,+o2,0xec08, 0xfc18, WR_d, 0, 
I32R6},
+{lwupc,   s,+o2,0xec10, 0xfc18, WR_d, 0, 
I32R6},
+{ldpc,s,+o1,0xec18, 0xfc1c, WR_d, 0, 
I64R6},
+{addiupc, s,+o2,0xec00, 0xfc18, WR_d, 0, 
I32R6},
+{auipc,   s,u,  0xec1e, 0xfc1f, WR_d, 0, 
I32R6},
+{aluipc,  s,u,  0xec1f, 0xfc1f, WR_d, 0, 
I32R6},
+{daui,s,t,u,0x7400, 0xfc00, RD_s|WR_t,0, 
I64R6},
+{dahi,s,u,  0x0406, 0xfc1f, RD_s, 0, 
I64R6},
+{dati,s,u,  0x041e, 0xfc1f, RD_s, 0, 
I64R6},
+{lsa, d,s,t,0x0005, 0xfc00073f, WR_d|RD_s|RD_t,   0, 
I32R6},
+{dlsa,d,s,t,0x0015, 0xfc00073f, WR_d|RD_s|RD_t,   0, 
I64R6},
 {clz, U,s,  0x0050, 0xfc1f07ff, WR_d|RD_s,0, 
I32R6},
 {clo, U,s,  0x0051, 0xfc1f07ff, WR_d|RD_s,0, 
I32R6},
 {dclz,U,s,  0x0052, 0xfc1f07ff, WR_d|RD_s,0, 
I64R6},
@@ -1822,6 +1839,7 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {lld,t,o(b),   0xd000, 0xfc00, LDD|RD_b|WR_t,  
0,  I3  },
 {lld, t,A(b),  0,(int) M_LLD_AB,   INSN_MACRO, 0,  
I3  },
 {lui, t,u, 0x3c00, 0xffe0, WR_t,   0,  
I1  },
+{aui, s,t,u,0x3c00, 0xfc00, RD_s|WR_t,0, 
I32R6},
 {luxc1,   D,t(b),  0x4c05, 0xfc00f83f, LDD|WR_D|RD_t|RD_b|FP_D, 0, 
I5|I33|N55},
 {lw,  t,o(b),  0x8c00, 0xfc00, LDD|RD_b|WR_t,  0,  
I1  },
 {lw,  t,A(b),  0,(int) M_LW_AB,INSN_MACRO, 0,  
I1  },
@@ -3645,10 +3663,28 @@ print_insn_args (const char *d,
  break;
 
 case 'o':
-delta = (l  OP_SH_DELTA_R6)  OP_MASK_DELTA_R6;
-if (delta  0x8000) {
-delta |= ~0x;
+switch (*(d+1)) {
+case '1':
+d++;
+delta = l  ((1  18) - 1);
+if (delta  0x2) {
+delta |= ~0x1;
+}
+break;
+case '2':
+d++;
+delta = l  ((1  19) - 1);
+if (delta  0x4) {
+delta |= ~0x3;
+}
+break;
+default:
+delta = (l  OP_SH_DELTA_R6)  OP_MASK_DELTA_R6;
+if (delta  0x8000) {
+delta |= ~0x;
+}
 }
+
 (*info-fprintf_func) (info-stream, %d, delta);
 break;
 
diff --git a/target-mips/translate.c b/target-mips/translate.c
index 06ececb..6f64c47 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -75,6 +75,7 @@ enum {
 OPC_BGTZ = (0x07  26),
 OPC_BGTZL= (0x17  26),
 OPC_JALX = (0x1D  26),  /* MIPS 16 only */
+OPC_DAUI = (0x1D  26),
 OPC_JALXS= OPC_JALX | 0x5,
 /* Load and stores */
 OPC_LDL  = (0x1A  26),
@@ -141,8 +142,25 @@ enum {
 /* Cache and prefetch */
 OPC_CACHE= (0x2F  26),
 OPC_PREF = (0x33  26),
-/* Reserved major opcode */
-OPC_MAJOR3B_RESERVED = (0x3B  26),
+/* PC-relative address computation / loads */
+OPC_PCREL= (0x3B  26),
+};
+
+/* PC-relative address computation / loads  */
+#define MASK_OPC_PCREL_TOP2BITS(op)  (MASK_OP_MAJOR(op) | (op  (3  19)))
+#define MASK_OPC_PCREL_TOP5BITS(op)  (MASK_OP_MAJOR(op) | (op  (0x1f  16)))
+enum {
+