From 32020236cf9e3c296ebb694f09a765d3b3c2639d Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 7 Aug 2020 02:07:32 -0700 Subject: [PATCH] x86: Style fix in the decoder class. Change-Id: If06a8771b5db0fb68e88b16dedfe60fc2ce306d9 Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/32894 Reviewed-by: Andreas Sandberg Maintainer: Gabe Black Tested-by: kokoro --- src/arch/x86/decoder.cc | 105 +++++++++++++++++++--------------------- src/arch/x86/decoder.hh | 99 ++++++++++++++++++------------------- 2 files changed, 97 insertions(+), 107 deletions(-) diff --git a/src/arch/x86/decoder.cc b/src/arch/x86/decoder.cc index 320cbdcbd..28034cba4 100644 --- a/src/arch/x86/decoder.cc +++ b/src/arch/x86/decoder.cc @@ -71,11 +71,11 @@ Decoder::doResetState() void Decoder::process() { - //This function drives the decoder state machine. + // This function drives the decoder state machine. - //Some sanity checks. You shouldn't try to process more bytes if - //there aren't any, and you shouldn't overwrite an already - //decoder ExtMachInst. + // Some sanity checks. You shouldn't try to process more bytes if + // there aren't any, and you shouldn't overwrite an already decoded + // ExtMachInst. assert(!outOfBytes); assert(!instDone); @@ -87,7 +87,7 @@ Decoder::process() instBytes->chunks.push_back(fetchChunk); } - //While there's still something to do... + // While there's still something to do... while (!instDone && !outOfBytes) { uint8_t nextByte = getNextByte(); switch (state) { @@ -170,8 +170,8 @@ Decoder::doFromCacheState() } } -//Either get a prefix and record it in the ExtMachInst, or send the -//state machine on to get the opcode(s). +// Either get a prefix and record it in the ExtMachInst, or send the +// state machine on to get the opcode(s). Decoder::State Decoder::doPrefixState(uint8_t nextByte) { @@ -182,9 +182,8 @@ Decoder::doPrefixState(uint8_t nextByte) prefix = 0; if (prefix) consumeByte(); - switch(prefix) - { - //Operand size override prefixes + switch(prefix) { + // Operand size override prefixes case OperandSizeOverride: DPRINTF(Decoder, "Found operand size override prefix.\n"); emi.legacy.op = true; @@ -193,7 +192,7 @@ Decoder::doPrefixState(uint8_t nextByte) DPRINTF(Decoder, "Found address size override prefix.\n"); emi.legacy.addr = true; break; - //Segment override prefixes + // Segment override prefixes case CSOverride: case DSOverride: case ESOverride: @@ -451,8 +450,8 @@ Decoder::processOpcode(ByteTable &immTable, ByteTable &modrmTable, State nextState = ErrorState; const uint8_t opcode = emi.opcode.op; - //Figure out the effective operand size. This can be overriden to - //a fixed value at the decoder level. + // Figure out the effective operand size. This can be overriden to + // a fixed value at the decoder level. int logOpSize; if (emi.rex.w) logOpSize = 3; // 64 bit operand size @@ -461,33 +460,33 @@ Decoder::processOpcode(ByteTable &immTable, ByteTable &modrmTable, else logOpSize = defOp; - //Set the actual op size + // Set the actual op size. emi.opSize = 1 << logOpSize; - //Figure out the effective address size. This can be overriden to - //a fixed value at the decoder level. + // Figure out the effective address size. This can be overriden to + // a fixed value at the decoder level. int logAddrSize; if (emi.legacy.addr) logAddrSize = altAddr; else logAddrSize = defAddr; - //Set the actual address size + // Set the actual address size. emi.addrSize = 1 << logAddrSize; - //Figure out the effective stack width. This can be overriden to - //a fixed value at the decoder level. + // Figure out the effective stack width. This can be overriden to + // a fixed value at the decoder level. emi.stackSize = 1 << stack; - //Figure out how big of an immediate we'll retreive based - //on the opcode. + // Figure out how big of an immediate we'll retreive based + // on the opcode. int immType = immTable[opcode]; if (addrSizedImm) immediateSize = SizeTypeToSize[logAddrSize - 1][immType]; else immediateSize = SizeTypeToSize[logOpSize - 1][immType]; - //Determine what to expect next + // Determine what to expect next. if (modrmTable[opcode]) { nextState = ModRMState; } else { @@ -501,9 +500,9 @@ Decoder::processOpcode(ByteTable &immTable, ByteTable &modrmTable, return nextState; } -//Get the ModRM byte and determine what displacement, if any, there is. -//Also determine whether or not to get the SIB byte, displacement, or -//immediate next. +// Get the ModRM byte and determine what displacement, if any, there is. +// Also determine whether or not to get the SIB byte, displacement, or +// immediate next. Decoder::State Decoder::doModRMState(uint8_t nextByte) { @@ -511,7 +510,7 @@ Decoder::doModRMState(uint8_t nextByte) ModRM modRM = nextByte; DPRINTF(Decoder, "Found modrm byte %#x.\n", nextByte); if (defOp == 1) { - //figure out 16 bit displacement size + // Figure out 16 bit displacement size. if ((modRM.mod == 0 && modRM.rm == 6) || modRM.mod == 2) displacementSize = 2; else if (modRM.mod == 1) @@ -519,7 +518,7 @@ Decoder::doModRMState(uint8_t nextByte) else displacementSize = 0; } else { - //figure out 32/64 bit displacement size + // Figure out 32/64 bit displacement size. if ((modRM.mod == 0 && modRM.rm == 5) || modRM.mod == 2) displacementSize = 4; else if (modRM.mod == 1) @@ -537,8 +536,8 @@ Decoder::doModRMState(uint8_t nextByte) immediateSize = (emi.opSize == 8) ? 4 : emi.opSize; } - //If there's an SIB, get that next. - //There is no SIB in 16 bit mode. + // If there's an SIB, get that next. + // There is no SIB in 16 bit mode. if (modRM.rm == 4 && modRM.mod != 3) { // && in 32/64 bit mode) nextState = SIBState; @@ -550,15 +549,15 @@ Decoder::doModRMState(uint8_t nextByte) instDone = true; nextState = ResetState; } - //The ModRM byte is consumed no matter what + // The ModRM byte is consumed no matter what. consumeByte(); emi.modRM = modRM; return nextState; } -//Get the SIB byte. We don't do anything with it at this point, other -//than storing it in the ExtMachInst. Determine if we need to get a -//displacement or immediate next. +// Get the SIB byte. We don't do anything with it at this point, other +// than storing it in the ExtMachInst. Determine if we need to get a +// displacement or immediate next. Decoder::State Decoder::doSIBState(uint8_t nextByte) { @@ -579,8 +578,7 @@ Decoder::doSIBState(uint8_t nextByte) return nextState; } -//Gather up the displacement, or at least as much of it -//as we can get. +// Gather up the displacement, or at least as much of it as we can get. Decoder::State Decoder::doDisplacementState() { @@ -594,9 +592,9 @@ Decoder::doDisplacementState() displacementSize, immediateCollected); if (displacementSize == immediateCollected) { - //Reset this for other immediates. + // Reset this for other immediates. immediateCollected = 0; - //Sign extend the displacement + // Sign extend the displacement. switch(displacementSize) { case 1: @@ -627,35 +625,30 @@ Decoder::doDisplacementState() return nextState; } -//Gather up the immediate, or at least as much of it -//as we can get +// Gather up the immediate, or at least as much of it as we can get. Decoder::State Decoder::doImmediateState() { State nextState = ErrorState; - getImmediate(immediateCollected, - emi.immediate, - immediateSize); + getImmediate(immediateCollected, emi.immediate, immediateSize); DPRINTF(Decoder, "Collecting %d byte immediate, got %d bytes.\n", immediateSize, immediateCollected); - if (immediateSize == immediateCollected) - { - //Reset this for other immediates. + if (immediateSize == immediateCollected) { + // Reset this for other immediates. immediateCollected = 0; //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. - switch(immediateSize) - { + // 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. + switch(immediateSize) { case 4: emi.immediate = sext<32>(emi.immediate); break; @@ -667,9 +660,9 @@ Decoder::doImmediateState() emi.immediate); instDone = true; nextState = ResetState; - } - else + } else { nextState = ImmediateState; + } return nextState; } diff --git a/src/arch/x86/decoder.hh b/src/arch/x86/decoder.hh index 6ff5fcdf8..b26241843 100644 --- a/src/arch/x86/decoder.hh +++ b/src/arch/x86/decoder.hh @@ -51,7 +51,7 @@ class ISA; class Decoder : public InstDecoder { private: - //These are defined and documented in decoder_tables.cc + // These are defined and documented in decoder_tables.cc static const uint8_t SizeTypeToSize[3][10]; typedef const uint8_t ByteTable[256]; static ByteTable Prefixes; @@ -81,19 +81,19 @@ class Decoder : public InstDecoder static InstBytes dummy; - //The bytes to be predecoded + // The bytes to be predecoded. MachInst fetchChunk; InstBytes *instBytes; int chunkIdx; - //The pc of the start of fetchChunk + // The pc of the start of fetchChunk. Addr basePC; - //The pc the current instruction started at + // The pc the current instruction started at. Addr origPC; - //The offset into fetchChunk of current processing + // The offset into fetchChunk of current processing. int offset; - //The extended machine instruction being generated + // The extended machine instruction being generated. ExtMachInst emi; - //Predecoding state + // Predecoding state. X86Mode mode; X86SubMode submode; uint8_t altOp; @@ -102,35 +102,38 @@ class Decoder : public InstDecoder uint8_t defAddr; uint8_t stack; - uint8_t getNextByte() + uint8_t + getNextByte() { return ((uint8_t *)&fetchChunk)[offset]; } - void getImmediate(int &collected, uint64_t ¤t, int size) + void + getImmediate(int &collected, uint64_t ¤t, int size) { - //Figure out how many bytes we still need to get for the - //immediate. + // Figure out how many bytes we still need to get for the + // immediate. int toGet = size - collected; - //Figure out how many bytes are left in our "buffer" + // Figure out how many bytes are left in our "buffer". int remaining = sizeof(MachInst) - offset; - //Get as much as we need, up to the amount available. + // Get as much as we need, up to the amount available. toGet = toGet > remaining ? remaining : toGet; - //Shift the bytes we want to be all the way to the right + // Shift the bytes we want to be all the way to the right uint64_t partialImm = fetchChunk >> (offset * 8); - //Mask off what we don't want + // Mask off what we don't want. partialImm &= mask(toGet * 8); - //Shift it over to overlay with our displacement. + // Shift it over to overlay with our displacement. partialImm <<= (immediateCollected * 8); - //Put it into our displacement + // Put it into our displacement. current |= partialImm; - //Update how many bytes we've collected. + // Update how many bytes we've collected. collected += toGet; consumeBytes(toGet); } - void updateOffsetState() + void + updateOffsetState() { assert(offset <= sizeof(MachInst)); if (offset == sizeof(MachInst)) { @@ -147,30 +150,32 @@ class Decoder : public InstDecoder } } - void consumeByte() + void + consumeByte() { offset++; updateOffsetState(); } - void consumeBytes(int numBytes) + void + consumeBytes(int numBytes) { offset += numBytes; updateOffsetState(); } - //State machine state + // State machine state. protected: - //Whether or not we're out of bytes + // Whether or not we're out of bytes. bool outOfBytes; - //Whether we've completed generating an ExtMachInst + // Whether we've completed generating an ExtMachInst. bool instDone; - //The size of the displacement value + // The size of the displacement value. int displacementSize; - //The size of the immediate value + // The size of the immediate value. int immediateSize; - //This is how much of any immediate value we've gotten. This is used - //for both the actual immediate and the displacement. + // This is how much of any immediate value we've gotten. This is used + // for both the actual immediate and the displacement. int immediateCollected; enum State { @@ -189,13 +194,13 @@ class Decoder : public InstDecoder SIBState, DisplacementState, ImmediateState, - //We should never get to this state. Getting here is an error. + // We should never get to this state. Getting here is an error. ErrorState }; State state; - //Functions to handle each of the states + // Functions to handle each of the states State doResetState(); State doFromCacheState(); State doPrefixState(uint8_t); @@ -212,7 +217,7 @@ class Decoder : public InstDecoder State doDisplacementState(); State doImmediateState(); - //Process the actual opcode found earlier, using the supplied tables. + // Process the actual opcode found earlier, using the supplied tables. State processOpcode(ByteTable &immTable, ByteTable &modrmTable, bool addrSizedImm = false); // Process the opcode found with VEX / XOP prefix. @@ -235,8 +240,7 @@ class Decoder : public InstDecoder public: Decoder(ISA* isa = nullptr) : basePC(0), origPC(0), offset(0), - outOfBytes(true), instDone(false), - state(ResetState) + outOfBytes(true), instDone(false), state(ResetState) { emi.reset(); mode = LongMode; @@ -253,7 +257,8 @@ class Decoder : public InstDecoder instMap = NULL; } - void setM5Reg(HandyM5Reg m5Reg) + void + setM5Reg(HandyM5Reg m5Reg) { mode = (X86Mode)(uint64_t)m5Reg.mode; submode = (X86SubMode)(uint64_t)m5Reg.submode; @@ -282,7 +287,8 @@ class Decoder : public InstDecoder } } - void takeOverFrom(Decoder *old) + void + takeOverFrom(Decoder *old) { mode = old->mode; submode = old->submode; @@ -295,16 +301,14 @@ class Decoder : public InstDecoder stack = old->stack; } - void reset() - { - state = ResetState; - } + void reset() { state = ResetState; } void process(); - //Use this to give data to the decoder. This should be used - //when there is control flow. - void moreBytes(const PCState &pc, Addr fetchPC, MachInst data) + // Use this to give data to the decoder. This should be used + // when there is control flow. + void + moreBytes(const PCState &pc, Addr fetchPC, MachInst data) { DPRINTF(Decoder, "Getting more bytes.\n"); basePC = fetchPC; @@ -314,15 +318,8 @@ class Decoder : public InstDecoder process(); } - bool needMoreBytes() - { - return outOfBytes; - } - - bool instReady() - { - return instDone; - } + bool needMoreBytes() { return outOfBytes; } + bool instReady() { return instDone; } void updateNPC(X86ISA::PCState &nextPC) -- 2.30.2