diff --git a/InstructionSets/x86/Implementation/PerformImplementation.hpp b/InstructionSets/x86/Implementation/PerformImplementation.hpp index 95e17007d..0411f16fb 100644 --- a/InstructionSets/x86/Implementation/PerformImplementation.hpp +++ b/InstructionSets/x86/Implementation/PerformImplementation.hpp @@ -1636,7 +1636,7 @@ template < // * use hard-coded register names where appropriate; // * return directly if there is definitely no possible write back to RAM; // * otherwise use the source() and destination() lambdas, and break in order to allow a writeback if necessary. - switch(instruction.operation) { + switch(instruction.operation()) { default: assert(false); diff --git a/InstructionSets/x86/Instruction.cpp b/InstructionSets/x86/Instruction.cpp index 824c36d96..7a7a02863 100644 --- a/InstructionSets/x86/Instruction.cpp +++ b/InstructionSets/x86/Instruction.cpp @@ -420,7 +420,7 @@ std::string InstructionSet::x86::to_string( case Source::IndirectNoBase: { std::stringstream stream; - if(!InstructionSet::x86::mnemonic_implies_data_size(instruction.operation)) { + if(!InstructionSet::x86::mnemonic_implies_data_size(instruction.operation())) { stream << InstructionSet::x86::to_string(operation_size) << ' '; } @@ -473,7 +473,7 @@ std::string InstructionSet::x86::to_string( std::string operation; // Add segment override, if any, ahead of some operations that won't otherwise print it. - switch(instruction.second.operation) { + switch(instruction.second.operation()) { default: break; case Operation::CMPS: @@ -505,14 +505,14 @@ std::string InstructionSet::x86::to_string( } // Add operation itself. - operation += to_string(instruction.second.operation, instruction.second.operation_size(), model); + operation += to_string(instruction.second.operation(), instruction.second.operation_size(), model); operation += " "; // Deal with a few special cases up front. - switch(instruction.second.operation) { + switch(instruction.second.operation()) { default: { - const int operands = max_displayed_operands(instruction.second.operation); - const bool displacement = has_displacement(instruction.second.operation); + const int operands = max_displayed_operands(instruction.second.operation()); + const bool displacement = has_displacement(instruction.second.operation()); const bool print_first = operands > 1 && instruction.second.destination().source() != Source::None; if(print_first) { operation += to_string(instruction.second.destination(), instruction.second, offset_length, immediate_length); diff --git a/InstructionSets/x86/Instruction.hpp b/InstructionSets/x86/Instruction.hpp index bcd186a88..71292b506 100644 --- a/InstructionSets/x86/Instruction.hpp +++ b/InstructionSets/x86/Instruction.hpp @@ -662,10 +662,12 @@ class DataPointer { template class Instruction { public: - Operation operation = Operation::Invalid; + Operation operation() const { + return operation_; + } bool operator ==(const Instruction &rhs) const { - if( operation != rhs.operation || + if( operation_ != rhs.operation_ || mem_exts_source_ != rhs.mem_exts_source_ || source_data_dest_sib_ != rhs.source_data_dest_sib_) { return false; @@ -673,7 +675,7 @@ template class Instruction { // Have already established above that this and RHS have the // same extensions, if any. - const int extension_count = has_length_extension() + has_displacement() + has_operand(); + const int extension_count = has_displacement() + has_operand(); for(int c = 0; c < extension_count; c++) { if(extensions_[c] != rhs.extensions_[c]) return false; } @@ -686,6 +688,8 @@ template class Instruction { using AddressT = ImmediateT; private: + Operation operation_ = Operation::Invalid; + // Packing and encoding of fields is admittedly somewhat convoluted; what this // achieves is that instructions will be sized: // @@ -711,34 +715,14 @@ template class Instruction { } // [b15, b14]: data size; - // [b13, b10]: source length (0 => has length extension); + // [b13]: lock; + // [b12, b10]: segment override; // [b9, b5]: top five of SIB; // [b4, b0]: dest. - uint16_t source_data_dest_sib_ = 1 << 10; // So that ::Invalid doesn't seem to have a length extension. + uint16_t source_data_dest_sib_ = 0; - // Note to future self: if source length continues to prove avoidable, reuse its four bits as: - // three bits: segment (as overridden, otherwise whichever operand has a segment, if either); - // one bit: an extra bit for Operation. - // - // Then what was the length extension will hold only a repetition, if any, and the lock bit. As luck would have - // it there are six valid segment registers so there is an available sentinel value to put into the segment - // field to indicate that there's an extension if necessary. A further three bits would need to be trimmed - // to do away with that extension entirely, but since lock is rarely used and repetitions apply only to a - // small number of operations I think it'd at least be a limited problem. - - bool has_length_extension() const { - return !((source_data_dest_sib_ >> 10) & 15); - } - - // {operand}, {displacement}, {length extension}. - // - // If length extension is present then: - // - // [b15, b6]: source length; - // [b5, b4]: repetition; - // [b3, b1]: segment override; - // b0: lock. - ImmediateT extensions_[3]{}; + // {operand}, {displacement}. + ImmediateT extensions_[2]{}; ImmediateT operand_extension() const { return extensions_[0]; @@ -746,9 +730,6 @@ template class Instruction { ImmediateT displacement_extension() const { return extensions_[(mem_exts_source_ >> 5) & 1]; } - ImmediateT length_extension() const { - return extensions_[((mem_exts_source_ >> 5) & 1) + ((mem_exts_source_ >> 6) & 1)]; - } public: /// @returns The number of bytes used for meaningful content within this class. A receiver must use at least @c sizeof(Instruction) bytes @@ -757,7 +738,7 @@ template class Instruction { size_t packing_size() const { return offsetof(Instruction, extensions) + - (has_displacement() + has_operand() + has_length_extension()) * sizeof(ImmediateT); + (has_displacement() + has_operand()) * sizeof(ImmediateT); // To consider in the future: the length extension is always the last one, // and uses only 8 bits of content within 32-bit instructions, so it'd be @@ -788,7 +769,7 @@ template class Instruction { ); } bool lock() const { - return has_length_extension() && length_extension()&1; + return source_data_dest_sib_ & (1 << 13); } AddressSize address_size() const { @@ -800,10 +781,9 @@ template class Instruction { /// or that used by stack instructions, but this function does not spend the time necessary to provide /// the correct default for those. Source segment_override() const { - if(!has_length_extension()) return Source::None; return Source( int(Source::ES) + - ((length_extension() >> 1) & 7) + ((source_data_dest_sib_ >> 10) & 7) ); } @@ -844,7 +824,7 @@ template class Instruction { DataSize data_size, DisplacementT displacement, ImmediateT operand) noexcept : - operation(operation), + operation_(operation), mem_exts_source_(uint8_t( (int(address_size) << 7) | (displacement ? 0x40 : 0x00) | @@ -854,14 +834,13 @@ template class Instruction { )), source_data_dest_sib_(uint16_t( (int(data_size) << 14) | - (( - (lock || (segment_override != Source::None)) - ) ? 0 : (1 << 10)) | + (lock ? (1 << 13) : 0) | + ((int(segment_override)&7) << 10) | ((uint8_t(sib) & 0xf8) << 2) | int(destination) | (destination == Source::Indirect ? (uint8_t(sib) & 7) : 0) - )) { - + )) + { // Decisions on whether to include operand, displacement and/or size extension words // have implicitly been made in the int packing above; honour them here. int extension = 0; @@ -873,12 +852,6 @@ template class Instruction { extensions_[extension] = ImmediateT(displacement); ++extension; } - if(has_length_extension()) { - extensions_[extension] = ImmediateT( - ((int(segment_override) & 7) << 1) | int(lock) - ); - ++extension; - } } }; diff --git a/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm b/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm index 73ef9698c..6502ce841 100644 --- a/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm +++ b/OSBindings/Mac/Clock SignalTests/x86DecoderTests.mm @@ -21,7 +21,7 @@ namespace { template void test(const InstructionT &instruction, DataSize size, Operation operation) { XCTAssertEqual(instruction.operation_size(), InstructionSet::x86::DataSize(size)); - XCTAssertEqual(instruction.operation, operation); + XCTAssertEqual(instruction.operation(), operation); } template void test( @@ -34,7 +34,7 @@ template void test( std::optional displacement = std::nullopt) { XCTAssertEqual(instruction.operation_size(), InstructionSet::x86::DataSize(size)); - XCTAssertEqual(instruction.operation, operation); + XCTAssertEqual(instruction.operation(), operation); if(source) XCTAssert(instruction.source() == *source); if(destination) XCTAssert(instruction.destination() == *destination); if(operand) XCTAssertEqual(instruction.operand(), *operand); @@ -46,7 +46,7 @@ template void test( Operation operation, std::optional operand = std::nullopt, std::optional displacement = std::nullopt) { - XCTAssertEqual(instruction.operation, operation); + XCTAssertEqual(instruction.operation(), operation); if(operand) XCTAssertEqual(instruction.operand(), *operand); if(displacement) XCTAssertEqual(instruction.displacement(), *displacement); } @@ -56,7 +56,7 @@ template void test_far( Operation operation, uint16_t segment, typename InstructionT::DisplacementT offset) { - XCTAssertEqual(instruction.operation, operation); + XCTAssertEqual(instruction.operation(), operation); XCTAssertEqual(instruction.segment(), segment); XCTAssertEqual(instruction.offset(), offset); }