This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH 1/6] S/390: Split disassembler routine into smaller functions
- From: Andreas Arnez <arnez at linux dot vnet dot ibm dot com>
- To: binutils at sourceware dot org
- Cc: Martin Schwidefsky <schwidefsky at de dot ibm dot com>, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- Date: Fri, 25 Jul 2014 19:01:48 +0200
- Subject: [PATCH 1/6] S/390: Split disassembler routine into smaller functions
- Authentication-results: sourceware.org; auth=none
- References: <1406307713-7926-1-git-send-email-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