-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SPC700 tests never start with H-flag set #18
Comments
Excellent, thanks for finding this! I fixed & pushed. Out of curiosity, do you have an SPC700 core that passes or is in conflict with the tests? SPC700 isn't super widely used unlike some, so having people test these is a great blessing. |
I do now have a SPC700-core that passes all of the (old) tests (haven't tested with the new set yet) as part of (eventually) getting back to working on my SNES emulator LakeSnes. While getting the tests to run I did run into some possible problems:
|
Quick reply from my phone, a little history
The SPC700 is part of a larger die with the DSP. Because of this it’s hard to use a logic analyzer on it. It’s a clean-room clone of the 6502 so although it’s similar, it does have differences. Over about a decade, Near/Byu and many others collaborated to reverse engineer it to the limits of emulation. That is to say, programs written to test cycle by cycle perform the same on Higan as on a real SPC700 in every possible measurable way. Every document has errors, including the Overload document, which I’ve seen specific references to, but the code stands as a sort of living documentation made over 10+ years for the best info we have on the SPC700. This is all captured in a bunch of forum threads.
For these reasons, both the fact it’s reverse engineered and it’s got so much work into it, I would side with Higan’s behavior as opposed to what you’d expect from a 6502 or find in any document.
The null values on idle are intended, because the value of the bus at that point doesn’t seem relevant. I believe this is similar to the behavior on the Z80 where the leads are left floating, but in the end it makes no difference - no hardware acts on it in any measurable way, a duplicate read or write is not performed - so I used null to describe that. If something like djnz should be doing a real read there, that’s an error on my part, and the cycle is marked incorrectly.
I definitely made mistakes translating from Higan. Validating against my core helped me catch a ton of them but probably not all. Especially since my SPc700 core doesn’t go cycle by cycle! But if there’s an error, my reference is Higan, in tribute to the amazing work done by a community of people over a long period of time.
As for WAI and STP, the length of that is a test-generation-time option. They don’t technically ever end, and we don’t have a way to know a lot about them for sure, since interrupts aren’t used in the SNES and nothing else ever used the CPU. You can’t execute one and carefully time an IRQ or something; since none ever occurs, you have effectively soft-locked the processor I almost didn’t even include them in the tests. In the end I figured, make it a generation-time option.
Why 7 cycles though? Well…the 65816 had a similar instruction, that actually pauses (IIRC) on cycle 2, kinda like repeating cycle 3 over and over, so it’s not exactly 3 cycles, it’s more like, 3 cycles until it will respond to IRQ. It’s of indeterminate length, in an infinite loop polling IRQ/RST. With no other reference to go from I just copied that behavior and said, hmm, 7 cycles sounds good. If your emulator is doing WAI and STP properly the tests should still pass, but you might get a cycle count difference error. I’m open to alternate suggestions.
I will double check behavior of specific instructions you’ve listed later, thanks so much for the thoughtful reply!
|
So investigating 0xFE, we have a test where.. cycle 1: read instruction IIRC I remember reading a thread about this. This behavior was determined by reading from the built-in timers in the SNES and using that as a branch offset, to measure which cycle the read actually latched on. I can't come up with source but I'm fairly confident in it. This gels with the Higan/Ares source. Mesen-S also agrees with this behavior, and SNES9x disagrees. In regards to EOR A, [dp]+Y aka opcode 0x57, the tests agree with Higan and SNES9x but disagree with Mesen-S. Higan and SNES9x include separate code for this difference specifically, whereas Mesen-S re-uses the same addressing code between opcodes. Probably this specific thing could be a generation-time option if you'd like, unless I track down the thread on the topic. Or look at git blame for those lines...It would certainly be easy enough to make it so. If it's good enough for SNES9x, it should be good enough for almost everyone else. |
I'm going to mark this as closed due to lack of movement |
The generated SPC700 tests never start with the H-flag set, meaning opcodes that depend on its state are not tested fully for correctness. This affects DAA (0xdf), DAS (0xbe) and CLRV (0xe0). Additionally, other opcodes could clear it accidentally and not be caught by the tests as they are now.
It looks like this is caused by the generator wanting to never start tests with the B-flag set, but masking off the wrong bit in the PSW by mistake. It's probably better to not mask off any bits, as never starting with the B-flag set would mean it doesn't check for opcodes accidentally clearing it.
The text was updated successfully, but these errors were encountered: