This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Cleanup ppc code dealing with opcode dumps


I was looking at the dump of the opcode tables (ie, #define PRINT_OPCODE_TABLE),
and I noticed that we don't dump the entire tables.  Specifically, we don't
dump the first entry in each opcode table due to how we try and check for
sorted major opcodes.  Therefore, I rewrote ppc_setup_opcodes() a little so
that we now dump the entire tables.  I also fixed a typo in the calculation
of the VLE opcode index, which was subtracting off powerpc_opcodes rather
than vle_opcodes.

That made me look at the consumers of that information and I noticed that
the initialization of the spe2_opcd_indices array was being initialized
each time disassemble_init_powerpc() is called, instead of just once like
powerpc_opcd_indices and vle_opcd_indices, so I moved its init loop next
to the powerpc and vle init loops.  The init loops for those made my head
hurt trying to determine what exactly they were computing and how, so I
rewrote those to something simpler (at least to me).  I did verify that we
get exactly the same *_opcd_indices[] values as before the patch.

Alan, what do you think of the cleanups below?  Testsuite runs show no
regressions.

Peter


include/
	* opcode/ppc.h (powerpc_num_opcodes): Change type to unsigned.
	(vle_num_opcodes): Likewise.
	(spe2_num_opcodes): Likewise.

opcodes/
	* ppc-opc.c (powerpc_num_opcodes): Likewise.
	(vle_num_opcodes): Likewise.
	(spe2_num_opcodes): Likewise.
	* ppc-dis.c (disassemble_init_powerpc) <powerpc_opcd_indices>: Rewrite
	initialization loop.
	(disassemble_init_powerpc) <vle_opcd_indices>: Likewise.
	(disassemble_init_powerpc) <spe2_opcd_indices>: Likewise.  Initialize
	only once.

gas/
	* config/tc-ppc.c (ppc_setup_opcodes) <powerpc_opcodes>: Rewrite code
	to dump the entire opcode table.
	(ppc_setup_opcodes) <spe2_opcodes>: Likewise.
	(ppc_setup_opcodes) <vle_opcodes>: Likewise.  Fix calculation of
	opcode index.

diff --git a/include/opcode/ppc.h b/include/opcode/ppc.h
index 962f898227..f0b643792c 100644
--- a/include/opcode/ppc.h
+++ b/include/opcode/ppc.h
@@ -75,11 +75,11 @@ struct powerpc_opcode
    in the order in which the disassembler should consider
    instructions.  */
 extern const struct powerpc_opcode powerpc_opcodes[];
-extern const int powerpc_num_opcodes;
+extern const unsigned int powerpc_num_opcodes;
 extern const struct powerpc_opcode vle_opcodes[];
-extern const int vle_num_opcodes;
+extern const unsigned int vle_num_opcodes;
 extern const struct powerpc_opcode spe2_opcodes[];
-extern const int spe2_num_opcodes;
+extern const unsigned int spe2_num_opcodes;
 
 /* Values defined for the flags field of a struct powerpc_opcode.  */
 
diff --git a/opcodes/ppc-opc.c b/opcodes/ppc-opc.c
index bb17f26c2e..1527aa5f4f 100644
--- a/opcodes/ppc-opc.c
+++ b/opcodes/ppc-opc.c
@@ -7551,7 +7551,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
 {"fcfidu.",	XRC(63,974,1),	XRA_MASK, POWER7|PPCA2,	PPCVLE,		{FRT, FRB}},
 };
 
-const int powerpc_num_opcodes =
+const unsigned int powerpc_num_opcodes =
   sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
 
 /* The VLE opcode table.
@@ -8479,7 +8479,7 @@ const struct powerpc_opcode vle_opcodes[] = {
 {"se_bl",	BD8(58,0,1),	BD8_MASK,	PPCVLE,	0,		{B8}},
 };
 
-const int vle_num_opcodes =
+const unsigned int vle_num_opcodes =
   sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
 
 /* The macro table.  This is only used by the assembler.  */
@@ -9370,5 +9370,5 @@ const struct powerpc_opcode spe2_opcodes[] = {
 {"evavgdsr",		  VX (4, 1663),		VX_MASK,		PPCSPE2, 0, {RD, RA, RB}},
 };
 
-const int spe2_num_opcodes =
+const unsigned int spe2_num_opcodes =
   sizeof (spe2_opcodes) / sizeof (spe2_opcodes[0]);
diff --git a/opcodes/ppc-dis.c b/opcodes/ppc-dis.c
index efa7898e71..f8b0292c4e 100644
--- a/opcodes/ppc-dis.c
+++ b/opcodes/ppc-dis.c
@@ -380,60 +380,44 @@ static unsigned short spe2_opcd_indices[SPE2_OPCD_SEGS+1];
 void
 disassemble_init_powerpc (struct disassemble_info *info)
 {
-  int i;
-  unsigned short last;
-
   if (powerpc_opcd_indices[PPC_OPCD_SEGS] == 0)
     {
-      i = powerpc_num_opcodes;
-      while (--i >= 0)
-	{
-	  unsigned op = PPC_OP (powerpc_opcodes[i].opcode);
-	  powerpc_opcd_indices[op] = i;
-	}
+      unsigned seg, idx, op;
 
-      last = powerpc_num_opcodes;
-      for (i = PPC_OPCD_SEGS; i > 0; --i)
+      /* PPC opcodes */
+      for (seg = 0, idx = 0; seg <= PPC_OPCD_SEGS; seg++)
 	{
-	  if (powerpc_opcd_indices[i] == 0)
-	    powerpc_opcd_indices[i] = last;
-	  last = powerpc_opcd_indices[i];
+	  powerpc_opcd_indices[seg] = idx;
+	  for (; idx < powerpc_num_opcodes; idx++)
+	    if (seg < PPC_OP (powerpc_opcodes[idx].opcode))
+	      break;
 	}
 
-      i = vle_num_opcodes;
-      while (--i >= 0)
+      /* VLE opcodes */
+      for (seg = 0, idx = 0; seg <= VLE_OPCD_SEGS; seg++)
 	{
-	  unsigned op = VLE_OP (vle_opcodes[i].opcode, vle_opcodes[i].mask);
-	  unsigned seg = VLE_OP_TO_SEG (op);
-	  vle_opcd_indices[seg] = i;
+	  vle_opcd_indices[seg] = idx;
+	  for (; idx < vle_num_opcodes; idx++)
+	    {
+	      op = VLE_OP (vle_opcodes[idx].opcode, vle_opcodes[idx].mask);
+	      if (seg < VLE_OP_TO_SEG (op))
+		break;
+	    }
 	}
 
-      last = vle_num_opcodes;
-      for (i = VLE_OPCD_SEGS; i > 0; --i)
+      /* SPE2 opcodes */
+      for (seg = 0, idx = 0; seg <= SPE2_OPCD_SEGS; seg++)
 	{
-	  if (vle_opcd_indices[i] == 0)
-	    vle_opcd_indices[i] = last;
-	  last = vle_opcd_indices[i];
+	  spe2_opcd_indices[seg] = idx;
+	  for (; idx < spe2_num_opcodes; idx++)
+	    {
+	      op = SPE2_XOP (spe2_opcodes[idx].opcode);
+	      if (seg < SPE2_XOP_TO_SEG (op))
+		break;
+	    }
 	}
     }
 
-  /* SPE2 opcodes */
-  i = spe2_num_opcodes;
-  while (--i >= 0)
-    {
-      unsigned xop = SPE2_XOP (spe2_opcodes[i].opcode);
-      unsigned seg = SPE2_XOP_TO_SEG (xop);
-      spe2_opcd_indices[seg] = i;
-    }
-
-  last = spe2_num_opcodes;
-  for (i = SPE2_OPCD_SEGS; i > 1; --i)
-    {
-      if (spe2_opcd_indices[i] == 0)
-	spe2_opcd_indices[i] = last;
-      last = spe2_opcd_indices[i];
-    }
-
   powerpc_init_dialect (info);
 }
 
diff --git a/gas/config/tc-ppc.c b/gas/config/tc-ppc.c
index e6cc26a1d2..22ca332002 100644
--- a/gas/config/tc-ppc.c
+++ b/gas/config/tc-ppc.c
@@ -1602,28 +1602,24 @@ ppc_setup_opcodes (void)
     {
       if (ENABLE_CHECKING)
 	{
-	  if (op != powerpc_opcodes)
-	    {
-	      int old_opcode = PPC_OP (op[-1].opcode);
-	      int new_opcode = PPC_OP (op[0].opcode);
+	  unsigned int new_opcode = PPC_OP (op[0].opcode);
 
 #ifdef PRINT_OPCODE_TABLE
-	      printf ("%-14s\t#%04u\tmajor op: 0x%x\top: 0x%llx\tmask: 0x%llx\tflags: 0x%llx\n",
-		      op->name, (unsigned int) (op - powerpc_opcodes),
-		      (unsigned int) new_opcode, (unsigned long long) op->opcode,
-		      (unsigned long long) op->mask, (unsigned long long) op->flags);
+	  printf ("%-14s\t#%04u\tmajor op: 0x%x\top: 0x%llx\tmask: 0x%llx\tflags: 0x%llx\n",
+		  op->name, (unsigned int) (op - powerpc_opcodes),
+		  new_opcode, (unsigned long long) op->opcode,
+		  (unsigned long long) op->mask, (unsigned long long) op->flags);
 #endif
 
-	      /* The major opcodes had better be sorted.  Code in the
-		 disassembler assumes the insns are sorted according to
-		 major opcode.  */
-	      if (new_opcode < old_opcode)
-		{
-		  as_bad (_("major opcode is not sorted for %s"),
-			  op->name);
-		  bad_insn = TRUE;
-		}
+	  /* The major opcodes had better be sorted.  Code in the disassembler
+	     assumes the insns are sorted according to major opcode.  */
+	  if (op != powerpc_opcodes
+	      && new_opcode < PPC_OP (op[-1].opcode))
+	    {
+	      as_bad (_("major opcode is not sorted for %s"), op->name);
+	      bad_insn = TRUE;
 	    }
+
 	  if ((op->flags & PPC_OPCODE_VLE) != 0)
 	    {
 	      as_bad (_("%s is enabled by vle flag"), op->name);
@@ -1663,30 +1659,22 @@ ppc_setup_opcodes (void)
     {
       if (ENABLE_CHECKING)
 	{
-	  if (op != vle_opcodes)
-	    {
-	      unsigned old_seg, new_seg;
-
-	      old_seg = VLE_OP (op[-1].opcode, op[-1].mask);
-	      old_seg = VLE_OP_TO_SEG (old_seg);
-	      new_seg = VLE_OP (op[0].opcode, op[0].mask);
-	      new_seg = VLE_OP_TO_SEG (new_seg);
+	  unsigned new_seg = VLE_OP_TO_SEG (VLE_OP (op[0].opcode, op[0].mask));
 
 #ifdef PRINT_OPCODE_TABLE
-	      printf ("%-14s\t#%04u\tmajor op: 0x%x\top: 0x%llx\tmask: 0x%llx\tflags: 0x%llx\n",
-		      op->name, (unsigned int) (op - powerpc_opcodes),
-		      (unsigned int) new_seg, (unsigned long long) op->opcode,
-		      (unsigned long long) op->mask, (unsigned long long) op->flags);
+	  printf ("%-14s\t#%04u\tmajor op: 0x%x\top: 0x%llx\tmask: 0x%llx\tflags: 0x%llx\n",
+		  op->name, (unsigned int) (op - vle_opcodes),
+		  (unsigned int) new_seg, (unsigned long long) op->opcode,
+		  (unsigned long long) op->mask, (unsigned long long) op->flags);
 #endif
-	      /* The major opcodes had better be sorted.  Code in the
-		 disassembler assumes the insns are sorted according to
-		 major opcode.  */
-	      if (new_seg < old_seg)
-		{
-		  as_bad (_("major opcode is not sorted for %s"),
-			  op->name);
-		  bad_insn = TRUE;
-		}
+
+	  /* The major opcodes had better be sorted.  Code in the disassembler
+	     assumes the insns are sorted according to major opcode.  */
+	  if (op != vle_opcodes
+	      && new_seg < VLE_OP_TO_SEG (VLE_OP (op[-1].opcode, op[-1].mask)))
+	    {
+	      as_bad (_("major opcode is not sorted for %s"), op->name);
+	      bad_insn = TRUE;
 	    }
 
 	  bad_insn |= insn_validate (op);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]