From 20d7079006a28b390d9a4c95568c58e65f30d544 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Mon, 23 Oct 2023 16:37:27 -0400 Subject: [PATCH] Start adaptation to new test disassembly form. --- InstructionSets/x86/Instruction.cpp | 45 +++++------ InstructionSets/x86/Instruction.hpp | 12 ++- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 79 ++++++++++++++++--- 3 files changed, 94 insertions(+), 42 deletions(-) diff --git a/InstructionSets/x86/Instruction.cpp b/InstructionSets/x86/Instruction.cpp index fd0a9b8d5..eebf4af33 100644 --- a/InstructionSets/x86/Instruction.cpp +++ b/InstructionSets/x86/Instruction.cpp @@ -8,6 +8,8 @@ #include "Instruction.hpp" +#include "../../Numeric/Carry.hpp" + #include #include #include @@ -335,33 +337,32 @@ std::string InstructionSet::x86::to_string( std::string operand; - auto append = [](std::stringstream &stream, auto value, int length, const char *prefix) { + auto append = [](std::stringstream &stream, auto value, int length) { switch(length) { case 0: if(!value) { - break; + return; } [[fallthrough]]; + case 2: - // If asked to pretend the offset was originally two digits then either of: an unsigned - // 8-bit value or a sign-extended 8-bit value as having been originally 8-bit. - // - // This kicks the issue of whether sign was extended appropriately to functionality tests. - if( - !(value & 0xff00) || - ((value & 0xff80) == 0xff80) || - ((value & 0xff80) == 0x0000) - ) { - stream << prefix << to_hex(value, 2); - break; - } - [[fallthrough]]; - default: - stream << prefix << to_hex(value, 4); - break; + value &= 0xff; + break; } + + stream << std::uppercase << std::hex << value << 'h'; }; + auto append_signed = [](std::stringstream &stream, auto value, int length) { + if(!value && !length) { + return; + } + + const bool is_negative = Numeric::top_bit() & value; + const uint64_t abs_value = std::abs(int16_t(value)); // TODO: don't assume 16-bit. + + stream << (is_negative ? '-' : '+') << std::uppercase << std::hex << abs_value << 'h'; + }; using Source = InstructionSet::x86::Source; const Source source = pointer.source(); switch(source) { @@ -370,7 +371,7 @@ std::string InstructionSet::x86::to_string( case Source::Immediate: { std::stringstream stream; - append(stream, instruction.operand(), immediate_length, ""); + append(stream, instruction.operand(), immediate_length); return stream.str(); } @@ -383,6 +384,7 @@ std::string InstructionSet::x86::to_string( stream << InstructionSet::x86::to_string(operation_size) << ' '; } + stream << '['; Source segment = instruction.segment_override(); if(segment == Source::None) { segment = pointer.default_segment(); @@ -392,7 +394,6 @@ std::string InstructionSet::x86::to_string( } stream << InstructionSet::x86::to_string(segment, InstructionSet::x86::DataSize::None) << ':'; - stream << '['; bool addOffset = false; switch(source) { default: break; @@ -408,11 +409,11 @@ std::string InstructionSet::x86::to_string( addOffset = true; break; case Source::DirectAddress: - stream << to_hex(instruction.offset(), 4); + stream << std::uppercase << std::hex << instruction.offset() << 'h'; break; } if(addOffset) { - append(stream, instruction.offset(), offset_length, "+"); + append_signed(stream, instruction.offset(), offset_length); } stream << ']'; return stream.str(); diff --git a/InstructionSets/x86/Instruction.hpp b/InstructionSets/x86/Instruction.hpp index 41417e30f..994674fd4 100644 --- a/InstructionSets/x86/Instruction.hpp +++ b/InstructionSets/x86/Instruction.hpp @@ -462,16 +462,14 @@ enum class Repetition: uint8_t { }; /// @returns @c true if @c operation supports repetition mode @c repetition; @c false otherwise. -constexpr bool supports(Operation operation, Repetition repetition) { +constexpr bool supports(Operation operation, [[maybe_unused]] Repetition repetition) { switch(operation) { default: return false; - case Operation::INS: - case Operation::OUTS: - return repetition == Repetition::RepE; - case Operation::Invalid: // Retain context here; it's used as an intermediate // state sometimes. + case Operation::INS: + case Operation::OUTS: case Operation::CMPS: case Operation::LODS: case Operation::MOVS: @@ -483,8 +481,8 @@ constexpr bool supports(Operation operation, Repetition repetition) { // IDIV — and possibly DIV — as a quirk, affecting the outcome (possibly negativing the result?). // So the test below should be a function of model, if I come to a conclusion about whether I'm // going for fidelity to the instruction set as generally implemented, or to Intel's specific implementation. - case Operation::IDIV: - return repetition == Repetition::RepNE; +// case Operation::IDIV: +// return repetition == Repetition::RepNE; } } diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 78bf0267c..2c8e4faa2 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -317,11 +317,71 @@ struct FailedExecution { - (NSArray *)testFiles { NSString *path = [NSString stringWithUTF8String:TestSuiteHome]; NSSet *allowList = [NSSet setWithArray:@[ - @"27.json.gz", // DAA - @"2F.json.gz", // DAS - @"D4.json.gz", // AAM - @"F6.7.json.gz", // IDIV - @"F7.7.json.gz", // IDIV + // Current decoding failures: + @"60.json.gz", + @"61.json.gz", + @"62.json.gz", + @"63.json.gz", + @"64.json.gz", + @"65.json.gz", + @"66.json.gz", + @"67.json.gz", + @"68.json.gz", + @"69.json.gz", + @"6A.json.gz", + @"6B.json.gz", + @"6C.json.gz", + @"6D.json.gz", + @"6E.json.gz", + @"6F.json.gz", + @"70.json.gz", + @"71.json.gz", + @"72.json.gz", + @"73.json.gz", + @"74.json.gz", + @"75.json.gz", + @"76.json.gz", + @"77.json.gz", + @"78.json.gz", + @"79.json.gz", + @"7A.json.gz", + @"7B.json.gz", + @"7C.json.gz", + @"7D.json.gz", + @"7E.json.gz", + @"7F.json.gz", + @"9A.json.gz", + @"A4.json.gz", + @"A5.json.gz", + @"A6.json.gz", + @"A7.json.gz", + @"AA.json.gz", + @"AB.json.gz", + @"AC.json.gz", + @"AD.json.gz", + @"AE.json.gz", + @"AF.json.gz", + @"CC.json.gz", + @"E0.json.gz", + @"E1.json.gz", + @"E2.json.gz", + @"E3.json.gz", + @"E4.json.gz", + @"E5.json.gz", + @"E6.json.gz", + @"E7.json.gz", + @"E8.json.gz", + @"E9.json.gz", + @"EA.json.gz", + @"EB.json.gz", + + + // Current execution failures: +// @"27.json.gz", // DAA +// @"2F.json.gz", // DAS +// @"D4.json.gz", // AAM +// @"F6.7.json.gz", // IDIV +// @"F7.7.json.gz", // IDIV ]]; NSSet *ignoreList = nil; @@ -418,11 +478,7 @@ struct FailedExecution { // Attempt clerical reconciliation: // - // TEMPORARY HACK: the test set incorrectly states 'bp+si' whenever it means 'bp+di'. - // Though it also uses 'bp+si' correctly when it means 'bp+si'. Until fixed, take - // a pass on potential issues there. - // - // SEPARATELY: The test suite retains a distinction between SHL and SAL, which the decoder doesn't. So consider that + // The test suite retains a distinction between SHL and SAL, which the decoder doesn't. So consider that // a potential point of difference. // // Also, the decoder treats INT3 and INT 3 as the same thing. So allow for a meshing of those. @@ -430,9 +486,6 @@ struct FailedExecution { while(!isEqual && adjustment) { NSString *alteredName = [test[@"name"] stringByTrimmingCharactersInSet:[NSCharacterSet whitespaceCharacterSet]]; - if(adjustment & 4) { - alteredName = [alteredName stringByReplacingOccurrencesOfString:@"bp+si" withString:@"bp+di"]; - } if(adjustment & 2) { alteredName = [alteredName stringByReplacingOccurrencesOfString:@"shl" withString:@"sal"]; }