From 47c24ff07ae3dccff92646ad72d62c4ee48fed20 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 6 Apr 2007 15:15:36 +0000 Subject: [PATCH 1/5] Clean up the macroop code. --HG-- extra : convert_revision : 3cf83c3e038fece6190dbb91f56deb0498c9a70d --- src/arch/x86/isa/formats/macroop.isa | 47 ++++++---------------------- 1 file changed, 10 insertions(+), 37 deletions(-) diff --git a/src/arch/x86/isa/formats/macroop.isa b/src/arch/x86/isa/formats/macroop.isa index 717103df1..455f4a496 100644 --- a/src/arch/x86/isa/formats/macroop.isa +++ b/src/arch/x86/isa/formats/macroop.isa @@ -85,9 +85,6 @@ output header {{ delete [] microOps; } - std::string generateDisassembly(Addr pc, - const SymbolTable *symtab) const; - StaticInstPtr * microOps; StaticInstPtr fetchMicroOp(MicroPC microPC) @@ -98,20 +95,6 @@ output header {{ %(BasicExecPanic)s }; - - // Base class for macroops which commit as they go. This is for - // instructions which can be partially completed like those with the - // rep prefix. This prevents those instructions from overflowing - // buffers with uncommitted microops. - class X86RollingMacroInst : public X86MacroInst - { - protected: - //Constructor. - X86RollingMacroInst(const char *mnem, ExtMachInst _machInst, - uint32_t _numMicroOps) - : X86MacroInst(mnem, _machInst, numMicroOps) - {} - }; }}; // Basic instruction class constructor template. @@ -121,34 +104,24 @@ def template MacroConstructor {{ { %(constructor)s; //alloc_micro_ops is the code that sets up the microOps - //array in the parent class. This hook will hopefully - //allow all that to be automated. + //array in the parent class. %(alloc_micro_ops)s; - setMicroFlags(); } }}; let {{ - def genMacroOp(name, Name, ops, rolling = False): + def genMacroOp(name, Name, opSeq, rolling = False): baseClass = 'X86MacroInst' - if rolling: - baseClass = 'X86RollingMacroInst' - numMicroOps = len(ops) + numMicroOps = len(opSeq.ops) allocMicroOps = '' micropc = 0 - allocMicroOps += \ - "microOps[0] = %s;\n" % \ - op.getAllocator(True, not rolling, True, False) - micropc += 1 - if numMicroOps > 2: - for op in ops[1:-1]: - allocMicroOps += \ - "microOps[%d] = %s;\n" % \ - (micropc, op.getAllocator(True, not rolling, False, False)) - micropc += 1 - allocMicroOps += \ - "microOps[%d] = %s;\n" % \ - op.getAllocator(True, not rolling, False, True) + for op in opSeq.ops: + allocMicroOps += \ + "microOps[%d] = %s;\n" % \ + (micropc, op.getAllocator(True, not rolling, + micropc == 0, + micropc == numMicroOps - 1)) + micropc += 1 iop = InstObjParams(name, Name, baseClass, {'code' : '', 'num_micro_ops' : numMicroOps, 'alloc_micro_ops' : allocMicroOps}) From e633e23a3ab32dda963746bea43147d54599c01c Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 6 Apr 2007 15:16:36 +0000 Subject: [PATCH 2/5] Add in a stub merging function --HG-- extra : convert_revision : 15e3cdb4ebcd31bc44204687ba59dde00c56c6be --- src/arch/x86/isa/base.isa | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/arch/x86/isa/base.isa b/src/arch/x86/isa/base.isa index 4776f7a7e..cd166b306 100644 --- a/src/arch/x86/isa/base.isa +++ b/src/arch/x86/isa/base.isa @@ -79,6 +79,13 @@ output header {{ void printReg(std::ostream &os, int reg) const; void printSrcReg(std::ostream &os, int reg) const; void printDestReg(std::ostream &os, int reg) const; + + inline uint64_t merge(uint64_t into, uint64_t val, int size) const + { + //FIXME This needs to be significantly more sophisticated + return val; + } + }; }}; From 75e8838ba4020a03f53b66488ecfc756112b2861 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 6 Apr 2007 15:19:23 +0000 Subject: [PATCH 3/5] Clean up the code a little, fix (I think) a perceived problem with immediate sizes, and sign extend the 32-bit-acting-like-64-bit-immediates. --HG-- extra : convert_revision : e59b747198cc79d50045bd2dc45b2e2b97bbffcc --- src/arch/x86/predecoder.cc | 64 ++++++++++++++++++++++--------- src/arch/x86/predecoder.hh | 2 +- src/arch/x86/predecoder_tables.cc | 18 ++++----- src/arch/x86/types.hh | 46 +++++++++++++--------- src/arch/x86/utility.hh | 3 +- 5 files changed, 85 insertions(+), 48 deletions(-) diff --git a/src/arch/x86/predecoder.cc b/src/arch/x86/predecoder.cc index 80971e7cf..573012ee6 100644 --- a/src/arch/x86/predecoder.cc +++ b/src/arch/x86/predecoder.cc @@ -117,37 +117,33 @@ namespace X86ISA //Operand size override prefixes case OperandSizeOverride: DPRINTF(Predecoder, "Found operand size override prefix.\n"); + emi.legacy.op = true; break; case AddressSizeOverride: DPRINTF(Predecoder, "Found address size override prefix.\n"); + emi.legacy.addr = true; break; //Segment override prefixes case CSOverride: - DPRINTF(Predecoder, "Found cs segment override.\n"); - break; case DSOverride: - DPRINTF(Predecoder, "Found ds segment override.\n"); - break; case ESOverride: - DPRINTF(Predecoder, "Found es segment override.\n"); - break; case FSOverride: - DPRINTF(Predecoder, "Found fs segment override.\n"); - break; case GSOverride: - DPRINTF(Predecoder, "Found gs segment override.\n"); - break; case SSOverride: - DPRINTF(Predecoder, "Found ss segment override.\n"); + DPRINTF(Predecoder, "Found segment override.\n"); + emi.legacy.seg = prefix; break; case Lock: DPRINTF(Predecoder, "Found lock prefix.\n"); + emi.legacy.lock = true; break; case Rep: DPRINTF(Predecoder, "Found rep prefix.\n"); + emi.legacy.rep = true; break; case Repne: DPRINTF(Predecoder, "Found repne prefix.\n"); + emi.legacy.repne = true; break; case RexPrefix: DPRINTF(Predecoder, "Found Rex prefix %#x.\n", nextByte); @@ -198,16 +194,36 @@ namespace X86ISA displacementCollected = 0; emi.displacement = 0; + //Figure out the effective operand size. This can be overriden to + //a fixed value at the decoder level. + if(/*FIXME long mode*/1) + { + if(emi.rex && emi.rex.w) + emi.opSize = 3; // 64 bit operand size + else if(emi.legacy.op) + emi.opSize = 1; // 16 bit operand size + else + emi.opSize = 2; // 32 bit operand size + } + else if(/*FIXME default 32*/1) + { + if(emi.legacy.op) + emi.opSize = 1; // 16 bit operand size + else + emi.opSize = 2; // 32 bit operand size + } + else // 16 bit default operand size + { + if(emi.legacy.op) + emi.opSize = 2; // 32 bit operand size + else + emi.opSize = 1; // 16 bit operand size + } + //Figure out how big of an immediate we'll retreive based //on the opcode. - int immType = ImmediateType[ - emi.opcode.num - 1][nextByte]; - if(0) //16 bit mode - immediateSize = ImmediateTypeToSize[0][immType]; - else if(!(emi.rex & 0x4)) //32 bit mode - immediateSize = ImmediateTypeToSize[1][immType]; - else //64 bit mode - immediateSize = ImmediateTypeToSize[2][immType]; + int immType = ImmediateType[emi.opcode.num - 1][nextByte]; + immediateSize = SizeTypeToSize[emi.opSize - 1][immType]; //Determine what to expect next if (UsesModRM[emi.opcode.num - 1][nextByte]) { @@ -351,6 +367,16 @@ namespace X86ISA if(immediateSize == immediateCollected) { + //XXX Warning! The following is an observed pattern and might + //not always be true! + + //Instructions which use 64 bit operands but 32 bit immediates + //need to have the immediate sign extended to 64 bits. + //Instructions which use true 64 bit immediates won't be + //affected, and instructions that use true 32 bit immediates + //won't notice. + if(immediateSize == 4) + emi.immediate = sext<32>(emi.immediate); DPRINTF(Predecoder, "Collected immediate %#x.\n", emi.immediate); emiIsReady = true; diff --git a/src/arch/x86/predecoder.hh b/src/arch/x86/predecoder.hh index 1df17d6d2..6562ab9f5 100644 --- a/src/arch/x86/predecoder.hh +++ b/src/arch/x86/predecoder.hh @@ -73,7 +73,7 @@ namespace X86ISA static const uint8_t Prefixes[256]; static const uint8_t UsesModRM[2][256]; static const uint8_t ImmediateType[2][256]; - static const uint8_t ImmediateTypeToSize[3][10]; + static const uint8_t SizeTypeToSize[3][10]; protected: ThreadContext * tc; diff --git a/src/arch/x86/predecoder_tables.cc b/src/arch/x86/predecoder_tables.cc index f233ad234..38b9c57a3 100644 --- a/src/arch/x86/predecoder_tables.cc +++ b/src/arch/x86/predecoder_tables.cc @@ -141,7 +141,7 @@ namespace X86ISA } }; - enum ImmediateTypes { + enum SizeType { NoImm, NI = NoImm, ByteImm, @@ -158,19 +158,19 @@ namespace X86ISA VW = VWordImm, ZWordImm, ZW = ZWordImm, - Pointer, - PO = Pointer, //The enter instruction takes -2- immediates for a total of 3 bytes Enter, - EN = Enter + EN = Enter, + Pointer, + PO = Pointer }; - const uint8_t Predecoder::ImmediateTypeToSize[3][10] = + const uint8_t Predecoder::SizeTypeToSize[3][10] = { -// noimm byte word dword qword oword vword zword enter - {0, 1, 2, 4, 8, 16, 2, 2, 3, 4}, //16 bit - {0, 1, 2, 4, 8, 16, 4, 4, 3, 6}, //32 bit - {0, 1, 2, 4, 8, 16, 4, 8, 3, 0} //64 bit +// noimm byte word dword qword oword vword zword enter pointer + {0, 1, 2, 4, 8, 16, 2, 2, 3, 4 }, //16 bit + {0, 1, 2, 4, 8, 16, 4, 4, 3, 6 }, //32 bit + {0, 1, 2, 4, 8, 16, 4, 8, 3, 0 } //64 bit }; //This table determines the immediate type. The first index is the diff --git a/src/arch/x86/types.hh b/src/arch/x86/types.hh index cdac3c00e..022f20ee5 100644 --- a/src/arch/x86/types.hh +++ b/src/arch/x86/types.hh @@ -70,25 +70,31 @@ namespace X86ISA typedef uint64_t MachInst; enum Prefixes { - NoOverride = 0, - CSOverride = 1, - DSOverride = 2, - ESOverride = 3, - FSOverride = 4, - GSOverride = 5, - SSOverride = 6, - //The Rex prefix obviously doesn't fit in with the above, but putting - //it here lets us save double the space the enums take up. - RexPrefix = 7, + NoOverride, + CSOverride, + DSOverride, + ESOverride, + FSOverride, + GSOverride, + SSOverride, + RexPrefix, + OperandSizeOverride, + AddressSizeOverride, + Lock, + Rep, + Repne + }; + + BitUnion8(LegacyPrefixVector) + Bitfield<7> repne; + Bitfield<6> rep; + Bitfield<5> lock; + Bitfield<4> addr; + Bitfield<3> op; //There can be only one segment override, so they share the //first 3 bits in the legacyPrefixes bitfield. - SegmentOverride = 0x7, - OperandSizeOverride = 8, - AddressSizeOverride = 16, - Lock = 32, - Rep = 64, - Repne = 128 - }; + Bitfield<2,0> seg; + EndBitUnion(LegacyPrefixVector) BitUnion8(ModRM) Bitfield<7,6> mod; @@ -118,7 +124,7 @@ namespace X86ISA struct ExtMachInst { //Prefixes - uint8_t legacy; + LegacyPrefixVector legacy; Rex rex; //This holds all of the bytes of the opcode struct @@ -140,6 +146,10 @@ namespace X86ISA //Immediate fields uint64_t immediate; uint64_t displacement; + + //The effective operand size. + uint8_t opSize; + //The }; inline static std::ostream & diff --git a/src/arch/x86/utility.hh b/src/arch/x86/utility.hh index e0bd09515..1c98e7fbc 100644 --- a/src/arch/x86/utility.hh +++ b/src/arch/x86/utility.hh @@ -78,7 +78,8 @@ namespace __hash_namespace { ((uint64_t)emi.opcode.prefixA << 16) | ((uint64_t)emi.opcode.prefixB << 8) | ((uint64_t)emi.opcode.op)) ^ - emi.immediate ^ emi.displacement; + emi.immediate ^ emi.displacement ^ + emi.opSize; }; }; } From 2a1c102f25e097ecbec303815182c9bd5332c2ef Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 6 Apr 2007 16:00:56 +0000 Subject: [PATCH 4/5] Refactored the x86 isa description some more. There should be more seperation between x86 specific parts, and those parts which are implemented in the isa description but could eventually be moved elsewhere. --HG-- rename : src/arch/x86/isa/formats/macroop.isa => src/arch/x86/isa/macroop.isa extra : convert_revision : 5ab40eedf574fce438d9fe90e00a496dc95c8bcf --- src/arch/x86/isa/formats/formats.isa | 3 --- src/arch/x86/isa/{formats => }/macroop.isa | 22 ++++++++++------- src/arch/x86/isa/main.isa | 28 ++++++++++++++-------- src/arch/x86/isa/microasm.isa | 2 +- src/arch/x86/isa/microops/base.isa | 3 +++ 5 files changed, 35 insertions(+), 23 deletions(-) rename src/arch/x86/isa/{formats => }/macroop.isa (91%) diff --git a/src/arch/x86/isa/formats/formats.isa b/src/arch/x86/isa/formats/formats.isa index f4e5c402f..d763c05bc 100644 --- a/src/arch/x86/isa/formats/formats.isa +++ b/src/arch/x86/isa/formats/formats.isa @@ -95,9 +95,6 @@ //malfunction of the decode mechanism. ##include "error.isa" -//Include code to build up macro op instructions -##include "macroop.isa" - //Include a format which implements a batch of instructions which do the same //thing on a variety of inputs ##include "multi.isa" diff --git a/src/arch/x86/isa/formats/macroop.isa b/src/arch/x86/isa/macroop.isa similarity index 91% rename from src/arch/x86/isa/formats/macroop.isa rename to src/arch/x86/isa/macroop.isa index 455f4a496..7d41a2dea 100644 --- a/src/arch/x86/isa/formats/macroop.isa +++ b/src/arch/x86/isa/macroop.isa @@ -55,16 +55,20 @@ // // Authors: Gabe Black -//////////////////////////////////////////////////////////////////// -// -// Instructions that do the same thing to multiple sets of arguments. -// +// Execute method for macroops. +def template MacroExecPanic {{ + Fault execute(%(CPU_exec_context)s *, Trace::InstRecord *) const + { + panic("Tried to execute macroop directly!"); + M5_DUMMY_RETURN + } +}}; output header {{ // Base class for most macroops, except ones that need to commit as // they go. - class X86MacroInst : public X86StaticInst + class X86MacroInst : public StaticInst { protected: const uint32_t numMicroOps; @@ -72,7 +76,7 @@ output header {{ //Constructor. X86MacroInst(const char *mnem, ExtMachInst _machInst, uint32_t _numMicroOps) - : X86StaticInst(mnem, _machInst, No_OpClass), + : StaticInst(mnem, _machInst, No_OpClass), numMicroOps(_numMicroOps) { assert(numMicroOps); @@ -93,7 +97,7 @@ output header {{ return microOps[microPC]; } - %(BasicExecPanic)s + %(MacroExecPanic)s }; }}; @@ -110,7 +114,7 @@ def template MacroConstructor {{ }}; let {{ - def genMacroOp(name, Name, opSeq, rolling = False): + def genMacroOp(name, Name, opSeq): baseClass = 'X86MacroInst' numMicroOps = len(opSeq.ops) allocMicroOps = '' @@ -118,7 +122,7 @@ let {{ for op in opSeq.ops: allocMicroOps += \ "microOps[%d] = %s;\n" % \ - (micropc, op.getAllocator(True, not rolling, + (micropc, op.getAllocator(True, op.delayed, micropc == 0, micropc == numMicroOps - 1)) micropc += 1 diff --git a/src/arch/x86/isa/main.isa b/src/arch/x86/isa/main.isa index cc3a9bee4..d9e90689d 100644 --- a/src/arch/x86/isa/main.isa +++ b/src/arch/x86/isa/main.isa @@ -72,26 +72,34 @@ namespace X86ISA; -//Include the simple microcode assembler +//Include the simple microcode assembler. This will hopefully stay +//unspecialized for x86 and can later be made available to other ISAs. ##include "microasm.isa" -//Include the bitfield definitions -##include "bitfields.isa" - -//Include the operand_types and operand definitions -##include "operands.isa" +//Include code to build macroops. +##include "macroop.isa" //Include the base class for x86 instructions, and some support code +//Code in this file should be general and useful everywhere ##include "base.isa" -//Include the instruction definitions -##include "insts/insts.isa" - //Include the definitions for the instruction formats ##include "formats/formats.isa" -//Include the definitions of the micro ops +//Include the operand_types and operand definitions. These are needed by +//the microop definitions. +##include "operands.isa" + +//Include the definitions of the micro ops. +//These are StaticInst classes which stand on their own and make up an +//internal instruction set. ##include "microops/microops.isa" +//Include the instruction definitions which are microop assembler programs. +##include "insts/insts.isa" + +//Include the bitfield definitions +##include "bitfields.isa" + //Include the decoder definition ##include "decoder/decoder.isa" diff --git a/src/arch/x86/isa/microasm.isa b/src/arch/x86/isa/microasm.isa index b94b55aab..0d9c2bc4c 100644 --- a/src/arch/x86/isa/microasm.isa +++ b/src/arch/x86/isa/microasm.isa @@ -69,7 +69,7 @@ let {{ # use that directly. newStmnt = '' if len(ops) == 1: - decode_block = "return (X86StaticInst *)(%s);" % \ + decode_block = "return %s;" % \ ops[0].getAllocator() return ('', '', decode_block, '') else: diff --git a/src/arch/x86/isa/microops/base.isa b/src/arch/x86/isa/microops/base.isa index b1351d999..beaa44b97 100644 --- a/src/arch/x86/isa/microops/base.isa +++ b/src/arch/x86/isa/microops/base.isa @@ -69,6 +69,9 @@ output header {{ class X86MicroOpBase : public X86StaticInst { protected: + uint8_t opSize; + uint8_t addrSize; + X86MicroOpBase(bool isMicro, bool isDelayed, bool isFirst, bool isLast, const char *mnem, ExtMachInst _machInst, From 59df95c7e6e5d1e0bee48946aea08e436785b298 Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 6 Apr 2007 16:39:25 +0000 Subject: [PATCH 5/5] Consolidated the microcode assembler to help separate it from more x86-centric stuff. --HG-- extra : convert_revision : 5e7e8026e24ce44a3dac4a358e0c3e5560685958 --- src/arch/x86/isa/microasm.isa | 68 +++++++++++++----------------- src/arch/x86/isa/microops/base.isa | 8 ++-- 2 files changed, 33 insertions(+), 43 deletions(-) diff --git a/src/arch/x86/isa/microasm.isa b/src/arch/x86/isa/microasm.isa index 0d9c2bc4c..d3ced71be 100644 --- a/src/arch/x86/isa/microasm.isa +++ b/src/arch/x86/isa/microasm.isa @@ -61,23 +61,6 @@ // variety of operands // -let {{ - # This builds either a regular or macro op to implement the sequence of - # ops we give it. - def genInst(name, Name, ops): - # If we can implement this instruction with exactly one microop, just - # use that directly. - newStmnt = '' - if len(ops) == 1: - decode_block = "return %s;" % \ - ops[0].getAllocator() - return ('', '', decode_block, '') - else: - # Build a macroop to contain the sequence of microops we've - # been given. - return genMacroOp(name, Name, ops) -}}; - let {{ # This code builds up a decode block which decodes based on switchval. # vals is a dict which matches case values with what should be decoded to. @@ -187,14 +170,8 @@ let {{ # At this point, we've built up "code" to have all the necessary extra # instructions needed to implement whatever types of operands were - # specified. Now we'll assemble it it into a microOp sequence. - ops = assembleMicro(code) - - # Build a macroop to contain the sequence of microops we've - # constructed. The decode block will be used to fill in our - # inner decode structure, and the rest will be concatenated and - # passed back. - return genInst(name, Name, ops) + # specified. Now we'll assemble it it into a StaticInst. + return assembleMicro(name, Name, code) }}; //////////////////////////////////////////////////////////////////// @@ -202,6 +179,13 @@ let {{ // The microcode assembler // +let {{ + # These are used when setting up microops so that they can specialize their + # base class template properly. + RegOpType = "RegisterOperand" + ImmOpType = "ImmediateOperand" +}}; + let {{ class MicroOpStatement(object): def __init__(self): @@ -242,19 +226,9 @@ let {{ return 'new %s%s(machInst%s%s)' % (self.className, signature, self.microFlagsText(microFlags), args) }}; -let {{ - def buildLabelDict(ops): - labels = {} - micropc = 0 - for op in ops: - if op.label: - labels[op.label] = count - micropc += 1 - return labels -}}; - let{{ - def assembleMicro(code): + def assembleMicro(name, Name, code): + # This function takes in a block of microcode assembly and returns # a python list of objects which describe it. @@ -341,7 +315,13 @@ let{{ lineMatch = lineRe.search(code) # Decode the labels into displacements - labels = buildLabelDict(statements) + + labels = {} + micropc = 0 + for statement in statements: + if statement.label: + labels[statement.label] = count + micropc += 1 micropc = 0 for statement in statements: for arg in statement.args: @@ -353,5 +333,15 @@ let{{ # micropc + 1 + displacement. arg["operandImm"] = labels[arg["operandLabel"]] - micropc - 1 micropc += 1 - return statements + + # If we can implement this instruction with exactly one microop, just + # use that directly. + if len(statements) == 1: + decode_block = "return %s;" % \ + statements[0].getAllocator() + return ('', '', decode_block, '') + else: + # Build a macroop to contain the sequence of microops we've + # been given. + return genMacroOp(name, Name, statements) }}; diff --git a/src/arch/x86/isa/microops/base.isa b/src/arch/x86/isa/microops/base.isa index beaa44b97..4254994f3 100644 --- a/src/arch/x86/isa/microops/base.isa +++ b/src/arch/x86/isa/microops/base.isa @@ -63,7 +63,7 @@ output header {{ }; }}; -//A class which is the base of all x86 micro ops it provides a function to +//A class which is the base of all x86 micro ops. It provides a function to //set necessary flags appropriately. output header {{ class X86MicroOpBase : public X86StaticInst @@ -97,6 +97,7 @@ def template BaseMicroOpTemplateDeclare {{ let {{ def buildBaseMicroOpTemplate(Name, numParams): + assert(numParams > 0) signature = "<" signature += "int SignatureOperandTypeSpecifier0" for count in xrange(1,numParams): @@ -105,10 +106,9 @@ let {{ signature += ">" subs = {"signature" : signature, "class_name" : Name} return BaseMicroOpTemplateDeclare.subst(subs) +}}; - RegOpType = "RegisterOperand" - ImmOpType = "ImmediateOperand" - +let {{ def buildMicroOpTemplateDict(*params): signature = "<" if len(params):