Added extra carry checks to the Flags UI for opcodes 8XY4, 8XY5 and 8XY7

[jon-axon]

As per issue #7 that I raised yesterday, I am submitting a PR for an enhancement to the Flags test to conduct an additional check on the VF register for each of the opcodes 8XY4, 8XY5 and 8XY7, in order to trap incorrect behaviour that is not detected by your existing three checkmarks in v3.

For the purposes of updating as many necessary files as possible as part of this PR, to save you a job, I have taken the liberty of assuming this would be a v3.1 release (as it does not seem significant enough to merit a v4).

As part of this PR, I have:

• Updated the src/tests/flags.8o file to include an additional check for these three opcodes, using VF as the VX argument. This is done via a 4th checkmark (and so includes the addition of a new 4-parameter "expect" macro in-keeping with your existing code style). The changes also involve re-organising the UI layout slightly to accommodate the additional checkmark
• Updated the address of jump quirk targets in src/index.8o and src/tests/quirks.8o from 0xE87 and 0xE8B to 0xE97 and 0xE9B respectively, to avoid an Octo "data overlap" error that occurred on build, which I assume was caused by the increased length of my flags test code now extending into your previous hardcoded memory space
• Updated the pictures/splash.png file to show v3.1
• Updated the pictures/flags.png file to show the updated UI layout
• Updated the pictures/chip-8-logo.png file to show v3.1 as per the updated splash screen
• Built the project to re-generate the docs/index.html, bin/chip8-test-suite.8o and bin/chip8-test-suite.ch8 files
• Updated the README.md file

I have NOT:

• Updated the animated pictures/animation.gif file

I have regression/system tested the full updated test suite (not just the Flags test) in the following contexts, and all appears well:

• In CHIP-8 mode in the standalone Octo interpreter built with the project
• In both CHIP-8 and SUPER-CHIP 1.1 compatibility modes in my own interpreter
• In VIP, SCHIP and XO-CHIP modes in your Silicon8 interpreter

[jon-axon]

@Timendus just tagging you here, in case you're not getting general notifications for this repo (not sure how often you check it)

[Timendus]

Thanks for the tag! I'm on holiday so no I'm not checking GitHub very often 😉 Thanks for a very comprehensive PR, I'll look in more detail soon!

[jon-axon]

No rush at all, have a good one!

[gulrak]

Nice work!

One little problem might be, that the program size now exceeds the "legal" size of a program for the classic COSMAC VIP, which is why Octo is not accepting it when being in COSMAC VIP mode. It takes up to 0xE9E of the RAM but staring 0xE90 is the part of RAM that is used by the CHIP-8 interpreter (the stack is there).

It would actually still work as the test suite doesn't need that much stack, but my bet is, there will be more emulators besides Octo limiting the available program size to the "semi-official" 3216 bytes that would not load the suite after that change.

There are a few (7) bytes unused between the last sprite and the jump0 targets, but that is not enough, so one would need to find another 8 to get back into the "valid" size.

[jon-axon]

Hi Gulrak, many thanks for taking the time to look at this, and also for spotting this issue, much appreciated!

This is actually somewhat annoying because I was aware of the reserved memory on the classic COSMAC VIP and my own interpreter tries to emulate this, so I thought I had it covered in my change to this test suite. However looking back at my code I see that I am treating the first reserved memory address in this mode as 0EA0 (which is sufficient for the new ROM size), whereas you correctly point out that everywhere else it seems to be 0E90, which is not sufficient.

Checking documentation I can find, I must admit I'm a bit confused about this reserved memory address. For example both here and here state that same thing, which is that:

1. the first 512 bytes are reserved
2. the last 352 bytes are reserved
3. this leaves 3216 usable bytes from 4096
4. this means that only 0200 to 0E8F is usable (exactly as you say above)

I am happy with 1 and 2, I can see how 3 being true would imply 4, however I don't follow what should be the very simple maths getting from steps 1 and 2 to step 3. i.e. if we have 4096 bytes to start with and subtract reserved spaces of 512 and 352 bytes, then shouldn't this leave 3232 bytes rather than 3216 i.e. 16 bytes more usable RAM than Octo and everyone else seems to state? This is where my interpreter's apparently incorrect reserved address of 0EA0 rather than 0E90 came from.

Obviously this is me missing something rather than everyone else being wrong, but I can't work out what it is - do you have any idea what is accounting for the extra 16 bytes of reserved RAM, please, to make the sums above add up?

Anyway, returning to my test suite changes, yes this PR is no good if it causes the ROM size to increase beyond what a strict VIP emulator would support and that definitely wasn't my intention. So, I'll take another look at this today and see what can be done. Unfortunately this is my first experience with Octo Assembly Language so I'm very much a novice and relying on intuition here, so it may be the case that it needs someone more experienced than me to resolve this! However I'll have a look and report back :smile:

Thanks again!

[jon-axon]

Sorry, just one further follow-up to this, then I'll shut up :smile:

Firstly, regarding the start of the reserved memory block, I'm even more confused now because I've taken a look in the appendix of an original COSMAC manual and it seems to me that the memory map shown here shows that the reserved block should start from 0xEA0 as I originally thought rather than 0xE90 as everyone else is apparently using:

I've messaged Laurence Scotford through his website to ask about this as he seems to have the most detailed write-up, so I'll report back what he says.

Regardless, looking through the chip8-test-suite code, if we want to revert the jump quirk targets to 0xE87 and 0xE8B as they were before my PR, then one fairly trivial way to reclaim more than enough bytes to accommodate the other changes would be to alter the "expect" macros used in the flags test. Currently these macros increment the x coordinate register as their final instruction, which wastes bytes when used for the final opcode displayed on each row, as the next program instruction then resets this same register to the start of the next line. So, by moving the x increment instruction out of the macro and explicitly including it only where required, we reclaim 10 bytes total I believe - enough to accommodate the other changes - without altering program behaviour or appearance in any way.

I'm sure there are other, cleverer ways to reclaim some bytes; this is just one obvious and easy-to-implement way I could do it, if you're happy with it?

[gulrak]

Actually I think you are right. Not sure who inspired who, but indeed the stack growing down from 0xED0 and given as 48 bytes is only going to 0xEA0 leaving usable memory from 0x200 to 0xE9F. Nothing is using the area before the stack and 48 is actually quite well more than enough for the stack. So it looks like Octo is wrong. And somehow they calculated the size wrong. Not only does the real VIP not use that area from observing my implementation, also the original RCA COSMAC VIP Instruction Manual has a memory map on page 36, showing this:

---

Location	Use
0000 . . . 01FF	CHIP-8 LANGUAGE INTERPRETER
0200 . . .	User Programs using CHIP-8 instruction set (1184 bytes available in 2048)
0YA0 . . . 0YCF	CHIP-S stack (4S bytes max. for up to 12 levels of subroutine nesting)
0YD0 . . . 0YEF 0YF0 0YF1 0YF2 0YF3 0YF4 0YF5 0YF6 0YF7 0YF8 0YF9 0YFA 0YFB 0YFC 0YFD 0YFE 0YFF	Reserved for CHIP-S INTERPRETER work area V0 V1 V2 V3 V4 V5 V6 V7 V8 V9 VA VB VC VD VE VF
0X00 . . . 01FF	256-byte RAM area for display refresh

0X = Highest on-card RAM page (07 for 2048-byte system)
0Y = 0X - 1 (06 for 2048-byte system)

---

So for a 4096-byte system the highest page is F and thus X=F and Y=E, and the 1184 bytes given plus additional 2048 bytes RAM give indeed 3232 bytes of usable memory for the CHIP-8 program.

So I conclude it really is 0xEA0, where the reserved memory starts and will also update my internal documentation.

I guess we rather need to open an issue with Octo and CHIP-8 extensions and compatibility :wink:

[gulrak]

Ah, in the time I needed to make ascii art out of it you added a bitmap. Wasn't there when I started. :laughing:

[jon-axon]

:grinning: ASCII art always gets bonus points!

Many thanks for checking this yourself and validating that I'm not going mad! I'll open issues in the other places as you suggest.

I guess it probably makes sense to keep the jump quirks where they were in chip8-test-suite regardless if possible (by reclaiming bytes elsewhere) so I'll make that change anyway, I think.

[jon-axon]

@Timendus, although it's been a very useful discussion for other reasons i.e. the reserved memory issue, I think I'm going to abandon this PR for now. In hindsight I was too keen and rushed to a solution, and I think we can do it better. It seems arbitrary that I've implemented the extra checks for the CARRY path only and not the HAPPY path, when it could equally well be done in either/both. But then that would take up even MORE real estate if continuing with the current UI approach ... and more memory. So on reflection, if implementing these extra checks (which I definitely still think is worthwhile), it might need a more significant overhaul to do it "properly" rather than the quick fix I thought would work.

I'll give it some more thought as and when time allows and hopefully submit a new PR in due course (unless you or someone else beats me to it!)

[Timendus]

Great discussion and great to see the ripple effect of this change in those other repositories 😄