marhel / r68k

A m68k emulator in rust - let r68k = musashi.clone();
MIT License
72 stars 7 forks source link

Improve QC coverage #75

Closed marhel closed 8 years ago

marhel commented 8 years ago

I noticed when implementing the MOVE ops, that the QC-test set-up didn't catch a few "obvious" errors.

First, it never loads Dn (or An) with values above 0xFF, and second, most addressing modes in effect always returns 0xAA, 0xAAAA or 0xAAAAAAAA, which is the value of the default memory fill. This does not sufficiently test the setting of condition codes on some ops, such as move, that doesn't also perform some calculation on the input values.

This should be improved by filling registers with up to maximum (0xFFFFFFFF), as well as allowing memory reads to return more varied data. The "obvious" solution to the former (setting the QC "size" to 0xFFFFFFFF seemed to dramatically increase test run-time, though).

Somewhat related to #58 because having the Musashi variant where at least memory accesses also take a instance-pointer/token, would likely simplify allowing more varied memory contents.

emoon commented 8 years ago

Yeah. To do more elaborate testing I'm pretty sure we need to speed up the process in general. Even if running in (system) parallel as you wrote before setup per instruction is fairly heavy now and would be even more effect if we have cases where we do more "elaborate" addressing mode testing.

marhel commented 8 years ago

Progress has been made on this. I rewrote the memory model for the Musashi memory callbacks this weekend to use indirection. This means that instead of having a static 16MB array for these functions to read and write from, we just store "wrote 68 to adress 0x7612" and so on.

This not only shaves 16MB off the executable size, but also removes the restriction where (test) code had to live in the first 1K of memory, as only that much was copied between cores during clone etc. for performance reasons. We now only copy the diffs, wherever they might be located in memory, which is much more efficient as well.

Not quite ready to push that yet, though, I also want to ensure QC tests can set a random u32 used as the default memory fill (providing data for all memory locations where nothing has been written yet)

emoon commented 8 years ago

Nice :)

marhel commented 8 years ago

Just pushed the results of this refactoring. Now both memory and registers are filled with a random value, where all 32 bits are random. This does a much better job of exposing implementation differences, but also causes a lot more address-errors, due to word/long memory access at odd addresses.

This exposed us to two issues in Musashi. First, if an instruction causes an address error, apparently Musashi does exception processing, and also executes the first user instruction in the handler (the one the exception vector points to). r68k stops execution after exception processing, before executing any further instructions. As we haven't actually set up the vectors to point to anything, Musashi is basically just asked to execute a random instruction, which corrupts the state we want to compare. For now, I just ask QC to discard the result of that test when this happens (after checking that the memory accesses up to and including reading the vector match up).

Secondly, I think I found a initialization bug in Musashi; when the "random instruction" executed causes a different exception setting the "exception processing" flag, that flag isn't reset when resetting Musashi to prepare for a different test - and a subsequent address error then pushes this flag onto the stack. I've included a patched Musashi here, but haven't had the energy to do a PR.

marhel commented 8 years ago

Perhaps we can setup all vectors to point to a NOP handler, when we have implemented NOP, and then allow r68k to continue one step further, if we want to compare states, but I'm not sure that would test the intended instruction much beyond noticing if flags are set before or after memory accesses.

emoon commented 8 years ago

Yeah I'm not sure what the best approach would be here

marhel commented 8 years ago

This has been fixed, most recently by this PR fixing the issue where Musashi if an instruction causes address error, kept executing one or more instructions in the address error exception handler, even though the address error processing already depleted all available cycles.

emoon commented 8 years ago

:+1: