[Qemu-devel] [PATCH 0/4] target-mips: fix warnings reported by clang

[Qemu-devel] [PATCH 0/4] target-mips: fix warnings reported by clang

[Leon Alrae]

Hi,

A few patches fixing warnings reported by clang:

target-mips/translate.c:1968:15: warning: no case matching constant switch
condition '16'
disas/mips.c:3804:27: warning: unused variable 'mips_msa_control_names_numeric'
[-Wunused-const-variable]
disas/mips.c:3515:27: warning: unused variable 'mips16_to_32_reg_map'
[-Wunused-const-variable]
target-mips/helper.c:393:27: warning: unused variable 'excp_names'
[-Wunused-const-variable]

Patch #3 in the series additionally removes block of code guarded by #if 0 that
has never been used since it was introduced.

Regards,
Leon

Leon Alrae (4):
  target-mips: convert single case switch into if statement
  disas/mips: remove unused mips_msa_control_names_numeric[32]
  disas/mips: remove unused mips16_to_32_reg_map[]
  target-mips: remove excp_names[] from linux-user as it is unused

 disas/mips.c            | 728 ------------------------------------------------
 target-mips/helper.c    |   2 +-
 target-mips/translate.c |   4 +-
 3 files changed, 2 insertions(+), 732 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 1/4] target-mips: convert single case switch into if statement

[Leon Alrae]

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/translate.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index f65ed84..1205909 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -1882,10 +1882,8 @@ static inline void gen_r6_cmp_ ## fmt(DisasContext * ctx, int n,        \
 {                                                                       \
     TCGv_i ## bits fp0 = tcg_temp_new_i ## bits();                      \
     TCGv_i ## bits fp1 = tcg_temp_new_i ## bits();                      \
-    switch (ifmt) {                                                     \
-    case FMT_D:                                                         \
+    if (ifmt == FMT_D) {                                                \
         check_cp1_registers(ctx, fs | ft | fd);                         \
-        break;                                                          \
     }                                                                   \
     gen_ldcmp_fpr ## bits(ctx, fp0, fs);                                \
     gen_ldcmp_fpr ## bits(ctx, fp1, ft);                                \
-- 
2.1.0

Re: [Qemu-devel] [PATCH 1/4] target-mips: convert single case switch into if statement

[Peter Maydell]

On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  target-mips/translate.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 2/4] disas/mips: remove unused mips_msa_control_names_numeric[32]

[Leon Alrae]

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 disas/mips.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index 2614c52..b94d5d9 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -3801,13 +3801,6 @@ static const char * const mips_hwr_names_mips3264r2[32] =
   "$24",  "$25",  "$26",  "$27",  "$28",  "$29",  "$30",  "$31"
 };
 
-static const char * const mips_msa_control_names_numeric[32] = {
-  "$0",   "$1",   "$2",   "$3",   "$4",   "$5",   "$6",   "$7",
-  "$8",   "$9",   "$10",  "$11",  "$12",  "$13",  "$14",  "$15",
-  "$16",  "$17",  "$18",  "$19",  "$20",  "$21",  "$22",  "$23",
-  "$24",  "$25",  "$26",  "$27",  "$28",  "$29",  "$30",  "$31"
-};
-
 static const char * const mips_msa_control_names_mips3264r2[32] = {
   "MSAIR", "MSACSR", "$2", "$3",  "$4",   "$5",   "$6",   "$7",
   "$8",   "$9",   "$10",  "$11",  "$12",  "$13",  "$14",  "$15",
-- 
2.1.0

Re: [Qemu-devel] [PATCH 2/4] disas/mips: remove unused mips_msa_control_names_numeric[32]

[Peter Maydell]

On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  disas/mips.c | 7 -------
>  1 file changed, 7 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Leon Alrae]

The array was "used" in a block of code which has never been enabled. Therefore
removing the array as well as 700 lines of never used code.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 disas/mips.c | 721 -----------------------------------------------------------
 1 file changed, 721 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index b94d5d9..b10dcf5 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -3511,15 +3511,6 @@ struct mips_cp0sel_name
   const char * const name;
 };
 
-/* The mips16 registers.  */
-static const unsigned int mips16_to_32_reg_map[] =
-{
-  16, 17, 2, 3, 4, 5, 6, 7
-};
-
-#define mips16_reg_names(rn)	mips_gpr_names[mips16_to_32_reg_map[rn]]
-
-
 static const char * const mips_gpr_names_numeric[32] =
 {
   "$0",   "$1",   "$2",   "$3",   "$4",   "$5",   "$6",   "$7",
@@ -5075,715 +5066,3 @@ print_insn_little_mips (bfd_vma memaddr, struct disassemble_info *info)
 {
   return _print_insn_mips (memaddr, info, BFD_ENDIAN_LITTLE);
 }
-\f

-/* Disassemble mips16 instructions.  */
-#if 0
-static int
-print_insn_mips16 (bfd_vma memaddr, struct disassemble_info *info)
-{
-  int status;
-  bfd_byte buffer[2];
-  int length;
-  int insn;
-  bfd_boolean use_extend;
-  int extend = 0;
-  const struct mips_opcode *op, *opend;
-
-  info->bytes_per_chunk = 2;
-  info->display_endian = info->endian;
-  info->insn_info_valid = 1;
-  info->branch_delay_insns = 0;
-  info->data_size = 0;
-  info->insn_type = dis_nonbranch;
-  info->target = 0;
-  info->target2 = 0;
-
-  status = (*info->read_memory_func) (memaddr, buffer, 2, info);
-  if (status != 0)
-    {
-      (*info->memory_error_func) (status, memaddr, info);
-      return -1;
-    }
-
-  length = 2;
-
-  if (info->endian == BFD_ENDIAN_BIG)
-    insn = bfd_getb16 (buffer);
-  else
-    insn = bfd_getl16 (buffer);
-
-  /* Handle the extend opcode specially.  */
-  use_extend = FALSE;
-  if ((insn & 0xf800) == 0xf000)
-    {
-      use_extend = TRUE;
-      extend = insn & 0x7ff;
-
-      memaddr += 2;
-
-      status = (*info->read_memory_func) (memaddr, buffer, 2, info);
-      if (status != 0)
-	{
-	  (*info->fprintf_func) (info->stream, "extend 0x%x",
-				 (unsigned int) extend);
-	  (*info->memory_error_func) (status, memaddr, info);
-	  return -1;
-	}
-
-      if (info->endian == BFD_ENDIAN_BIG)
-	insn = bfd_getb16 (buffer);
-      else
-	insn = bfd_getl16 (buffer);
-
-      /* Check for an extend opcode followed by an extend opcode.  */
-      if ((insn & 0xf800) == 0xf000)
-	{
-	  (*info->fprintf_func) (info->stream, "extend 0x%x",
-				 (unsigned int) extend);
-	  info->insn_type = dis_noninsn;
-	  return length;
-	}
-
-      length += 2;
-    }
-
-  /* FIXME: Should probably use a hash table on the major opcode here.  */
-
-  opend = mips16_opcodes + bfd_mips16_num_opcodes;
-  for (op = mips16_opcodes; op < opend; op++)
-    {
-      if (op->pinfo != INSN_MACRO
-	  && !(no_aliases && (op->pinfo2 & INSN2_ALIAS))
-	  && (insn & op->mask) == op->match)
-	{
-	  const char *s;
-
-	  if (strchr (op->args, 'a') != NULL)
-	    {
-	      if (use_extend)
-		{
-		  (*info->fprintf_func) (info->stream, "extend 0x%x",
-					 (unsigned int) extend);
-		  info->insn_type = dis_noninsn;
-		  return length - 2;
-		}
-
-	      use_extend = FALSE;
-
-	      memaddr += 2;
-
-	      status = (*info->read_memory_func) (memaddr, buffer, 2,
-						  info);
-	      if (status == 0)
-		{
-		  use_extend = TRUE;
-		  if (info->endian == BFD_ENDIAN_BIG)
-		    extend = bfd_getb16 (buffer);
-		  else
-		    extend = bfd_getl16 (buffer);
-		  length += 2;
-		}
-	    }
-
-	  (*info->fprintf_func) (info->stream, "%s", op->name);
-	  if (op->args[0] != '\0')
-	    (*info->fprintf_func) (info->stream, "\t");
-
-	  for (s = op->args; *s != '\0'; s++)
-	    {
-	      if (*s == ','
-		  && s[1] == 'w'
-		  && (((insn >> MIPS16OP_SH_RX) & MIPS16OP_MASK_RX)
-		      == ((insn >> MIPS16OP_SH_RY) & MIPS16OP_MASK_RY)))
-		{
-		  /* Skip the register and the comma.  */
-		  ++s;
-		  continue;
-		}
-	      if (*s == ','
-		  && s[1] == 'v'
-		  && (((insn >> MIPS16OP_SH_RZ) & MIPS16OP_MASK_RZ)
-		      == ((insn >> MIPS16OP_SH_RX) & MIPS16OP_MASK_RX)))
-		{
-		  /* Skip the register and the comma.  */
-		  ++s;
-		  continue;
-		}
-	      print_mips16_insn_arg (*s, op, insn, use_extend, extend, memaddr,
-				     info);
-	    }
-
-	  if ((op->pinfo & INSN_UNCOND_BRANCH_DELAY) != 0)
-	    {
-	      info->branch_delay_insns = 1;
-	      if (info->insn_type != dis_jsr)
-		info->insn_type = dis_branch;
-	    }
-
-	  return length;
-	}
-    }
-
-  if (use_extend)
-    (*info->fprintf_func) (info->stream, "0x%x", extend | 0xf000);
-  (*info->fprintf_func) (info->stream, "0x%x", insn);
-  info->insn_type = dis_noninsn;
-
-  return length;
-}
-
-/* Disassemble an operand for a mips16 instruction.  */
-
-static void
-print_mips16_insn_arg (char type,
-		       const struct mips_opcode *op,
-		       int l,
-		       bfd_boolean use_extend,
-		       int extend,
-		       bfd_vma memaddr,
-		       struct disassemble_info *info)
-{
-  switch (type)
-    {
-    case ',':
-    case '(':
-    case ')':
-      (*info->fprintf_func) (info->stream, "%c", type);
-      break;
-
-    case 'y':
-    case 'w':
-      (*info->fprintf_func) (info->stream, "%s",
-			     mips16_reg_names(((l >> MIPS16OP_SH_RY)
-					       & MIPS16OP_MASK_RY)));
-      break;
-
-    case 'x':
-    case 'v':
-      (*info->fprintf_func) (info->stream, "%s",
-			     mips16_reg_names(((l >> MIPS16OP_SH_RX)
-					       & MIPS16OP_MASK_RX)));
-      break;
-
-    case 'z':
-      (*info->fprintf_func) (info->stream, "%s",
-			     mips16_reg_names(((l >> MIPS16OP_SH_RZ)
-					       & MIPS16OP_MASK_RZ)));
-      break;
-
-    case 'Z':
-      (*info->fprintf_func) (info->stream, "%s",
-			     mips16_reg_names(((l >> MIPS16OP_SH_MOVE32Z)
-					       & MIPS16OP_MASK_MOVE32Z)));
-      break;
-
-    case '0':
-      (*info->fprintf_func) (info->stream, "%s", mips_gpr_names[0]);
-      break;
-
-    case 'S':
-      (*info->fprintf_func) (info->stream, "%s", mips_gpr_names[29]);
-      break;
-
-    case 'P':
-      (*info->fprintf_func) (info->stream, "$pc");
-      break;
-
-    case 'R':
-      (*info->fprintf_func) (info->stream, "%s", mips_gpr_names[31]);
-      break;
-
-    case 'X':
-      (*info->fprintf_func) (info->stream, "%s",
-			     mips_gpr_names[((l >> MIPS16OP_SH_REGR32)
-					    & MIPS16OP_MASK_REGR32)]);
-      break;
-
-    case 'Y':
-      (*info->fprintf_func) (info->stream, "%s",
-			     mips_gpr_names[MIPS16OP_EXTRACT_REG32R (l)]);
-      break;
-
-    case '<':
-    case '>':
-    case '[':
-    case ']':
-    case '4':
-    case '5':
-    case 'H':
-    case 'W':
-    case 'D':
-    case 'j':
-    case '6':
-    case '8':
-    case 'V':
-    case 'C':
-    case 'U':
-    case 'k':
-    case 'K':
-    case 'p':
-    case 'q':
-    case 'A':
-    case 'B':
-    case 'E':
-      {
-	int immed, nbits, shift, signedp, extbits, pcrel, extu, branch;
-
-	shift = 0;
-	signedp = 0;
-	extbits = 16;
-	pcrel = 0;
-	extu = 0;
-	branch = 0;
-	switch (type)
-	  {
-	  case '<':
-	    nbits = 3;
-	    immed = (l >> MIPS16OP_SH_RZ) & MIPS16OP_MASK_RZ;
-	    extbits = 5;
-	    extu = 1;
-	    break;
-	  case '>':
-	    nbits = 3;
-	    immed = (l >> MIPS16OP_SH_RX) & MIPS16OP_MASK_RX;
-	    extbits = 5;
-	    extu = 1;
-	    break;
-	  case '[':
-	    nbits = 3;
-	    immed = (l >> MIPS16OP_SH_RZ) & MIPS16OP_MASK_RZ;
-	    extbits = 6;
-	    extu = 1;
-	    break;
-	  case ']':
-	    nbits = 3;
-	    immed = (l >> MIPS16OP_SH_RX) & MIPS16OP_MASK_RX;
-	    extbits = 6;
-	    extu = 1;
-	    break;
-	  case '4':
-	    nbits = 4;
-	    immed = (l >> MIPS16OP_SH_IMM4) & MIPS16OP_MASK_IMM4;
-	    signedp = 1;
-	    extbits = 15;
-	    break;
-	  case '5':
-	    nbits = 5;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    info->insn_type = dis_dref;
-	    info->data_size = 1;
-	    break;
-	  case 'H':
-	    nbits = 5;
-	    shift = 1;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    info->insn_type = dis_dref;
-	    info->data_size = 2;
-	    break;
-	  case 'W':
-	    nbits = 5;
-	    shift = 2;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    if ((op->pinfo & MIPS16_INSN_READ_PC) == 0
-		&& (op->pinfo & MIPS16_INSN_READ_SP) == 0)
-	      {
-		info->insn_type = dis_dref;
-		info->data_size = 4;
-	      }
-	    break;
-	  case 'D':
-	    nbits = 5;
-	    shift = 3;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    info->insn_type = dis_dref;
-	    info->data_size = 8;
-	    break;
-	  case 'j':
-	    nbits = 5;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    signedp = 1;
-	    break;
-	  case '6':
-	    nbits = 6;
-	    immed = (l >> MIPS16OP_SH_IMM6) & MIPS16OP_MASK_IMM6;
-	    break;
-	  case '8':
-	    nbits = 8;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    break;
-	  case 'V':
-	    nbits = 8;
-	    shift = 2;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    /* FIXME: This might be lw, or it might be addiu to $sp or
-               $pc.  We assume it's load.  */
-	    info->insn_type = dis_dref;
-	    info->data_size = 4;
-	    break;
-	  case 'C':
-	    nbits = 8;
-	    shift = 3;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    info->insn_type = dis_dref;
-	    info->data_size = 8;
-	    break;
-	  case 'U':
-	    nbits = 8;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    extu = 1;
-	    break;
-	  case 'k':
-	    nbits = 8;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    signedp = 1;
-	    break;
-	  case 'K':
-	    nbits = 8;
-	    shift = 3;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    signedp = 1;
-	    break;
-	  case 'p':
-	    nbits = 8;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    signedp = 1;
-	    pcrel = 1;
-	    branch = 1;
-	    info->insn_type = dis_condbranch;
-	    break;
-	  case 'q':
-	    nbits = 11;
-	    immed = (l >> MIPS16OP_SH_IMM11) & MIPS16OP_MASK_IMM11;
-	    signedp = 1;
-	    pcrel = 1;
-	    branch = 1;
-	    info->insn_type = dis_branch;
-	    break;
-	  case 'A':
-	    nbits = 8;
-	    shift = 2;
-	    immed = (l >> MIPS16OP_SH_IMM8) & MIPS16OP_MASK_IMM8;
-	    pcrel = 1;
-	    /* FIXME: This can be lw or la.  We assume it is lw.  */
-	    info->insn_type = dis_dref;
-	    info->data_size = 4;
-	    break;
-	  case 'B':
-	    nbits = 5;
-	    shift = 3;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    pcrel = 1;
-	    info->insn_type = dis_dref;
-	    info->data_size = 8;
-	    break;
-	  case 'E':
-	    nbits = 5;
-	    shift = 2;
-	    immed = (l >> MIPS16OP_SH_IMM5) & MIPS16OP_MASK_IMM5;
-	    pcrel = 1;
-	    break;
-	  default:
-	    abort ();
-	  }
-
-	if (! use_extend)
-	  {
-	    if (signedp && immed >= (1 << (nbits - 1)))
-	      immed -= 1 << nbits;
-	    immed <<= shift;
-	    if ((type == '<' || type == '>' || type == '[' || type == ']')
-		&& immed == 0)
-	      immed = 8;
-	  }
-	else
-	  {
-	    if (extbits == 16)
-	      immed |= ((extend & 0x1f) << 11) | (extend & 0x7e0);
-	    else if (extbits == 15)
-	      immed |= ((extend & 0xf) << 11) | (extend & 0x7f0);
-	    else
-	      immed = ((extend >> 6) & 0x1f) | (extend & 0x20);
-	    immed &= (1 << extbits) - 1;
-	    if (! extu && immed >= (1 << (extbits - 1)))
-	      immed -= 1 << extbits;
-	  }
-
-	if (! pcrel)
-	  (*info->fprintf_func) (info->stream, "%d", immed);
-	else
-	  {
-	    bfd_vma baseaddr;
-
-	    if (branch)
-	      {
-		immed *= 2;
-		baseaddr = memaddr + 2;
-	      }
-	    else if (use_extend)
-	      baseaddr = memaddr - 2;
-	    else
-	      {
-		int status;
-		bfd_byte buffer[2];
-
-		baseaddr = memaddr;
-
-		/* If this instruction is in the delay slot of a jr
-                   instruction, the base address is the address of the
-                   jr instruction.  If it is in the delay slot of jalr
-                   instruction, the base address is the address of the
-                   jalr instruction.  This test is unreliable: we have
-                   no way of knowing whether the previous word is
-                   instruction or data.  */
-		status = (*info->read_memory_func) (memaddr - 4, buffer, 2,
-						    info);
-		if (status == 0
-		    && (((info->endian == BFD_ENDIAN_BIG
-			  ? bfd_getb16 (buffer)
-			  : bfd_getl16 (buffer))
-			 & 0xf800) == 0x1800))
-		  baseaddr = memaddr - 4;
-		else
-		  {
-		    status = (*info->read_memory_func) (memaddr - 2, buffer,
-							2, info);
-		    if (status == 0
-			&& (((info->endian == BFD_ENDIAN_BIG
-			      ? bfd_getb16 (buffer)
-			      : bfd_getl16 (buffer))
-			     & 0xf81f) == 0xe800))
-		      baseaddr = memaddr - 2;
-		  }
-	      }
-	    info->target = (baseaddr & ~((1 << shift) - 1)) + immed;
-	    if (pcrel && branch
-		&& info->flavour == bfd_target_unknown_flavour)
-	      /* For gdb disassembler, maintain odd address.  */
-	      info->target |= 1;
-	    (*info->print_address_func) (info->target, info);
-	  }
-      }
-      break;
-
-    case 'a':
-      {
-	int jalx = l & 0x400;
-
-	if (! use_extend)
-	  extend = 0;
-	l = ((l & 0x1f) << 23) | ((l & 0x3e0) << 13) | (extend << 2);
-	if (!jalx && info->flavour == bfd_target_unknown_flavour)
-	  /* For gdb disassembler, maintain odd address.  */
-	  l |= 1;
-      }
-      info->target = ((memaddr + 4) & ~(bfd_vma) 0x0fffffff) | l;
-      (*info->print_address_func) (info->target, info);
-      info->insn_type = dis_jsr;
-      info->branch_delay_insns = 1;
-      break;
-
-    case 'l':
-    case 'L':
-      {
-	int need_comma, amask, smask;
-
-	need_comma = 0;
-
-	l = (l >> MIPS16OP_SH_IMM6) & MIPS16OP_MASK_IMM6;
-
-	amask = (l >> 3) & 7;
-
-	if (amask > 0 && amask < 5)
-	  {
-	    (*info->fprintf_func) (info->stream, "%s", mips_gpr_names[4]);
-	    if (amask > 1)
-	      (*info->fprintf_func) (info->stream, "-%s",
-				     mips_gpr_names[amask + 3]);
-	    need_comma = 1;
-	  }
-
-	smask = (l >> 1) & 3;
-	if (smask == 3)
-	  {
-	    (*info->fprintf_func) (info->stream, "%s??",
-				   need_comma ? "," : "");
-	    need_comma = 1;
-	  }
-	else if (smask > 0)
-	  {
-	    (*info->fprintf_func) (info->stream, "%s%s",
-				   need_comma ? "," : "",
-				   mips_gpr_names[16]);
-	    if (smask > 1)
-	      (*info->fprintf_func) (info->stream, "-%s",
-				     mips_gpr_names[smask + 15]);
-	    need_comma = 1;
-	  }
-
-	if (l & 1)
-	  {
-	    (*info->fprintf_func) (info->stream, "%s%s",
-				   need_comma ? "," : "",
-				   mips_gpr_names[31]);
-	    need_comma = 1;
-	  }
-
-	if (amask == 5 || amask == 6)
-	  {
-	    (*info->fprintf_func) (info->stream, "%s$f0",
-				   need_comma ? "," : "");
-	    if (amask == 6)
-	      (*info->fprintf_func) (info->stream, "-$f1");
-	  }
-      }
-      break;
-
-    case 'm':
-    case 'M':
-      /* MIPS16e save/restore.  */
-      {
-      int need_comma = 0;
-      int amask, args, statics;
-      int nsreg, smask;
-      int framesz;
-      int i, j;
-
-      l = l & 0x7f;
-      if (use_extend)
-        l |= extend << 16;
-
-      amask = (l >> 16) & 0xf;
-      if (amask == MIPS16_ALL_ARGS)
-        {
-          args = 4;
-          statics = 0;
-        }
-      else if (amask == MIPS16_ALL_STATICS)
-        {
-          args = 0;
-          statics = 4;
-        }
-      else
-        {
-          args = amask >> 2;
-          statics = amask & 3;
-        }
-
-      if (args > 0) {
-          (*info->fprintf_func) (info->stream, "%s", mips_gpr_names[4]);
-          if (args > 1)
-            (*info->fprintf_func) (info->stream, "-%s",
-                                   mips_gpr_names[4 + args - 1]);
-          need_comma = 1;
-      }
-
-      framesz = (((l >> 16) & 0xf0) | (l & 0x0f)) * 8;
-      if (framesz == 0 && !use_extend)
-        framesz = 128;
-
-      (*info->fprintf_func) (info->stream, "%s%d",
-                             need_comma ? "," : "",
-                             framesz);
-
-      if (l & 0x40)                   /* $ra */
-        (*info->fprintf_func) (info->stream, ",%s", mips_gpr_names[31]);
-
-      nsreg = (l >> 24) & 0x7;
-      smask = 0;
-      if (l & 0x20)                   /* $s0 */
-        smask |= 1 << 0;
-      if (l & 0x10)                   /* $s1 */
-        smask |= 1 << 1;
-      if (nsreg > 0)                  /* $s2-$s8 */
-        smask |= ((1 << nsreg) - 1) << 2;
-
-      /* Find first set static reg bit.  */
-      for (i = 0; i < 9; i++)
-        {
-          if (smask & (1 << i))
-            {
-              (*info->fprintf_func) (info->stream, ",%s",
-                                     mips_gpr_names[i == 8 ? 30 : (16 + i)]);
-              /* Skip over string of set bits.  */
-              for (j = i; smask & (2 << j); j++)
-                continue;
-              if (j > i)
-                (*info->fprintf_func) (info->stream, "-%s",
-                                       mips_gpr_names[j == 8 ? 30 : (16 + j)]);
-              i = j + 1;
-            }
-        }
-
-      /* Statics $ax - $a3.  */
-      if (statics == 1)
-        (*info->fprintf_func) (info->stream, ",%s", mips_gpr_names[7]);
-      else if (statics > 0)
-        (*info->fprintf_func) (info->stream, ",%s-%s",
-                               mips_gpr_names[7 - statics + 1],
-                               mips_gpr_names[7]);
-      }
-      break;
-
-    default:
-      /* xgettext:c-format */
-      (*info->fprintf_func)
-	(info->stream,
-	 _("# internal disassembler error, unrecognised modifier (%c)"),
-	 type);
-      abort ();
-    }
-}
-
-void
-print_mips_disassembler_options (FILE *stream)
-{
-  unsigned int i;
-
-  fprintf (stream, _("\n\
-The following MIPS specific disassembler options are supported for use\n\
-with the -M switch (multiple options should be separated by commas):\n"));
-
-  fprintf (stream, _("\n\
-  gpr-names=ABI            Print GPR names according to  specified ABI.\n\
-                           Default: based on binary being disassembled.\n"));
-
-  fprintf (stream, _("\n\
-  fpr-names=ABI            Print FPR names according to specified ABI.\n\
-                           Default: numeric.\n"));
-
-  fprintf (stream, _("\n\
-  cp0-names=ARCH           Print CP0 register names according to\n\
-                           specified architecture.\n\
-                           Default: based on binary being disassembled.\n"));
-
-  fprintf (stream, _("\n\
-  hwr-names=ARCH           Print HWR names according to specified\n\
-			   architecture.\n\
-                           Default: based on binary being disassembled.\n"));
-
-  fprintf (stream, _("\n\
-  reg-names=ABI            Print GPR and FPR names according to\n\
-                           specified ABI.\n"));
-
-  fprintf (stream, _("\n\
-  reg-names=ARCH           Print CP0 register and HWR names according to\n\
-                           specified architecture.\n"));
-
-  fprintf (stream, _("\n\
-  For the options above, the following values are supported for \"ABI\":\n\
-   "));
-  for (i = 0; i < ARRAY_SIZE (mips_abi_choices); i++)
-    fprintf (stream, " %s", mips_abi_choices[i].name);
-  fprintf (stream, _("\n"));
-
-  fprintf (stream, _("\n\
-  For the options above, The following values are supported for \"ARCH\":\n\
-   "));
-  for (i = 0; i < ARRAY_SIZE (mips_arch_choices); i++)
-    if (*mips_arch_choices[i].name != '\0')
-      fprintf (stream, " %s", mips_arch_choices[i].name);
-  fprintf (stream, _("\n"));
-
-  fprintf (stream, _("\n"));
-}
-#endif
-- 
2.1.0

Re: [Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Peter Maydell]

On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
> The array was "used" in a block of code which has never been enabled. Therefore
> removing the array as well as 700 lines of never used code.

That's quite a lot to remove -- are you sure we're never going to want
to disassemble mips16 insns in future?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Leon Alrae]

On 12/12/2014 12:25, Peter Maydell wrote:
> On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> The array was "used" in a block of code which has never been enabled. Therefore
>> removing the array as well as 700 lines of never used code.
> 
> That's quite a lot to remove -- are you sure we're never going to want
> to disassemble mips16 insns in future?

I quickly tried to enable it but it doesn't seem to be complete. For
example it requires "extern const struct mips_opcode mips16_opcodes[]"
which isn't anywhere in the codebase. Moreover, according to the git
blame it has always been disabled since 2005 thus in my opinion there is
no point in keeping this code. I do want to disassemble mips16 insns, if
someone has proper implementation I'm more than happy to include it.

Regards,
Leon

Re: [Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Peter Maydell]

On 12 December 2014 at 14:28, Leon Alrae <leon.alrae@imgtec.com> wrote:
> On 12/12/2014 12:25, Peter Maydell wrote:
>> On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
>>> The array was "used" in a block of code which has never been enabled. Therefore
>>> removing the array as well as 700 lines of never used code.
>>
>> That's quite a lot to remove -- are you sure we're never going to want
>> to disassemble mips16 insns in future?
>
> I quickly tried to enable it but it doesn't seem to be complete. For
> example it requires "extern const struct mips_opcode mips16_opcodes[]"
> which isn't anywhere in the codebase. Moreover, according to the git
> blame it has always been disabled since 2005 thus in my opinion there is
> no point in keeping this code. I do want to disassemble mips16 insns, if
> someone has proper implementation I'm more than happy to include it.

The trouble (as you may be aware) is that the obvious choice for
an implementation is upstream binutils, except that is GPLv3
whereas we are stuck with the disassemblers from the last GPLv2
binutils release, for license compatibility reasons.

If you (by which I mean Imagination) contributed the MIPS
disassemblers to binutils in the first place then you may
be able to provide QEMU with a GPLv2 version (effectively
dual-licensing your own code). Ask your legal people for
advice :-)

Failing that, the best source for a mips16 disassembler is
going to be "fix the bugs in the currently disabled code we
have in the tree", in which case deleting it all would be
a backwards step...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Leon Alrae]

On 12/12/2014 14:51, Peter Maydell wrote:
> On 12 December 2014 at 14:28, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> On 12/12/2014 12:25, Peter Maydell wrote:
>>> On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
>>>> The array was "used" in a block of code which has never been enabled. Therefore
>>>> removing the array as well as 700 lines of never used code.
>>>
>>> That's quite a lot to remove -- are you sure we're never going to want
>>> to disassemble mips16 insns in future?
>>
>> I quickly tried to enable it but it doesn't seem to be complete. For
>> example it requires "extern const struct mips_opcode mips16_opcodes[]"
>> which isn't anywhere in the codebase. Moreover, according to the git
>> blame it has always been disabled since 2005 thus in my opinion there is
>> no point in keeping this code. I do want to disassemble mips16 insns, if
>> someone has proper implementation I'm more than happy to include it.
> 
> The trouble (as you may be aware) is that the obvious choice for
> an implementation is upstream binutils, except that is GPLv3
> whereas we are stuck with the disassemblers from the last GPLv2
> binutils release, for license compatibility reasons.
> 
> If you (by which I mean Imagination) contributed the MIPS
> disassemblers to binutils in the first place then you may
> be able to provide QEMU with a GPLv2 version (effectively
> dual-licensing your own code). Ask your legal people for
> advice :-)

I see, so updating disassemblers in QEMU using binutils implementation
might not be as simple as I was assuming.

> 
> Failing that, the best source for a mips16 disassembler is
> going to be "fix the bugs in the currently disabled code we
> have in the tree", in which case deleting it all would be
> a backwards step...

Agreed. It may have greater value than I thought. I'll leave it then.

Thanks for reviewing and clarifying.

Regards,
Leon

Re: [Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Peter Maydell]

On 12 December 2014 at 15:52, Leon Alrae <leon.alrae@imgtec.com> wrote:
> On 12/12/2014 14:51, Peter Maydell wrote:
>> Failing that, the best source for a mips16 disassembler is
>> going to be "fix the bugs in the currently disabled code we
>> have in the tree", in which case deleting it all would be
>> a backwards step...
>
> Agreed. It may have greater value than I thought. I'll leave it then.

You can silence the warning by sticking #if 0 round
the offending data (which is probably why the code has the
#if 0).

-- PMM

Re: [Qemu-devel] [PATCH 3/4] disas/mips: remove unused mips16_to_32_reg_map[]

[Leon Alrae]

On 12/12/2014 15:53, Peter Maydell wrote:
> On 12 December 2014 at 15:52, Leon Alrae <leon.alrae@imgtec.com> wrote:
>> On 12/12/2014 14:51, Peter Maydell wrote:
>>> Failing that, the best source for a mips16 disassembler is
>>> going to be "fix the bugs in the currently disabled code we
>>> have in the tree", in which case deleting it all would be
>>> a backwards step...
>>
>> Agreed. It may have greater value than I thought. I'll leave it then.
> 
> You can silence the warning by sticking #if 0 round
> the offending data (which is probably why the code has the
> #if 0).

Done in v2 of this patch.

Thanks,
Leon

[Qemu-devel] [PATCH 4/4] target-mips: remove excp_names[] from linux-user as it is unused

[Leon Alrae]

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-mips/helper.c b/target-mips/helper.c
index c4b3658..7d26705 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -388,7 +388,6 @@ hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int r
         return physical;
     }
 }
-#endif
 
 static const char * const excp_names[EXCP_LAST + 1] = {
     [EXCP_RESET] = "reset",
@@ -429,6 +428,7 @@ static const char * const excp_names[EXCP_LAST + 1] = {
     [EXCP_MSADIS] = "MSA disabled",
     [EXCP_MSAFPE] = "MSA floating point",
 };
+#endif
 
 target_ulong exception_resume_pc (CPUMIPSState *env)
 {
-- 
2.1.0

Re: [Qemu-devel] [PATCH 4/4] target-mips: remove excp_names[] from linux-user as it is unused

[Peter Maydell]

On 12 December 2014 at 09:30, Leon Alrae <leon.alrae@imgtec.com> wrote:
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  target-mips/helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM