From ed3922e458b7c590a5ee6743f0f5c121d4959935 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Thu, 9 Nov 2023 11:55:36 -0500 Subject: [PATCH] Switch to accepting failures only with a reason. --- OSBindings/Mac/Clock SignalTests/8088Tests.mm | 124 +++++++++++------- 1 file changed, 75 insertions(+), 49 deletions(-) diff --git a/OSBindings/Mac/Clock SignalTests/8088Tests.mm b/OSBindings/Mac/Clock SignalTests/8088Tests.mm index 0cf44421f..f53a51688 100644 --- a/OSBindings/Mac/Clock SignalTests/8088Tests.mm +++ b/OSBindings/Mac/Clock SignalTests/8088Tests.mm @@ -387,16 +387,17 @@ struct FailedExecution { @implementation i8088Tests { std::vector execution_failures; + std::vector permitted_failures; ExecutionSupport execution_support; } - (NSArray *)testFiles { NSString *path = [NSString stringWithUTF8String:TestSuiteHome]; NSSet *allowList = [NSSet setWithArray:@[ - // Current execution failures: -// @"D4.json.gz", // AAM -// @"F6.7.json.gz", // IDIV -// @"F7.7.json.gz", // IDIV + // Current execution failures, albeit all permitted: + @"D4.json.gz", // AAM + @"F6.7.json.gz", // IDIV byte + @"F7.7.json.gz", // IDIV word ]]; NSSet *ignoreList = nil; @@ -639,53 +640,82 @@ struct FailedExecution { const bool registersEqual = intended_registers == execution_support.registers; const bool flagsEqual = (intended_flags.get() & flags_mask) == (execution_support.flags.get() & flags_mask); - if(!flagsEqual || !registersEqual || !ramEqual) { - FailedExecution failure; - failure.instruction = decoded.second; - failure.test_name = std::string([test[@"name"] UTF8String]); + // Exit if no issues were found. + if(flagsEqual && registersEqual && ramEqual) { + return; + } - NSMutableArray *reasons = [[NSMutableArray alloc] init]; - if(!flagsEqual) { - Flags difference; - difference.set((intended_flags.get() ^ execution_support.flags.get()) & flags_mask); - [reasons addObject: - [NSString stringWithFormat:@"flags differs; errors in %s", - difference.to_string().c_str()]]; + // Presume this is a genuine failure. + std::vector *failure_list = &execution_failures; + + // Redirect it if it's an acceptable failure. + using Operation = InstructionSet::x86::Operation; + + // AAM 00h throws its exception only after modifying flags in an undocumented manner; + // I'm not too concerned about this because AAM 00h is an undocumented usage of 00h, + // not even supported by NEC amongst others, and the proper exception is being thrown. + if(decoded.second.operation() == Operation::AAM && !decoded.second.operand()) { + failure_list = &permitted_failures; + } + + // IDIV_REP has a couple of cases... + if(decoded.second.operation() == Operation::IDIV_REP) { + // For reasons I don't understand, sometimes the test set doesn't increment the IP + // across a REP_IDIV. I don't think (?) this correlates to real 8086 behaviour. + // More research required, but for now I'm not treating this as a roadblock. + Registers advanced_registers = intended_registers; + advanced_registers.ip_ += decoded.first; + if(advanced_registers == execution_support.registers && ramEqual && flagsEqual) { + failure_list = &permitted_failures; } - if(!registersEqual) { - NSMutableArray *registers = [[NSMutableArray alloc] init]; + } + + // Record a failure. + FailedExecution failure; + failure.instruction = decoded.second; + failure.test_name = std::string([test[@"name"] UTF8String]); + + NSMutableArray *reasons = [[NSMutableArray alloc] init]; + if(!flagsEqual) { + Flags difference; + difference.set((intended_flags.get() ^ execution_support.flags.get()) & flags_mask); + [reasons addObject: + [NSString stringWithFormat:@"flags differs; errors in %s", + difference.to_string().c_str()]]; + } + if(!registersEqual) { + NSMutableArray *registers = [[NSMutableArray alloc] init]; #define Reg(x) \ if(intended_registers.x() != execution_support.registers.x()) \ [registers addObject: \ [NSString stringWithFormat: \ @#x" is %04x rather than %04x", execution_support.registers.x(), intended_registers.x()]]; - Reg(ax); - Reg(cx); - Reg(dx); - Reg(bx); - Reg(sp); - Reg(bp); - Reg(si); - Reg(di); - Reg(ip); - Reg(es); - Reg(cs); - Reg(ds); - Reg(ss); + Reg(ax); + Reg(cx); + Reg(dx); + Reg(bx); + Reg(sp); + Reg(bp); + Reg(si); + Reg(di); + Reg(ip); + Reg(es); + Reg(cs); + Reg(ds); + Reg(ss); #undef Reg - [reasons addObject:[NSString stringWithFormat: - @"registers don't match: %@", [registers componentsJoinedByString:@", "] - ]]; - } - if(!ramEqual) { - [reasons addObject:@"RAM contents don't match"]; - } - - failure.reason = std::string([reasons componentsJoinedByString:@"; "].UTF8String); - execution_failures.push_back(std::move(failure)); + [reasons addObject:[NSString stringWithFormat: + @"registers don't match: %@", [registers componentsJoinedByString:@", "] + ]]; } + if(!ramEqual) { + [reasons addObject:@"RAM contents don't match"]; + } + + failure.reason = std::string([reasons componentsJoinedByString:@"; "].UTF8String); + failure_list->push_back(std::move(failure)); } - (void)printFailures:(NSArray *)failures { @@ -744,20 +774,16 @@ struct FailedExecution { } // Lock in current failure rate. - XCTAssertLessThanOrEqual(execution_failures.size(), 4009); - - // Current accepted failures: - // * 2484 instances of LEA from a register, which officially has undefined results; - // * 42 instances of AAM 00h for which I can't figure out what to do with flags; and - // * 1486 instances of IDIV, most either with a rep or repne that on the 8086 specifically negatives the result, - // but some admittedly still unexplained (primarily setting overflow even though the result doesn't overflow; - // a couple of online 8086 emulators also didn't throw so maybe this is an 8086 quirk?) + XCTAssertLessThanOrEqual(execution_failures.size(), 0); for(const auto &failure: execution_failures) { NSLog(@"Failed %s — %s", failure.test_name.c_str(), failure.reason.c_str()); } + for(const auto &failure: permitted_failures) { + NSLog(@"Permitted failure of %s — %s", failure.test_name.c_str(), failure.reason.c_str()); + } - NSLog(@"Files with failures were: %@", failures); + NSLog(@"Files with failures, permitted or otherwise, were: %@", failures); } @end