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

Implement SUBA #48

Closed
marhel opened this issue Feb 2, 2016 · 11 comments
Closed

Implement SUBA #48

marhel opened this issue Feb 2, 2016 · 11 comments

Comments

@marhel
Copy link
Owner

marhel commented Feb 2, 2016

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
Copy link
Contributor

emoon commented Feb 2, 2016

I will take this one

@emoon
Copy link
Contributor

emoon commented Feb 3, 2016

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
Copy link
Contributor

emoon commented Feb 3, 2016

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

@marhel
Copy link
Owner Author

marhel commented Feb 3, 2016

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
Copy link
Owner Author

marhel commented Feb 3, 2016

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
Copy link
Contributor

emoon commented Feb 3, 2016

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
Copy link
Contributor

emoon commented Feb 3, 2016

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

@marhel
Copy link
Owner Author

marhel commented Feb 3, 2016

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

@emoon
Copy link
Contributor

emoon commented Feb 3, 2016

That would be great.. Thanks!

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

@marhel
Copy link
Owner Author

marhel commented Feb 3, 2016

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 emoon mentioned this issue Feb 3, 2016
@emoon
Copy link
Contributor

emoon commented Feb 3, 2016

Done

@marhel marhel closed this as completed Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants