PowerPC does exceptions and hardware and stuff, Memory doesn't.
I did not realize until a few minutes ago that there were two versions of these functions. This is why namespaces suck. Anyway, these were added by Mullin earlier this year.
They weren't sufficient and are made redundant by previous commits; they
also (on master) caused breakage due to Jit64::psq_stXX assuming writes
would be fastmem and not clobber a register under certain conditions.
That really needs to be refactored, but for now, this works.
Change TMemCheck::Action to return whether to break rather than calling
PPCDebugInterface::BreakNow, as this simplified the implementation; then
remove said method, as that was its only caller. One "interface" method
down, many to go...
Without fastmem, the JIT code still does an inline check for RAM
addresses. With watchpoints we have to disable that too. (Hardware
watchpoints would avoid all the slow, but be complicated to implement
and limited in number - I doubt most people debugging games care much if
they run slower.)
With this change and watchpoints enabled, Melee runs at no more than 40%
speed, despite running at full speed without them. Oh well. Better
works slowly than doesn't bloody work.
Incidentally, I'm getting an unrelated crash in
PowerPC::HostIsRAMAddress when shutting down a game. This code sucks.
- Move JitState::memcheck to JitOptions because it's an option.
- Add JitOptions::fastmem; switch JIT code to checking that rather than
bFastmem directly.
- Add JitBase::UpdateMemoryOptions(), which sets both two JIT options
(replacing the duplicate lines in Jit64 and JitIL that set memcheck
from bMMU).
- (!) The ARM JITs both had some lines that checked js.memcheck
despite it being uninitialized in their cases. I've added
UpdateMemoryOptions to both. There is a chance this could make
something slower compared to the old behavior if the uninitialized
value happened to be nonzero... hdkr should check this.
- UpdateMemoryOptions forces jo.fastmem and jo.memcheck off and on,
respectively, if there are any watchpoints set.
- Also call that function from ClearCache.
- Have MemChecks call ClearCache when the {first,last} watchpoint is
{added,removed}.
Enabling jo.memcheck (bah, confusing names) is currently pointless
because hitting a watchpoint does not interrupt the basic block. That
will change in the next commit.
On OS X, this broke Cmd-V to paste in the text boxes. Apparently wx
thinks having mnemonics (which are Alt-* on Windows) be Cmd-* on OS X,
even if this disables standard shortcuts, is a good idea.
Lioncash suggested just getting rid of the accelerators on non-menu
controls, so I'm doing that rather than disabling them only on OS X.
1) Apparently wxString::Format is type safe, and passing a u32 to it
with the format "%lu" crashes with a meaningless assertion failure.
Sure, it's the wrong type, but the error sure doesn't help...
2) "A MenuItem ID of Zero does not work under Mac". Thanks for the
helpful assert message, no thanks for making your construct have random
platform-specific differences for no reason (it's not like menu item IDs
directly correspond to a part of Cocoa's menu API like they do on
Win32).
This was causing a race condition where the "absurdly large aux buffer"
panic alert would be triggered in the last bit of fifo processing on the
CPU thread in deterministic mode (i.e. netplay). SyncGPU is supposed to
move the auxiliary queue data to the beginning of the containing buffer
so we don't have to deal with wraparound; if GpuRunningState is false,
however, it just returns, because it's set to false by another thread -
thus it doesn't know whether RunGpuLoop is still executing (in which
case it can't just reset the pointers, because it may still be using the
buffer) or not (in which case the condition variable it normally waits
for to avoid the previous problem will never be signaled). However,
SyncGPU's caller PushFifoAuxBuffer wasn't aware of this, so if the
buffer was filling at just the right time, it'd stay full and that
function would complain that it was about to overflow it. Similar
problem with ReadDataFromFifoOnCPU afaik. Fix this by returning early
from those as well; other callers of SyncGPU should be safe. A
*slightly* cleaner alternative would be giving the CPU thread a way to
tell when RunGpuLoop has actually exited, but whatever, this works.
Core::Shutdown was only called on app exit, yet the emu thread exits
whenever emulation stops; if you launched a new game it would just join
via the destructor when s_emu_thread was set to a new thread.
(Incidentally, the destructor also makes explicitly joining on app exit
rather pointless.)
Because the GUI thread wasn't waiting for the CPU thread to fully shut
down, Core::IsRunning would remain true briefly after CFrame::DoStop
which, given Dolphin's penchant for accessing variables belonging to
other threads, can only mean trouble... In my case, because the previous
commit caused UpdateGUI, which is called at the end of DoStop, to call
PauseAndLock, which checks IsRunning, pressing stop at the right time
would cause strange behavior.
Since its lifetime is managed on the CPU thread, this (or a refactoring)
is absolutely required. One of the functions with a PauseAndLock call
added is CFrame::UpdateGUI; this is fine now, since it's called only
after important events happen, so just make sure not to call it every
frame or something :)
My recent update to that check broke compilation on 10.9:
https://code.google.com/p/dolphin-emu/issues/detail?id=7900
However, on further review, the check isn't actually necessary. If the
OS X version is older than LSMinimumSystemVersion in Info.plist, the
system will generally refuse to run the binary in the first place. You
can try to launch it via a terminal, but at that point it's the user's
problem if it crashes.
Also, this avoids keeping the system awake if a game is not being
played.
Frankly, I don't know what the point of precisely tracking these things
is, but that's how the API works. Feel free to add analogous
functionality on other platforms.
There was a longstanding hack that defined ucontext_t manually to work
around the lack of this header on the Android NDK. However, it looks
like newer NDK versions now have it like good little POSIX boys, and my
recent header reshuffle broke the build on those versions, presumably
because the real and fake definitions of ucontext_t end up included in
the same file where they weren't under the old organization.
Rather than try to revert the conflict, this commit just removes the
hack. The buildbot's NDK will need to be upgraded.
Regression by 3fed975bac caused netplay to
crash on OS X. While I'm at it, fix the long-standing "unsafe i guess"
AddPendingEvent, since we depend on wx 3 now...
Changing PATH basically screws everything up, and while the attempt to
avoid MacPorts copies of system libraries was well-intentioned, it made
the OS X buildbot unable to pick up ffmpeg and libusb.
It's sufficient to put /usr first to make sure we use the system copies
of duplicated libraries.
m_strGPUDeterminismMode can be set by either the global or game
settings. Either way, it's then supposed to be parsed into an enum,
m_GPUDeterminismMode. However, the code to do this was placed right
after checking for game settings, which doesn't happen at all if there
isn't a valid title ID. Move it outside the if block.
- Get rid of ArmMemTools.cpp and rename x64MemTools.cpp to MemTools.cpp.
ArmMemTools was almost identical to the POSIX part of x64MemTools, and
the two differences, (a) lack of sigaltstack, which I added to the
latter recently, and (b) use of r10 to determine the fault address
instead of info->si_addr (meaning it only works for specifically
formatted JIT code), I don't think are necessary. (Plus Android, see
below.)
- Rename Core/PowerPC/JitCommon/JitBackpatch.h to Core/MachineContext.h.
It doesn't contain anything JIT-specific anymore, and e.g. locking
will want to use faulting support regardless of whether any JIT is in
use.
- Get rid of different definitions of SContext for different
architectures under __linux__, since this is POSIX. The exception is
of course Android being shitty; I moved the workaround definition from
ArmMemTools.cpp to here.
- Get rid of #ifdefs around EMM::InstallExceptionHandler and just
provide an empty implementation for unsupported systems (i.e.
_M_GENERIC really). Added const bool g_exception_handlers_supported
for future use; currently exception handlers are only used by the JIT,
whose use implies non-M_GENERIC, but locking will change that.
- Remove an unnecessary typedef.
This is a higher level, more concise wrapper for bitsets which supports
efficiently counting and iterating over set bits. It's similar to
std::bitset, but the latter does not support efficient iteration (and at
least in libc++, the count algorithm is subpar, not that it really
matters). The converted uses include both bitsets and, notably,
considerably less efficient regular arrays (for in/out registers in
PPCAnalyst).
Unfortunately, this may slightly pessimize unoptimized builds.
std::array does not have an initializer list constructor, instead (for
some reason) being defined to contain one public array member, allowing
it to be directly initialized. Thus the most explicit way to initialize
it is with two braces, one for the struct and one for the array. C++
allows the second pair of braces to be omitted, but clang complains
about it.
This is good hygiene, and also happens to be required to build Dolphin
using Clang modules.
(Under this setup, each header file becomes a module, and each #include
is automatically translated to a module import. Recursive includes
still leak through (by default), but modules are compiled independently,
and can't depend on defines or types having previously been set up. The
main reason to retrofit it onto Dolphin is compilation performance - no
more textual includes whatsoever, rather than putting a few blessed
common headers into a PCH. Unfortunately, I found multiple Clang bugs
while trying to build Dolphin this way, so it's not ready yet, but I can
start with this prerequisite.)
I found it via clang complaining about a useless null check on an array,
but I decided to get rid of the array in favor of dynamic allocation, as
there was no reason to assume a maximum length of 0x32 bytes. Plus, add
a CFString type check just in case, and switch to UTF-8 in the
off-chance it matters.
The result has not actually been tested, as I have no CD drive.
comex