[libcpu] [419] x86: Decoding for opcode 0xc0 ("shift group 2")

[lib...@gh11.de]

Revision

419

Author

penberg

Date

2010-04-25 04:56:21 -0700 (Sun, 25 Apr 2010)

Log Message

x86: Decoding for opcode 0xc0 ("shift group 2")

Signed-off-by: Pekka Enberg <pen...@cs.helsinki.fi>

Modified Paths

• trunk/arch/x86/x86_decode.cpp
• trunk/arch/x86/x86_decode.h
• trunk/arch/x86/x86_disasm.cpp
• trunk/arch/x86/x86_isa.h
• trunk/test/bin/x86/i8086/i8086.S
• trunk/test/bin/x86/i8086/i8086.bin

Diff

Modified: trunk/arch/x86/x86_decode.cpp (418 => 419)

--- trunk/arch/x86/x86_decode.cpp	2010-04-25 11:56:04 UTC (rev 418)
+++ trunk/arch/x86/x86_decode.cpp	2010-04-25 11:56:21 UTC (rev 419)
@@ -8,6 +8,11 @@
 #include "x86_isa.h"
 #include "x86_decode.h"
 
+/*
+ * First byte of an element in 'decode_table' is the instruction type.
+ */
+#define X86_INSTR_TYPE_MASK	0xff
+
 static const uint32_t decode_table[256] = {
 	/*[0x0]*/	INSTR_ADD | ADDMODE_REG_RM | WIDTH_BYTE,
 	/*[0x1]*/	INSTR_ADD | ADDMODE_REG_RM | WIDTH_FULL,
@@ -201,7 +206,7 @@
 	/*[0xBD]*/	INSTR_MOV | ADDMODE_IMM_REG | WIDTH_FULL,
 	/*[0xBE]*/	INSTR_MOV | ADDMODE_IMM_REG | WIDTH_FULL,
 	/*[0xBF]*/	INSTR_MOV | ADDMODE_IMM_REG | WIDTH_FULL,
-	/*[0xC0]*/	0,
+	/*[0xC0]*/	INSTR_SHIFT_GRP2 | ADDMODE_IMM_RM | WIDTH_BYTE,
 	/*[0xC1]*/	0,
 	/*[0xC2]*/	0,
 	/*[0xC3]*/	INSTR_RET | ADDMODE_IMPLIED,
@@ -267,6 +272,17 @@
 	/*[0xFF]*/	0,
 };
 
+static const uint32_t shift_grp2_decode_table[8] = {
+	/*[0x00]*/	INSTR_ROL,
+	/*[0x01]*/	INSTR_ROR,
+	/*[0x02]*/	INSTR_RCL,
+	/*[0x03]*/	INSTR_RCR,
+	/*[0x04]*/	INSTR_SHL,
+	/*[0x05]*/	INSTR_SHR,
+	/*[0x06]*/	0,
+	/*[0x07]*/	INSTR_SAR,
+};
+
 static uint8_t
 decode_dst_reg(struct x86_instr *instr)
 {
@@ -432,6 +448,7 @@
 int
 arch_8086_decode_instr(struct x86_instr *instr, uint8_t* RAM, addr_t pc)
 {
+	uint32_t decode;
 	uint8_t opcode;
 
 	instr->nr_bytes = 1;
@@ -468,16 +485,27 @@
 done_prefixes:
 
 	/* Opcode byte */
+	decode		= decode_table[opcode];
+
 	instr->opcode	= opcode;
+	instr->type	= decode & X86_INSTR_TYPE_MASK;
+	instr->flags	= decode & ~X86_INSTR_TYPE_MASK;
 
-	instr->flags	= decode_table[opcode];
-
 	if (instr->flags == 0)	/* Unrecognized? */
 		return -1;
 
 	if (instr->flags & MOD_RM)
 		decode_modrm_byte(instr, RAM[pc++]);
 
+	/* Opcode groups */
+	switch (instr->type) {
+	case INSTR_SHIFT_GRP2:
+		instr->type	= shift_grp2_decode_table[instr->reg_opc];
+		break;
+	default:
+		break;
+	}
+
 	if (instr->flags & MEM_DISP_MASK)
 		decode_disp(instr, RAM, &pc);
 

Modified: trunk/arch/x86/x86_decode.h (418 => 419)

--- trunk/arch/x86/x86_decode.h	2010-04-25 11:56:04 UTC (rev 418)
+++ trunk/arch/x86/x86_decode.h	2010-04-25 11:56:21 UTC (rev 419)
@@ -74,16 +74,17 @@
  *	Addressing modes.
  */
 enum x86_addmode {
-	ADDMODE_REG		= SRC_REG|DST_NONE,		/* register */
+	ADDMODE_ACC_MEM		= SRC_ACC|DST_MEM|DIR_REVERSED,	/* AL/AX -> memory */
+	ADDMODE_ACC_REG		= SRC_ACC|DST_REG,		/* AL/AX -> reg */
+	ADDMODE_IMM		= SRC_IMM|DST_NONE,		/* immediate operand */
+	ADDMODE_IMM_ACC		= SRC_IMM|DST_ACC,		/* immediate -> AL/AX */
 	ADDMODE_IMM_REG		= SRC_IMM|DST_REG,		/* immediate -> register */
+	ADDMODE_IMM_RM		= SRC_IMM|MOD_RM|DIR_REVERSED,	/* immediate -> register/memory */
 	ADDMODE_IMPLIED		= SRC_NONE|DST_NONE,		/* no operands */
+	ADDMODE_MEM_ACC		= SRC_ACC|DST_MEM,		/* memory -> AL/AX */
+	ADDMODE_REG		= SRC_REG|DST_NONE,		/* register */
 	ADDMODE_REG_RM		= SRC_REG|MOD_RM|DIR_REVERSED,	/* register -> register/memory */
 	ADDMODE_RM_REG		= DST_REG|MOD_RM,		/* register/memory -> register */
-	ADDMODE_IMM_ACC		= SRC_IMM|DST_ACC,		/* immediate -> AL/AX */
-	ADDMODE_ACC_REG		= SRC_ACC|DST_REG,		/* AL/AX -> reg */
-	ADDMODE_ACC_MEM		= SRC_ACC|DST_MEM|DIR_REVERSED,	/* AL/AX -> memory */
-	ADDMODE_MEM_ACC		= SRC_ACC|DST_MEM,		/* memory -> AL/AX */
-	ADDMODE_IMM		= SRC_IMM|DST_NONE,		/* immediate operand */
 };
 
 struct x86_instr {
@@ -97,6 +98,7 @@
 	uint32_t		disp;		/* Address displacement */
 	uint32_t		imm_data;	/* Immediate data */
 
+	unsigned long		type;		/* See enum x86_instr_types */
 	unsigned long		flags;		/* See enum x86_instr_flags */
 	enum x86_seg_override	seg_override;
 	enum x86_rep_prefix	rep_prefix;
@@ -104,11 +106,6 @@
 	struct x86_operand	dst;
 };
 
-static uint8_t x86_instr_type(struct x86_instr *instr)
-{
-	return instr->flags & 0xff;
-}
-
 int
 arch_8086_decode_instr(struct x86_instr *instr, uint8_t* RAM, addr_t pc);
 

Modified: trunk/arch/x86/x86_disasm.cpp (418 => 419)

--- trunk/arch/x86/x86_disasm.cpp	2010-04-25 11:56:04 UTC (rev 418)
+++ trunk/arch/x86/x86_disasm.cpp	2010-04-25 11:56:21 UTC (rev 419)
@@ -25,9 +25,9 @@
 	/* [INSTR_CLD] */	 "cld",
 	/* [INSTR_CLI] */	 "cli",
 	/* [INSTR_CMC] */	 "cmc",
-	/* [INSTR_CMP] */	 "cmp",
 	/* [INSTR_CMPSB] */	 "cmpsb",
 	/* [INSTR_CMPSW] */	 "cmpsw",
+	/* [INSTR_CMP] */	 "cmp",
 	/* [INSTR_CWD] */	 "cwd",
 	/* [INSTR_DAA] */	 "daa",
 	/* [INSTR_DAS] */	 "das",
@@ -36,41 +36,41 @@
 	/* [INSTR_HLT] */	 "hlt",
 	/* [INSTR_IDIV] */	 "idiv",
 	/* [INSTR_IMUL] */	 "imul",
-	/* [INSTR_IN] */	 "in",
 	/* [INSTR_INC] */	 "inc",
+	/* [INSTR_INTO] */	 "into",
 	/* [INSTR_INT] */	 "int",
-	/* [INSTR_INTO] */	 "into",
+	/* [INSTR_IN] */	 "in",
 	/* [INSTR_IRET] */	 "iret",
+	/* [INSTR_JAE] */	 "jae",
 	/* [INSTR_JA] */	 "ja",
-	/* [INSTR_JAE] */	 "jae",
+	/* [INSTR_JBE] */	 "jbe",
 	/* [INSTR_JB] */	 "jb",
-	/* [INSTR_JBE] */	 "jbe",
+	/* [INSTR_JCXZ] */	 "jcxz",
 	/* [INSTR_JC] */	 "jc",
-	/* [INSTR_JCXZ] */	 "jcxz",
 	/* [INSTR_JE] */	 "je",
+	/* [INSTR_JGE] */	 "jge",
 	/* [INSTR_JG] */	 "jg",
-	/* [INSTR_JGE] */	 "jge",
+	/* [INSTR_JLE] */	 "jle",
 	/* [INSTR_JL] */	 "jl",
-	/* [INSTR_JLE] */	 "jle",
 	/* [INSTR_JMP] */	 "jmp",
+	/* [INSTR_JNAE] */	 "jnae",
 	/* [INSTR_JNA] */	 "jna",
-	/* [INSTR_JNAE] */	 "jnae",
+	/* [INSTR_JNBE] */	 "jnbe",
 	/* [INSTR_JNB] */	 "jnb",
-	/* [INSTR_JNBE] */	 "jnbe",
 	/* [INSTR_JNC] */	 "jnc",
 	/* [INSTR_JNE] */	 "jne",
+	/* [INSTR_JNGE] */	 "jnge",
 	/* [INSTR_JNG] */	 "jng",
-	/* [INSTR_JNGE] */	 "jnge",
+	/* [INSTR_JNLE] */	 "jnle",
 	/* [INSTR_JNL] */	 "jnl",
-	/* [INSTR_JNLE] */	 "jnle",
 	/* [INSTR_JNO] */	 "jno",
 	/* [INSTR_JNP] */	 "jnp",
 	/* [INSTR_JNS] */	 "jns",
 	/* [INSTR_JNZ] */	 "jnz",
 	/* [INSTR_JO] */	 "jo",
-	/* [INSTR_JP] */	 "jp",
 	/* [INSTR_JPE] */	 "jpe",
 	/* [INSTR_JPO] */	 "jpo",
+	/* [INSTR_JP] */	 "jp",
 	/* [INSTR_JS] */	 "js",
 	/* [INSTR_JZ] */	 "jz",
 	/* [INSTR_LAHF] */	 "lahf",
@@ -79,35 +79,35 @@
 	/* [INSTR_LES] */	 "les",
 	/* [INSTR_LODSB] */	 "lodsb",
 	/* [INSTR_LODSW] */	 "lodsw",
-	/* [INSTR_LOOP] */	 "loop",
 	/* [INSTR_LOOPE] */	 "loope",
 	/* [INSTR_LOOPNE] */	 "loopne",
 	/* [INSTR_LOOPNZ] */	 "loopnz",
 	/* [INSTR_LOOPZ] */	 "loopz",
-	/* [INSTR_MOV] */	 "mov",
+	/* [INSTR_LOOP] */	 "loop",
 	/* [INSTR_MOVSB] */	 "movsb",
 	/* [INSTR_MOVSW] */	 "movsw",
+	/* [INSTR_MOV] */	 "mov",
 	/* [INSTR_MUL] */	 "mul",
 	/* [INSTR_NEG] */	 "neg",
 	/* [INSTR_NOP] */	 "nop",
 	/* [INSTR_NOT] */	 "not",
 	/* [INSTR_OR] */	 "or",
 	/* [INSTR_OUT] */	 "out",
-	/* [INSTR_POP] */	 "pop",
 	/* [INSTR_POPA] */	 "popa",
 	/* [INSTR_POPF] */	 "popf",
-	/* [INSTR_PUSH] */	 "push",
+	/* [INSTR_POP] */	 "pop",
 	/* [INSTR_PUSHA] */	 "pusha",
 	/* [INSTR_PUSHF] */	 "pushf",
+	/* [INSTR_PUSH] */	 "push",
 	/* [INSTR_RCL] */	 "rcl",
 	/* [INSTR_RCR] */	 "rcr",
-	/* [INSTR_REP] */	 "rep",
 	/* [INSTR_REPE] */	 "repe",
 	/* [INSTR_REPNE] */	 "repne",
 	/* [INSTR_REPNZ] */	 "repnz",
 	/* [INSTR_REPZ] */	 "repz",
+	/* [INSTR_REP] */	 "rep",
+	/* [INSTR_RETF] */	 "retf",
 	/* [INSTR_RET] */	 "ret",
-	/* [INSTR_RETF] */	 "retf",
 	/* [INSTR_ROL] */	 "rol",
 	/* [INSTR_ROR] */	 "ror",
 	/* [INSTR_SAHF] */	 "sahf",
@@ -116,6 +116,7 @@
 	/* [INSTR_SBB] */	 "sbb",
 	/* [INSTR_SCASB] */	 "scasb",
 	/* [INSTR_SCASW] */	 "scasw",
+	/* [INSTR_SHIFT_GRP2] */ "<grp2>",
 	/* [INSTR_SHL] */	 "shl",
 	/* [INSTR_SHR] */	 "shr",
 	/* [INSTR_STC] */	 "stc",
@@ -132,7 +133,7 @@
 
 static const char *to_mnemonic(struct x86_instr *instr)
 {
-	return mnemo[x86_instr_type(instr)];
+	return mnemo[instr->type];
 }
 
 static const char *byte_reg_names[] = {

Modified: trunk/arch/x86/x86_isa.h (418 => 419)

--- trunk/arch/x86/x86_isa.h	2010-04-25 11:56:04 UTC (rev 418)
+++ trunk/arch/x86/x86_isa.h	2010-04-25 11:56:21 UTC (rev 419)
@@ -12,7 +12,7 @@
 
  * http://www.emu8086.com/assembly_language_tutorial_assembler_reference/8086_instruction_set.html
  */
-enum {
+enum x86_instr_types {
 	INSTR_AAA,
 	INSTR_AAD,
 	INSTR_AAM,
@@ -117,6 +117,7 @@
 	INSTR_SBB,
 	INSTR_SCASB,
 	INSTR_SCASW,
+	INSTR_SHIFT_GRP2,
 	INSTR_SHL,
 	INSTR_SHR,
 	INSTR_STC,

Modified: trunk/test/bin/x86/i8086/i8086.S (418 => 419)

--- trunk/test/bin/x86/i8086/i8086.S	2010-04-25 11:56:04 UTC (rev 418)
+++ trunk/test/bin/x86/i8086/i8086.S	2010-04-25 11:56:21 UTC (rev 419)
@@ -364,5 +364,28 @@
 	xchgw %si, %ax
 	xchgw %di, %ax
 
+	# 0xc0
+	rolb $0x00, %al
+	rolb $0xff, %bh
+
+	rorb $0x00, %al
+	rorb $0xff, %bh
+
+	rclb $0x00, %al
+	rclb $0xff, %bh
+
+	rcrb $0x00, %al
+	rcrb $0xff, %bh
+
+	sarb $0x00, %al
+	sarb $0xff, %bh
+
+	# The sal and shl instructions perform the same operation.
+	shlb $0x00, %al
+	shlb $0xff, %bh
+
+	shrb $0x00, %al
+	shrb $0xff, %bh
+
 	# 0xd7
 	xlat

Modified: trunk/test/bin/x86/i8086/i8086.bin

(Binary files differ)

 
--
Subscription settings: http://groups.google.com/group/libcpu/subscribe?hl=en

[James Abbatiello]

On Sun, Apr 25, 2010 at 7:56 AM, <lib...@gh11.de> wrote:

Revision

419

-	/* [INSTR_CMP] */	 "cmp",
 	/* [INSTR_CMPSB] */	 "cmpsb",
 	/* [INSTR_CMPSW] */	 "cmpsw",
+	/* [INSTR_CMP] */	 "cmp",
 	/* [INSTR_CWD] */	 "cwd",
 	/* [INSTR_DAA] */	 "daa",
 	/* [INSTR_DAS] */	 "das",

Why do this?  CMP normally sorts before CMPSB, at least according to normal English sorting rules.  Similarly with IN/INC, RET/RETF, etc.

--
James Abbatiello

[Pekka Enberg]

That's what vim sort does by default. I prefer mechanical sorting
because this array and the x86_instr_types enum need to be in the same
order.

Pekka

[James Abbatiello]

On Sun, Apr 25, 2010 at 2:47 PM, Pekka Enberg <pen...@cs.helsinki.fi> wrote:

That's what vim sort does by default. I prefer mechanical sorting

because this array and the x86_instr_types enum need to be in the same
order.

Mechanical sorting seems to have let you down.
From x86_disasm.cpp:

    /* [INSTR_CMPSB] */     "cmpsb",
    /* [INSTR_CMPSW] */     "cmpsw",
    /* [INSTR_CMP] */     "cmp",

From x86_isa.h:
    INSTR_CMP,
    INSTR_CMPSB,
    INSTR_CMPSW,

The problem here is that ASCII ',' comes before A-Z but ASCII ']' comes after.
Perhaps we could create a file called x86_instr.inc which contains lines like
  INSTR(AAA, "aaa")
  INSTR(AAD, "aad")
  INSTR(AAM, "aam")
And then, in x86_isa.h:
  enum x86_instr_types {
  #define INSTR(name,str) INSTR_##name,
  #include "x86_instr.inc"
  #undef INSTR
  };
And in x86_disasm.cpp:
  static const char* mnemo[] = {
  #define INSTR(name,str) str,
  #include "x86_instr.inc"
  #undef INSTR
  };

Coupling this with an assert() that mnemo was in sorted order would prevent anyone from accidentally putting an addition in the wrong place.  The downside is that it makes things a bit tougher to read and to find out where the INSTR_* definitions come from.  Thoughts?

--
James Abbatiello

[Pekka Enberg]

On Sun, Apr 25, 2010 at 10:12 PM, James Abbatiello <abb...@gmail.com> wrote:
> On Sun, Apr 25, 2010 at 2:47 PM, Pekka Enberg <pen...@cs.helsinki.fi>
> wrote:
>>
>> That's what vim sort does by default. I prefer mechanical sorting
>> because this array and the x86_instr_types enum need to be in the same
>> order.
>
> Mechanical sorting seems to have let you down.

Argh!

I like it. We can do

DEFINE_INSTR(INSTR_AAA, "aaa")

to make it all nicely greppable.

[Conrado Miranda]

Or maybe INSTR(AAA) with

enum x86_instr_types {
#define INSTR(name,str) INSTR_##name,
#include "x86_instr.inc"
#undef INSTR
};

static const char* mnemo[] = {

#define INSTR(name,str) #str,
#include "x86_instr.inc"
#undef INSTR
};

This avoids mistyping (can happen to everyone =]) but the string gets
different case. (Just a thought).