Comprehensive CPU test suite

[sy2002]

Enhance test_programs/cpu_test.asm to be (nearly) complete.

Step 1: Do a bit more than today :-)

[sy2002]

@bernd-ulmann Please decide, if you would like to do this step 1 "a bit more than today" in about the next 5-8 weeks (target: V1.6) or later (target: V1.7). Please use the label feature on the right side here in GitHub to label "V1.6" or "V1.7"

Branch: Please use develop

[sy2002]

@MJoergen Just in case you'd like to support Bernd here: He hates this topic, but it is pretty important, though.

[MJoergen]

I'll look into this issue. It seems like a good way for me to get to know the CPU and the ISA.

[sy2002]

Sounds great, Michael: V1.6 oder V1.7?

[MJoergen]

I've now started work on this, and creaked a branch dev-cputest (from develop branch). I'll write and debug it using the emulator only, and when it's done test it on the hardware. Seems realistic to include it in V1.6

[sy2002]

Cool - this is good news.

[sy2002]

AWESOME :-) I just looked into your commits so far and I am stunned: This CPU Test is really going to be comprehensive. Totally amazing. Thank you ;-)

[sy2002]

@MJoergen I was just wondering: Might it make sense to get rid of regtest.asm (find some clever way to test, if all 256 register banks are there using some better checksum mechanism that I did back in the days. Maybe CRC16?) and add the functionality of regtest.asm into your new cpu_test.asm?

EDIT: One of our (yet unwritten) programming conventions is, that we use the semicolon ; in assembler to denote comments. So intead of writing

// ---------------------------------------------------------------------------
// Test unconditional absolute and relative branches.

L_UNC_0         ABRA    E_UNC_1, !1             // Verify "absolute branch never" is not taken.

                ABRA    L_UNC_1, 1              // Verify "absolute branch always" is taken.
                HALT

you would write

; ---------------------------------------------------------------------------
; Test unconditional absolute and relative branches.

L_UNC_0         ABRA    E_UNC_1, !1             ; Verify "absolute branch never" is not taken.

                ABRA    L_UNC_1, 1              ; Verify "absolute branch always" is taken.
                HALT

This is how Bernd and I did it everywhere in the project so far, includig everything in test_programs and the source code of the monitor.

Now... ...as this is a very liberal project: This is just a suggestion. You actually do not need too change anything. 😃

[sy2002]

One more thing, additionally to the long comment I wrote above ;-) As soon as you start testing this on real hardware, make sure that you turn the third switch (counted from the right of your Nexys4 DDR board), that is SW2 to ON. The result:

This will turn on the CPU Debug Mode and the address where the HALT instruction occured will be shown on the 7-segment display.

[MJoergen]

Regarding comments: Good point about consistency. I'll change to match the existing style.

[sy2002]

@bernd-ulmann This subtle hardware bug might be interessting for you, too, just maybe out of academic curiosity :-)

@MJoergen:

Today, while implementing the timer interrupt generator hardware, I discovered a subtle CPU bug in the hardware: When executing a CMP opcode, the CPU always writes back the second operand. So for example, when you write

CMP 0x1234, R4

then implicitly always a

MOVE R4, R4

is being executed - but without messing up the status register. This does not really cost anything and in most cases you would not notice. But in my special case the following situation appeared: In a test Interrupt Service Routine (ISR) I wrote:

        MOVE    IO$TIMER_0_CNT, R10
        [... more code ...]

H_ISR   MOVE    0xCCCC, @R12++
        CMP     1, @R10

The timer interrupt generator device is implemented in a way, that it is being reset, when you write to any of the registers PRE, CNT or INT. Well: Who would expect that the timer interrupt generator device is being reset when executing

        CMP     1, @R10

Nobody! ... but it is happening due to the CPU bug and the implicit MOVE @R10, @R10 that is being executed "inside" the execution of CMP 1, @R10.

Next steps:

1. I am going to fix this now inside the branch dev-int.

2. You might want to write some CPU test, that provokes the bug (is this possible anyway since it happens "inside" the CPU?); if yes it might make sense to have such a CPU test. If you want to test the fix: take the CPU from dev-int and merge it into your dev-cputest. If this is not possible: I will test the situation thoroughly in the simulator using vhdl/sim/dev_int.vhd.

3. Let's hope that the fix does not have bad side effects, somewhere deep in our code base... 😟

WOW! This incident shows me how important your cpu_test.asm is. I wonder, how we survived all the time without it ?! 😏

[sy2002]

P.S.: My CPU bugfix for the above-mentioned subtlety nearly had a nasty side effect and nearly killed something like CMP 0x1234, @R1++, so a post increment after comparing using an indirect adressing mode for the second operand.

Well, "nearly" means I saw it and it did not happen. But this was luck.

So I guess the cputest needs these kind of tests, too :-)

[MJoergen]

Very interesting.

I was actually thinking about testing things like that. Specifically, I was thinking about testing that each CPU instruction performs the expected number of reads and writes and to the correct addresses. I remember something about the 6502 processor having a similar bug, where the INC instruction (if I remember correctly) did one read and TWO writes to memory. Well, something along those lines at least.

I suggest that the cpu_test.asm be functional only, in that it should be a test that is independent of the hardware.

And then we could make another test, e.g. cpu_bus_test.asm, that tests the correct bus functionality of the cpu. The point is, that to do this test we need some way of monitoring the bus activity of the cpu, and we need to figure out how to do that. This will probably require some extra support in the emulator and the FPGA. So for instance some debug module that records all the reads and writes to RAM and I/O.

[sy2002]

Thank you. Sounds like the perfect way forward.

[sy2002]

Just saw the huge progress you made today. Excellent! 👍
Seeing, that you write tests for MOVE: It reminded me at a really nasty and hard to find and hard to understand CPU bug, that we once had. You might want to have a look at the test program predec.asm to learn more and check, if you are already covering this case.

[MJoergen]

Thank you for the link. That is exactly the kind of thing I want to test!

As of today I'm about half way done with the test suite. The plan for the test suite is to test the following two areas:

1. All possible combinations of instructions and status flags.
2. All possible combinations of instructions and addressing modes.

I've now completed part 1, and have just started on part 2. I'm still only running on the emulator, from the assumption that the emulator behaves correctly.

And I'm still thinking about how to test the CPU bus behavior. Ideally, it should be done in a way that works identical in the emulator and the hardware, and should be fully automatic, i.e. no manual verification. Any ideas are welcome, I have not made up my mind yet on how to do that.

[MJoergen]

Just for fun, I tried the recent version of the CPU test suite on hardware, and I've already filed an Issue #49 :-)

[sy2002]

I need to admit that i am a bit scared about you running this kind of super comprehensive test suite against the hardware 😅

[sy2002]

Your first check-in today: 4:26 (am !) wow... impressive! Are you such an early bird or are you currently at another timezone than Denmark?

[MJoergen]

Your first check-in today: 4:26 (am !) wow... impressive! Are you such an early bird or are you currently at another timezone than Denmark?

Just woke up early this morning :-D

[MJoergen]

I just created a separate Issue #55 to handle the bus functional test suite.

The current CPU test suite is now almost complete (not counting possible changes due to Issue #53).

Here is a list of the remaining tests to perform:

• Verify that the PC can be accessed via R15 (both read and write).
• Verify the NOT instruction with all addressing modes (as a representative of opcodes with one argument).
• Verify the instructions RSUB and ASUB, and the use of the Stack Pointer and R13.
• Verify registers banking.

The register bank verification could be done using a Pseudo Random Number Generator to generate a deterministic sequence of 2056 different values that get written to each register, and then reading back each value and comparing with the expected.

[sy2002]

I love your idea of the Pseudo Random Number Generator with the deterministic sequence running through all 2048 registers. This is much better than my old and pretty dumb test_programs/regbank.asm 👍

[MJoergen]

When testing register banking I came across a peculiarity, and I'm not sure what the intended behaviour is.

After the following two instructions, carry is NOT set (at least in the emulator):

MOVE    0xFF01, R14       // Clear carry
ADD     0x0100, R14       // This should generate carry

The value of R14 after these instructions is 0x0001, and I would expect the value 0x0005.

In the emulator, the addition does generate carry, which updates the register R14, but then the result of the addition is written, which overwrites the carry flag.

I suppose this behavior is correct, but just want to mention it here, and hear your opinion.

[MJoergen]

Just to note:

Verify the NOT instruction with all addressing modes (as a representative of opcodes with one argument).

This was not added, because there is nothing special about the NOT instruction: It takes two operands like all other instructions.

[bernd-ulmann]

When testing register banking I came across a peculiarity, and I'm not sure what the intended behaviour is.

After the following two instructions, carry is NOT set (at least in the emulator):

MOVE    0xFF01, R14       // Clear carry
ADD     0x0100, R14       // This should generate carry

The value of R14 after these instructions is 0x0001, and I would expect the value 0x0005.

In the emulator, the addition does generate carry, which updates the register R14, but then the result of the addition is written, which overwrites the carry flag.

I suppose this behavior is correct, but just want to mention it here, and hear your opinion.

This behaviour is correct. SR is always updated before the write back to whatever destination register an instruction uses. This is necessary since it would be impossible otherwise to set/clear SR bits by bitwise AND/OR/XOR instructions. So in this case it is correct that the C flag is cleared in this case.

[sy2002]

@MJoergen YOU DID IT!! :-) This is amazing: With 3.823 lines of code, this program is even larger than Q-TRIS! ;-) I just tested it on the emulator and here is what I got:

So everything is good (given the caveat described in issue #57). Now let's test it on hardware: Issue #51 is waiting... 😄 I will tomorrow be able to get my hands on an FPGA again - today I only have my laptop ;-)