marhel / r68k

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

Diff MAME version of Musashi with current #58

Open emoon opened 8 years ago

emoon commented 8 years ago

There seems to have been a fair amount of changes over here at the MAME repro

https://github.com/mamedev/mame/commits/115ffcb10a45bca233fa6e22f674d8db8982b7df/src/emu/cpu/m68000

It might be worth taking a look and diff against the current one (here https://github.com/kstenerud/Musashi ) to make sure we don't implement something in bad way.

I don't think many of the MAME changes has made it back to the original repro.

marhel commented 8 years ago

Very interesting! I completely agree it's worth checking out!

emoon commented 8 years ago

:+1:

marhel commented 8 years ago

And the other way around, MAME still have the ADDA bug, it seems. https://github.com/mamedev/mame/blob/master/src/devices/cpu/m68000/m68k_in.cpp#L1248-L1254 and https://github.com/mamedev/mame/blob/master/src/devices/cpu/m68000/m68k_in.cpp#L1273-L1279 should evaluate OPER_AY before AX...

emoon commented 8 years ago

Ah.. Interesting :)

This is something that one needs to make sure of to handle also:

https://github.com/mamedev/mame/commit/b025f698a1c6a5c8dc4414f708f1f1699c0c2d02

emoon commented 8 years ago

For the disassembler I found quite a bit of issues as well when I worked on this

https://github.com/aquynh/capstone/blob/next/arch/M68K/M68KDisassembler.c

I know I submitted a few changes back but I missed quite a few I'm pretty sure of

emoon commented 8 years ago

Oh btw. Notice this clr on 000 https://github.com/mamedev/mame/commit/844e69607172242308670f67bb069bd6b5d0d40c

marhel commented 8 years ago

I've noticed that as well, but didn't care for now, as I feel it would only affect "real-world" devices.

emoon commented 8 years ago

Yeah I guess that is true.

emoon commented 8 years ago

And I guess this is taken into account with the cycle count? As I would assume this would make clr slower but I may be wrong.

marhel commented 8 years ago

If we where to add more accurate cycle counting based on actual RAM-accesses etc, then yes we would need to account for one less read-cycle. Today if the opcodehandler first access RAM (which should costs 4 cycles I think) and then cause an exception, we act as if the instruction was in fact "for free" and only count exception cycles. In real life, the actual cycles taken would depend on how much work the instruction had done before the exception happened, but cycles are not accounted for on that level.

marhel commented 8 years ago

Obviously a "fixed" implementation of CLR would do one more memory read, which would take non-zero time in the real world.

emoon commented 8 years ago

Yeah I wonder if a move (with a dn register set to zero) would be faster in the real-world then compared to a clr

emoon commented 8 years ago

I might actually test it at some point and see :)

marhel commented 8 years ago

Ah, you meant real-world CLR, yes this "feature" makes the instruction 4 clocks slower than absolutely necessary, which you should be able to see if comparing the cycle counts for CLR in the 010, which doesn't do a useless read first.

emoon commented 8 years ago

Yeah I just wonder if a move d0,(a0)+ would be faster than clr.w (a0)+ in the real-world.

marhel commented 8 years ago

Well, either way that should be evident if you compare cycle counts for the two, see the M68000UM section 8, which I don't have time to do now ;)

emoon commented 8 years ago

Yeah, will do :)

emoon commented 8 years ago

Looks like its taking the same amount of cycles (12)

marhel commented 8 years ago

Oops, they have fixed the adda-bug, I now notice!

emoon commented 8 years ago

Ah :) Yeah it would be interesting to just diff the code but it seems they have C++:ified the code so that might prove a bit hard :/

marhel commented 8 years ago

I took a look yesterday, and there seems to be history back to dec 2007, using Musashi 3.31 according to the source code. In particular, one interesting change, is fixing the dependencies on global functions preventing the use of several different Musashi-cores at the same time. Remaining global functions takes a CPU-core as an argument, I think.

Maybe, with some work, it would be possible to both use latest MAME-Musashi (4.90 i think), which I guess would include a few fixes not in https://github.com/kstenerud/Musashi, as well as remove the globals-depencencies that force us to serialize Musashi access with a MUTEX. This would improve test performance greatly, allowing multithreaded tests.

Don't know how much dependencies to the rest of MAME that has crept into the codebase, though.

emoon commented 8 years ago

Yeah that would be nice. I really never liked that Musashi has global functions with no context so yes that would be nice to get it.

As MAME is multi cpu I would imagine they keep the code fairly separate and just don't throw lots of specific stuff in there but you never know.

emoon commented 8 years ago

You didn't find any other opcode specific fixes? (I haven't looked myself)

marhel commented 8 years ago

No,, but I didn't spend too much time looking either. It seemed that the adda fix was just a side effect of removing a lot of macros. Several fixes to 020+ though, including actually emulating 020+ and ColdFire

emoon commented 8 years ago

Alright. Good to know.

marhel commented 8 years ago

And I think they added some more event callbacks (one for TAS in particular, which apparently uses a different bus-cycle in real-world, which can have HW-side-effects which might need to be emulated), as well as allowing bus errors to be triggered externally, and some fixes to exception stack formats IIRC.

emoon commented 8 years ago

I see. I guess some Arcades/games may have relied on it (both by intention and non-intention)

marhel commented 8 years ago

Good guess. They specifically mentioned two games that actually depended on the TAS write not to succeed (apparantly, the callback allows the host to specify if the write should be allowed or not).

emoon commented 8 years ago

Ah I see... Must have been "fun" to track down :)