Fixing Nestest failiures

Discuss emulation of the Nintendo Entertainment System and Famicom.

Moderator: Moderators

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Fixing Nestest failiures

Post by aphex » Tue Jul 20, 2010 7:34 am

Hi everyone
Here is a screenshot of the nestest.nes rom completed in my emulator

Image

As you can see, many of the tests fail, but the test numbers that are indicated are mostly all just general failiures, such as "SBC # Failiure", so it's difficult to track down exactly what is wrong. I'll start with the first error, 73. This is listed here as an SBC # Faliure. Here is my SBC instruction as it stands at the moment.

Code: Select all

void NesMainClass::_SBC(u8 toSub, int cycles) 
{
	cpuCycles += cycles;

	// Store the A register 
	u8 beforeSub = regs->A;	

	// Do the subtraction
	regs->A -= toSub;

	// Subtract one more if carry is clear
	if (!TestBit(regs->P, FLAG_C))
		regs->A--;

	// Is the Negative/Sign bit set?
	regs->P = (TestBit(regs->A, 7)) ? 
		SET_N_FLAG : CLR_N_FLAG;

	// Is the result zero?
	regs->P = (regs->A == 0) ? 
		SET_Z_FLAG : CLR_Z_FLAG;

	// Was there a borrow
	regs->P = (beforeSub - toSub >= 0) ? 
		SET_C_FLAG : CLR_C_FLAG;

	// The Overflow flag works by indicating whether there was a sign overflow
	// i.e The result was outside the range of -128 to 127 
	regs->P = (regs->A > 127 || (s8)beforeSub + TestBit(regs->P, FLAG_C) -1 - (s8)toSub < -128) ?
		SET_V_FLAG : CLR_V_FLAG;
}	
Assuming all the various addressing modes are correct, the only problem i can think of is that the overflow being set for a signed underflow as well. (ie should only be set if the result is more that 127 and not less than -128 as well? Im not sure...). I really can't see the problem here, can anyone else spot any problems with this code?

thanks :)

pdq
Posts: 6
Joined: Fri Jun 25, 2010 6:15 am

Post by pdq » Tue Jul 20, 2010 11:35 am

It's much easier if you grab the golden nestest log from here and compare your CPU state against it, instruction by instruction.

NOTE: you will need to set your PC to $C000 (ie bypass the reset/power interrupt vectors).

http://wiki.nesdev.com/w/index.php/Emulator_tests

User avatar
James
Posts: 429
Joined: Sat Jan 22, 2005 8:51 am
Location: Chicago, IL
Contact:

Post by James » Tue Jul 20, 2010 12:38 pm

This topic comes up every once in a while and I'm always tempted to reply with my take on it.  Here goes:

First of all, I don't understand how you're setting the processor flags (looks like regs->P can be overwritten by subsequent tests).  In any event, I think your overflow logic is off.

The overflow flag is set when the result is outside of the signed char range (i.e., > 127 or < -128).  With SBC, an overflow can only happen when the signs, and hence the high bit, of the operands differ.  A positive number subtracted from a positive number, or a negative number subtracted from a negative number, will never overflow.  Examples:

5 - 127 = -122 (no overflow)
-5 - -128 = 123 (no overflow)
5 - -128 = 133 (overflow -- signed result is -123, so high bit is 1)
-5 - 127 = -132 (overflow -- signed result is 124, so high bit is 0)

Plug in various numbers and you'll see that you can't make 2 same-signed numbers overflow.

With this knowledge, you can implement the following logic: SBC performs A - M - (carry ? 0 : 1).  If the signs (high bit) of A and M differ and the signs of A and the result differ, then overflow has occurred:

overflow = (A^M) & (A^result) & 0x80 ? true : false;

For ADC (A + M + (carry ? 1 : 0)), only 2 positive, or 2 negative, numbers can result in overflow.  Therefore, you need to test that the signs of both A and M differ from sign of the result:

overflow = (A^result) & (M^result) & 0x80 ? true : false;

Hope this helps,
James

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Tue Jul 20, 2010 5:19 pm

@pdq

I have already done this, and I actually have programmed my emulator to output an identicaly formatted log file to the nestest.log file. Although, the contents is different at this point obviously.

@James

To make things clearer, take this for example (but you're right lol)

Code: Select all

regs->P = (TestBit(regs->A, 7)) ? SET_N_FLAG : CLR_N_FLAG;
SET_N_FLAG is a macro which is defined like so.

Code: Select all

(regs->P |= 0x80)
CLR_N_FLAG is a macro which is defined like so.

Code: Select all

(regs->P &= 0x7F)
So, to put another way, i'm simply saying.

Code: Select all

if (TestBit(regs->A, 7)) 
{
    regs->P = (regs->P |= 0x80);
}
else
{
    regs->P = (regs->P &= 0x7F);
}
...and you know what? You just made me realise that I am not setting the flags wrong, but i am doing something pointless. The most logical way to do this would be like this.

Code: Select all

(TestBit(regs->A, 7)) ? SET_N_FLAG : CLR_N_FLAG;
Sorry about that (and for going on about it), I just recently re-wrote all the flag logic so i missed that, thanks for pointing it out. I was basically assigning regs->P to itself for no reason... :oops:

Anyway, as for my SBC problem, thanks alot, that seemed to fix my problem. Here is my new flag test in SBC

Code: Select all

	((beforeSub^toSub) & (beforeSub^regs->A) & 0x80) ? 
		SET_V_FLAG : CLR_V_FLAG;
Although, now its replaced with another problem, "035h - CPX # failure (messed up flags)".

Any ideas? I can't figure it out. My flags seem fine, here is my CMP instruction.

Code: Select all

void NesMainClass::_CMP(u8 cmpReg, u8 toCmp, int cycles)
{
	cpuCycles += cycles;	

	cmpReg -= toCmp;

	// Is the Negative/Sign bit set?
	(TestBit(cmpReg, 7)) ? 
		SET_N_FLAG : CLR_N_FLAG;

	// Is the result zero?
	(cmpReg == 0) ? 
		SET_Z_FLAG : CLR_Z_FLAG;

	// Should we set the carry?
	((s8)cmpReg >= 0) ? 
		SET_C_FLAG : CLR_C_FLAG;
}
Here is where the immediate CPX is called.

Code: Select all

case 0xE0: _CMP(regs->X, readMem(regs->PC), 2); regs->PC++; break;
Any ideas? Im sure my flags are correct in my cmp instruction, ill keep checking.

Thanks alot for your help :)

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Tue Jul 20, 2010 5:26 pm

Ok, i think i got the CPX flags problem fixed.

Code: Select all

void NesMainClass::_CMP(u8 cmpReg, u8 toCmp, int cycles)
{
	cpuCycles += cycles;	

	// Is the Negative/Sign bit set?
	(TestBit(cmpReg-toCmp, 7)) ? 
		SET_N_FLAG : CLR_N_FLAG;

	// Is the result zero?
	(cmpReg-toCmp == 0) ? 
		SET_Z_FLAG : CLR_Z_FLAG;

	// Should we set the carry?
	(cmpReg-toCmp >= 0) ? 
		SET_C_FLAG : CLR_C_FLAG;
}
Now im getting an ADC error (my emulator is so buggy lol). Im getting "022h - ADC # failure". Here is my ADC instruction

Code: Select all

void NesMainClass::_ADC(u8 toAdd, int cycles) 
{
	cpuCycles += cycles;

	// Store the A register so we can check if there was a carry 
	u8 beforeAdd = regs->A;

	// A reg = A reg   +          Carry           + some value
	regs->A = (regs->A + TestBit(regs->P, FLAG_C) + toAdd);

	// Is the Negative/Sign bit set?
	(TestBit(regs->A, 7)) ? 
		SET_N_FLAG : CLR_N_FLAG;

	// Is the result zero?
	(regs->A == 0) ? 
		SET_Z_FLAG : CLR_Z_FLAG;

	// Was there a carry
	((beforeAdd + toAdd) > 255) ? 
		SET_C_FLAG : CLR_C_FLAG;

	// Set overflow?
	((beforeAdd^regs->A) & (toAdd^regs->A) & 0x80) ?
		SET_V_FLAG : CLR_V_FLAG;
}
I'll keep seeing if i can fix it, but open to any ideas. Thanks :)

pdq
Posts: 6
Joined: Fri Jun 25, 2010 6:15 am

Post by pdq » Tue Jul 20, 2010 5:38 pm

If you have your nestest log, then you should just post the failing instruction along with the golden one here. It's much more obvious as to exactly what you got wrong.

Your carry seems borked because a u8 - u8 is always >= 0. Have you tried this instead?

Code: Select all

(cmpReg>=toCmp) ? 
      SET_C_FLAG : CLR_C_FLAG;

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Tue Jul 20, 2010 6:44 pm

a u8 - u8 is always >= 0
I beg to differ, in C++ if you to the sum 10 - 15, you will get -5, unless you typecast the result of course. For example, this will evaluate as true.

Code: Select all

if (10-15 < 0)
As for the nestest log, I will show you the first part that isn't the same. (ignoring scanline and small cycle differences)

Golden nestest.log, Starting at line 102

Code: Select all

C824  48        PHA                             A:FF X:00 Y:00 P:AD SP:FB CYC: 86 SL:243
C825  28        PLP                             A:FF X:00 Y:00 P:AD SP:FA CYC: 95 SL:243
C826  D0 09     BNE $C831                       A:FF X:00 Y:00 P:EF SP:FB CYC:107 SL:243
My nestest log, starting at line 102

Code: Select all

C824  48        PHA                             A:FF X:00 Y:00 P:AD SP:FB CYC:90 SL:1
C825  28        PLP                             A:FF X:00 Y:00 P:AD SP:FA CYC:102 SL:1
C826  D0 09     BNE $C831                       A:FF X:00 Y:00 P:FF SP:FB CYC:108 SL:1
As you can see, A is pushed on to the stack (which contains FF), then it is pulled from the stack in to the flags register, (which should now = FF right?). In the golden nestest log, even though FF is pushed on to the stack (PHA) and that byte is pulled back in to the flags register, EF appears in the flags register. ie the brk flag is unset.

Is this just my incompitance in not handling the brk flag correctly? Should the brk flag only be set by the brk instruction and never be set b any other instruction?

pdq
Posts: 6
Joined: Fri Jun 25, 2010 6:15 am

Post by pdq » Tue Jul 20, 2010 7:22 pm

Is this just my incompitance in not handling the brk flag correctly? Should the brk flag only be set by the brk instruction and never be set b any other instruction?
Yes, in fact you shouldn't even implement the B flag in your P reg (it's always zero). However, when P is pushed onto the stack for BRK or PHP, the byte is ORed so that B is set.

User avatar
blargg
Posts: 3715
Joined: Mon Sep 27, 2004 8:33 am
Location: Central Texas, USA
Contact:

Post by blargg » Tue Jul 20, 2010 8:31 pm

First thing you need to do is put your log and the nestest log in the same format. In your case, I'd strip off all the timing information after the registers. Then use a text file comparison tool that shows mismatching lines. Doing it manually is a very bad idea. You'll miss differences and spend lots of time doing something humans aren't suited for.

EDIT: whoops, they are in the same format. The wrapped lines threw me off. :)
Last edited by blargg on Tue Jul 20, 2010 10:37 pm, edited 1 time in total.

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Tue Jul 20, 2010 9:32 pm

Hi Blargg
Both logs are basically in the exact same format now, and I'm actually making some great progress since my first post, I now pass 10 of the 13 tests and lots of games that didn't boot before are starting to work.

Image

Something I realised is that the Indirect Indexed instructions such as LDA (nn,X), suffers from the same bug as JMP Indirect when fetching the high byte of the word addres from past a page boundry. This isn't documented anywhere as far as I'm aware, maybe it should be put on the nesdev wiki?

Anyway, your advice is good and has come to mind to me too, but will be more useful when I have ironed out more bugs as the differences in the two files are fairly easy to track down at the moment. This is mainly due to the fact that they are "fairly" small in size and that the errors are pretty much contiguous at the moment.

I will likely find more issues with my instruction set so i'll keep posting in this thread if need be.

Thanks for all your help everyone

User avatar
blargg
Posts: 3715
Joined: Mon Sep 27, 2004 8:33 am
Location: Central Texas, USA
Contact:

Post by blargg » Tue Jul 20, 2010 10:41 pm

Whoops, you're right. The line-wrapping of the logs threw me off.

It's documented in most 6502 references that zero-page addressing wraps around. LDX #1 : LDA $FF,X will load from $0000, not $0100, for example.

Once you pass nestest, be sure to run my instruction tests as well (the other ones on the Wiki page).

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Wed Jul 21, 2010 11:53 am

Blargg, I tried your instruction tests as well, but something is going very wrong in my emulator. Here is a screenshot of the rom single "01-implied.nes", rom. (same story for all the rom singles)


Image

After a short time, I hit an illegal instruction. I'm positive it's some memory writes to the nametables that are going wrong here and not my scanline rendering routine. I'll have to resolve my final two errors in your nestest.nes rom first, but have you got any ideas about which instruction(s) might be causing this, as this is a common problem with a fair few games. On the bright side, I passed 11 out of the 13 tests now.


Image

User avatar
blargg
Posts: 3715
Joined: Mon Sep 27, 2004 8:33 am
Location: Central Texas, USA
Contact:

Post by blargg » Wed Jul 21, 2010 8:40 pm

As stated on the Wiki, it's best to get nestest passing first. Since you have an instruction log, it's easy to figure out why it's failing. After that's passing all the official instructions at least, then move on to mine. If your PPU isn't working, take a look at the readme, which explains other ways you can read the result. For one, there's audio beeps. If your APU doesn't work, then you can examine $6000-$7FFF to obtain an ASCII version of the output, that lists failed instructions.

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Wed Jul 21, 2010 8:57 pm

As stated on the Wiki, it's best to get nestest passing first.
Yes, I will get the last two tests fixed (hopefully) first before I move on to other test roms.

As for my PPU emulation, the only problem (optimistically) is that incorrect data is been written to the nametables. As you said though, i would need to make sure that I'm passing all the basic instruction tests before I should go any deeper in to these more specific problems.

I'm just getting into APU emulation, so I'll probably be asking some questions on this soon, although sound emulation isn't as scary as I once thought it was :?

Thanks Blargg

aphex
Posts: 25
Joined: Thu Apr 22, 2010 1:35 pm
Location: England

Post by aphex » Thu Jul 22, 2010 2:55 am

My emu finally passed all the tests :D

Image

Thanks so much for all the help, this is a major mile stone for me as alot of the games are booting and playing fine, and of course, thanks for writing the nestest Blargg.

Now, the other instruction tests are running ok, although, I am getting lots of errors. for example, here is the rom single, "01-implied.nes"

Image

Now all these are showing that there is some sort of problem with those instructions, correct?

Post Reply