[PATCH 1/6] S/390: Split disassembler routine into smaller functions

[Andreas Arnez <arnez at linux dot vnet dot ibm dot com>]

Extract certain logic from print_insn_s390() into separate functions.
This improves readability and reduces some code repetition.

opcodes/
	* s390-dis.c (s390_insn_length, s390_insn_matches_opcode)
	(s390_print_insn_with_opcode, opcode_mask_more_specific): New
	static functions, code was moved from...
	(print_insn_s390): ...here.
	(s390_extract_operand): Adjust comment.  Change type of first
	parameter from 'unsigned char *' to 'const bfd_byte *'.
---
 opcodes/s390-dis.c | 190 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 110 insertions(+), 80 deletions(-)

diff --git a/opcodes/s390-dis.c b/opcodes/s390-dis.c
index 252500c..444e7ae 100644
--- a/opcodes/s390-dis.c
+++ b/opcodes/s390-dis.c
@@ -69,14 +69,38 @@ init_disasm (struct disassemble_info *info)
   init_flag = 1;
 }
 
+/* Derive the length of an instruction from its first byte.  */
+
+static inline int
+s390_insn_length (const bfd_byte *buffer)
+{
+  /* 00xxxxxx -> 2, 01xxxxxx/10xxxxxx -> 4, 11xxxxxx -> 6.  */
+  return ((((buffer[0] >> 6) + 1) >> 1) + 1) << 1;
+}
+
+/* Match the instruction in BUFFER against the given OPCODE, excluding
+   the first byte.  */
+
+static inline int
+s390_insn_matches_opcode (const bfd_byte *buffer,
+			  const struct s390_opcode *opcode)
+{
+  return (buffer[1] & opcode->mask[1]) == opcode->opcode[1]
+    && (buffer[2] & opcode->mask[2]) == opcode->opcode[2]
+    && (buffer[3] & opcode->mask[3]) == opcode->opcode[3]
+    && (buffer[4] & opcode->mask[4]) == opcode->opcode[4]
+    && (buffer[5] & opcode->mask[5]) == opcode->opcode[5];
+}
+
 /* Extracts an operand value from an instruction.  */
 /* We do not perform the shift operation for larl-type address
    operands here since that would lead to an overflow of the 32 bit
    integer value.  Instead the shift operation is done when printing
-   the operand in print_insn_s390.  */
+   the operand.  */
 
 static inline unsigned int
-s390_extract_operand (unsigned char *insn, const struct s390_operand *operand)
+s390_extract_operand (const bfd_byte *insn,
+		      const struct s390_operand *operand)
 {
   unsigned int val;
   int bits;
@@ -110,6 +134,84 @@ s390_extract_operand (unsigned char *insn, const struct s390_operand *operand)
   return val;
 }
 
+/* Print the S390 instruction in BUFFER, assuming that it matches the
+   given OPCODE.  */
+
+static void
+s390_print_insn_with_opcode (bfd_vma memaddr,
+			     struct disassemble_info *info,
+			     const bfd_byte *buffer,
+			     const struct s390_opcode *opcode)
+{
+  const unsigned char *opindex;
+  char separator;
+
+  if (opcode->operands[0] != 0)
+    (*info->fprintf_func) (info->stream, "%s\t", opcode->name);
+  else
+    (*info->fprintf_func) (info->stream, "%s", opcode->name);
+
+  /* Extract the operands.  */
+  separator = 0;
+  for (opindex = opcode->operands; *opindex != 0; opindex++)
+    {
+      const struct s390_operand *operand = s390_operands + *opindex;
+      unsigned int value = s390_extract_operand (buffer, operand);
+
+      if ((operand->flags & S390_OPERAND_INDEX) && value == 0)
+	continue;
+      if ((operand->flags & S390_OPERAND_BASE) &&
+	  value == 0 && separator == '(')
+	{
+	  separator = ',';
+	  continue;
+	}
+
+      if (separator)
+	(*info->fprintf_func) (info->stream, "%c", separator);
+
+      if (operand->flags & S390_OPERAND_GPR)
+	(*info->fprintf_func) (info->stream, "%%r%i", value);
+      else if (operand->flags & S390_OPERAND_FPR)
+	(*info->fprintf_func) (info->stream, "%%f%i", value);
+      else if (operand->flags & S390_OPERAND_AR)
+	(*info->fprintf_func) (info->stream, "%%a%i", value);
+      else if (operand->flags & S390_OPERAND_CR)
+	(*info->fprintf_func) (info->stream, "%%c%i", value);
+      else if (operand->flags & S390_OPERAND_PCREL)
+	(*info->print_address_func) (memaddr + (int)value + (int)value,
+				     info);
+      else if (operand->flags & S390_OPERAND_SIGNED)
+	(*info->fprintf_func) (info->stream, "%i", (int) value);
+      else
+	(*info->fprintf_func) (info->stream, "%u", value);
+
+      if (operand->flags & S390_OPERAND_DISP)
+	{
+	  separator = '(';
+	}
+      else if (operand->flags & S390_OPERAND_BASE)
+	{
+	  (*info->fprintf_func) (info->stream, ")");
+	  separator = ',';
+	}
+      else
+	separator = ',';
+    }
+}
+
+/* Check whether opcode A's mask is more specific than that of B.  */
+
+static int
+opcode_mask_more_specific (const struct s390_opcode *a,
+			   const struct s390_opcode *b)
+{
+  return ( ((int)a->mask[0] + a->mask[1] + a->mask[2]
+	    + a->mask[3] + a->mask[4] + a->mask[5])
+	   > ((int)b->mask[0] + b->mask[1] + b->mask[2]
+	      + b->mask[3] + b->mask[4] + b->mask[5]) );
+}
+
 /* Print a S390 instruction.  */
 
 int
@@ -120,7 +222,6 @@ print_insn_s390 (bfd_vma memaddr, struct disassemble_info *info)
   const struct s390_opcode *opcode_end;
   unsigned int value;
   int status, opsize, bufsize;
-  char separator;
 
   if (init_flag == 0)
     init_disasm (info);
@@ -141,16 +242,13 @@ print_insn_s390 (bfd_vma memaddr, struct disassemble_info *info)
 	  (*info->memory_error_func) (status, memaddr, info);
 	  return -1;
 	}
-      /* Opsize calculation looks strange but it works
-	 00xxxxxx -> 2 bytes, 01xxxxxx/10xxxxxx -> 4 bytes,
-	 11xxxxxx -> 6 bytes.  */
-      opsize = ((((buffer[0] >> 6) + 1) >> 1) + 1) << 1;
+      opsize = s390_insn_length (buffer);
       status = opsize > bufsize;
     }
   else
     {
       bufsize = 6;
-      opsize = ((((buffer[0] >> 6) + 1) >> 1) + 1) << 1;
+      opsize = s390_insn_length (buffer);
     }
 
   if (status == 0)
@@ -163,19 +261,11 @@ print_insn_s390 (bfd_vma memaddr, struct disassemble_info *info)
 	   (opcode < opcode_end) && (buffer[0] == opcode->opcode[0]);
 	   opcode++)
 	{
-	  const struct s390_operand *operand;
-	  const unsigned char *opindex;
-
 	  /* Check architecture.  */
 	  if (!(opcode->modes & current_arch_mask))
 	    continue;
 
-	  /* Check signature of the opcode.  */
-	  if ((buffer[1] & opcode->mask[1]) != opcode->opcode[1]
-	      || (buffer[2] & opcode->mask[2]) != opcode->opcode[2]
-	      || (buffer[3] & opcode->mask[3]) != opcode->opcode[3]
-	      || (buffer[4] & opcode->mask[4]) != opcode->opcode[4]
-	      || (buffer[5] & opcode->mask[5]) != opcode->opcode[5])
+	  if (!s390_insn_matches_opcode (buffer, opcode))
 	    continue;
 
 	  /* Advance to an opcode with a more specific mask.  */
@@ -184,75 +274,15 @@ print_insn_s390 (bfd_vma memaddr, struct disassemble_info *info)
 	      if ((buffer[0] & op->mask[0]) != op->opcode[0])
 		break;
 
-	      if ((buffer[1] & op->mask[1]) != op->opcode[1]
-		  || (buffer[2] & op->mask[2]) != op->opcode[2]
-		  || (buffer[3] & op->mask[3]) != op->opcode[3]
-		  || (buffer[4] & op->mask[4]) != op->opcode[4]
-		  || (buffer[5] & op->mask[5]) != op->opcode[5])
+	      if (!s390_insn_matches_opcode (buffer, op))
 		continue;
 
-	      if (((int)opcode->mask[0] + opcode->mask[1] +
-		   opcode->mask[2] + opcode->mask[3] +
-		   opcode->mask[4] + opcode->mask[5]) <
-		  ((int)op->mask[0] + op->mask[1] +
-		   op->mask[2] + op->mask[3] +
-		   op->mask[4] + op->mask[5]))
+	      if (opcode_mask_more_specific (op, opcode))
 		opcode = op;
 	    }
 
 	  /* The instruction is valid.  */
-	  if (opcode->operands[0] != 0)
-	    (*info->fprintf_func) (info->stream, "%s\t", opcode->name);
-	  else
-	    (*info->fprintf_func) (info->stream, "%s", opcode->name);
-
-	  /* Extract the operands.  */
-	  separator = 0;
-	  for (opindex = opcode->operands; *opindex != 0; opindex++)
-	    {
-	      operand = s390_operands + *opindex;
-	      value = s390_extract_operand (buffer, operand);
-
-	      if ((operand->flags & S390_OPERAND_INDEX) && value == 0)
-		continue;
-	      if ((operand->flags & S390_OPERAND_BASE) &&
-		  value == 0 && separator == '(')
-		{
-		  separator = ',';
-		  continue;
-		}
-
-	      if (separator)
-		(*info->fprintf_func) (info->stream, "%c", separator);
-
-	      if (operand->flags & S390_OPERAND_GPR)
-		(*info->fprintf_func) (info->stream, "%%r%i", value);
-	      else if (operand->flags & S390_OPERAND_FPR)
-		(*info->fprintf_func) (info->stream, "%%f%i", value);
-	      else if (operand->flags & S390_OPERAND_AR)
-		(*info->fprintf_func) (info->stream, "%%a%i", value);
-	      else if (operand->flags & S390_OPERAND_CR)
-		(*info->fprintf_func) (info->stream, "%%c%i", value);
-	      else if (operand->flags & S390_OPERAND_PCREL)
-		(*info->print_address_func) (memaddr + (int)value + (int)value,
-					     info);
-	      else if (operand->flags & S390_OPERAND_SIGNED)
-		(*info->fprintf_func) (info->stream, "%i", (int) value);
-	      else
-		(*info->fprintf_func) (info->stream, "%u", value);
-
-	      if (operand->flags & S390_OPERAND_DISP)
-		{
-		  separator = '(';
-		}
-	      else if (operand->flags & S390_OPERAND_BASE)
-		{
-		  (*info->fprintf_func) (info->stream, ")");
-		  separator = ',';
-		}
-	      else
-		separator = ',';
-	    }
+	  s390_print_insn_with_opcode (memaddr, info, buffer, opcode);
 
 	  /* Found instruction, printed it, return its size.  */
 	  return opsize;
-- 
1.8.4.2