Skip to content
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

sm83: Possible errors in tests #19

Closed
SlyMarbo opened this issue Dec 31, 2022 · 3 comments
Closed

sm83: Possible errors in tests #19

SlyMarbo opened this issue Dec 31, 2022 · 3 comments

Comments

@SlyMarbo
Copy link

Thanks for such a detailed set of tests! They're incredibly helpful.

It's quite possible that I've misunderstood, but I think three of the test files have errors:

  1. The tests for 0xE9 (JP (HL)) have a RAM entry for the initial program counter (0xE49E / 58526 in the first test case), instead of the HL register (0xBC84 / 48260 in the first test case). This means that the emulator will find an arbitrary value in (HL) and jump to an arbitrary address. This appears to be true for all of the 0xE9 test cases.
  2. The tests for 0xFB (EI) expect the EI instruction to ignore IME and set the IE register. My reading of the manual on page 32 suggests that this is incorrect and the instruction should instead ignore IE and set IME. This would be consistent with the corresponding tests for 0xF3 (DI). I was slightly confused by the presence of the IE register in the tests, as the README says that memory is flat and registers are ignored.
  3. A much more minor issue is that the final states for the tests for 0xFE (CP d8) appear to be missing a value for the IE register. This caused my tests to fail as they were reusing the value from previous tests. I fixed this by more robustly resetting the test state, but it would be nice not to need to. If I've understood correctly, the IE register could be removed from all tests, which would also work.

Thanks again for all your work. Apologies if I've misunderstood anything.

@raddad772
Copy link
Owner

re: #2. You're definitely correct, IE/IME is not handled right in the tests. I wrote these before I understood what I was doing inn regards to interrupts. I've needed to fix it but haven't had the motivation, now I do, thanks! Keep in mind, setting IME to 1 has a delay to it.
re: #3 Yes IE is not relevant to these tests, I should remove it.

re: #1. not sure I understand. Are you saying you think the program counter should be where the HL register points? JP HL aka JP (HL) (thanks Zilog) is a 1-byte 1-M-cycle opcode that sets PC to HL. That 1 byte in RAM that the PC is pointed to should be the 0xE9 instruction. It should then set PC = HL. Does the test not do this?

JP (HL)/JP HL is a bit confusing due to borrowing syntax from the Z80 which was also a little confusing on this

@SlyMarbo
Copy link
Author

re: #1, my understanding is that JP (HL) follows these logical steps:

  1. Read the 16-bit value in the HL register, which we'll call value X.
  2. Fetch the 16-bit value at memory address X, which we'll call value Y.
  3. Set the program counter to Y.

Assuming that's correct, I would expect the test case to have one RAM entry, where the address is equal to the contents of HL (X). The value at that address (Y) is what we should expect the program counter to be at the end of the test. As it is, the tests appear to have one RAM entry, but the address is equal to the program counter's initial value. This means that in step 1 above, the read of HL will return some value X, but then in step 2, the memory lookup will not find the RAM entry from the test (as the address is wrong). I hope that's a bit clearer.

Thanks again.

@raddad772
Copy link
Owner

I've got multiple independent confirmation that it is done JP (HL) correctly. I'll open a new Issue about the IME/IE register issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants