marhel / r68k

A m68k emulator in rust - let r68k = musashi.clone();
MIT License
72 stars 7 forks source link

Implement SUBA #48

Closed marhel closed 8 years ago

marhel commented 8 years ago

Please implement the instruction for Subtract Address. See implementation of ADDA. Also, please consider implementing related SUB-instructions at the same time. For more information, please read the instruction contribution guidelines

emoon commented 8 years ago

I will take this one

emoon commented 8 years ago

I'm trying to implement this but the test for suba_16_pd is failing. I wonder if Musashi may have a similar bug as ADDA (or was that bug for ADDA only?)

emoon commented 8 years ago

My code might of course be wrong but just want to check

marhel commented 8 years ago

I haven't looked, but if you take a look at the PR fixing adda, the problem should be explained in enough detail for you to check Musashis suba implementation

marhel commented 8 years ago

If pd and pi both fail (sometimes, as it is random based) but never the others, I'd say it's very likely there's a bug in Musashi suba as well

emoon commented 8 years ago

Yeah if you look at the mame code:

https://github.com/mamedev/mame/blob/master/src/devices/cpu/m68000/m68k_in.cpp#L9590

And the original

https://github.com/kstenerud/Musashi/blob/master/m68k_in.c#L9455

it's likely a bug in SUBA also

emoon commented 8 years ago

I'm running all the tests except pd and pi to verify that the current code work as expect with it.

marhel commented 8 years ago

When I have time, I can fix suba in Musashi locally, abd retest

emoon commented 8 years ago

That would be great.. Thanks!

I will hold of with my suba PR right now and do something else in between.

marhel commented 8 years ago

No, please do the PR, it will help me verify the fix to Musashi, without duplicating your work. If there's still something, you can always update the PR before I merge.

emoon commented 8 years ago

Done