This was caused, because we were saving the `break_on_hit` flag with the letter `p`. Then while loading the breakpoints, we read the flag with the letter `b`, resulting in the `break_on_hit` flag being always false
Filesystem accesses aren't magically faster when they are done by ES,
so this commit changes our content wrapper IPC commands to take FS
access times and read operations into account.
This should make content read timings a lot more accurate and closer
to console. Note that the accuracy of the timings are limited to the
accuracy of the emulated FS timings, and currently performance
differences between IOS9-IOS28 and newer IOS versions are not emulated.
Part 1 of fixing https://bugs.dolphin-emu.org/issues/11346
(part 2 will involve emulating those differences)
This makes it more convenient to emulate timings for IPC commands that
perform internal IOS <-> IOS IPC, for example ES relying on FS
for filesystem access.
According to hwtests, older versions of IOS are slower at performing
various filesystem operations:
https://docs.google.com/spreadsheets/d/1OKo9IUuKCrniz4m0kYIaMP_qFtOCmAzHZ_zAmobvBcc/edit
(courtesy of JMC)
A quick glance at IOS9 reveals that older versions of IOS have a
simplistic implementation of memcpy that does not optimize large copies
by copying 16 bytes or 32 bytes per chunk, which makes cached reads
and writes noticeably slower -- the difference was significant enough
that the OoT speedrunning community noticed that IOS9 (the IOS that
is used for the OoT VC title) was slower.
More or less a complete rewrite of the function which aims
to be equally good or better for each given input, without
relying on special cases like the old implementation did.
In particular, we now have more extensive support for
MOVN, as mentioned in a TODO comment.
Instead of constructing IPCCommandResult with static member functions
in the Device class, we can just add the relevant constructors to the
reply struct itself. Makes more sense than putting it in Device
when the struct is used in the kernel code and doesn't use any Device
specific members...
This commit also changes the IPC command handlers to return an optional
IPCCommandResult rather than an IPCCommandResult. This removes the need
for a separate boolean that indicates whether the "result" is actually
a reply, and also avoids the need to set dummy result values and ticks.
It also makes it really obvious which commands can result in no reply
being generated.
Finally, this commit renames IPCCommandResult to IPCReply since the
struct is now only used for actual replies. This new name is less
verbose in my opinion.
The diff is quite large since this touches every command handler, but
the only functional change is that I fixed EnqueueIPCReply to
take a s64 for cycles_in_future to match IPCReply.
PrepareForState is now unnecessary with the new implementation of
HostFileSystem::DoState, which does what the old implementation
(CWII_IPC_HLE_Device_FileIO::PrepareForState) used to do.
I don't really see the use of this. (Maybe in the past it
was used for when we need a constant number of instructions
for backpatching? But we don't use MOVI2R for that now.)
Now that the ES class (now called ESDevice) and the ES namespace do
not conflict anymore, "IOS::" can be dropped in a lot of cases.
This also removes "IOS::HLE::" for code that is already in that
namespace. Some of those names used to be explicitly qualified
only for historical reasons.
There are no functional changes.
Some of the device names can be ambiguous and require fully or partly
qualifying the name (e.g. IOS::HLE::FS::) in a somewhat verbose way.
Additionally, insufficiently qualified names are prone to breaking.
Consider the example of IOS::HLE::FS:: (namespace) and
IOS::HLE::Device::FS (class). If we use FS::Foo in a file that doesn't
know about the class, everything will work fine. However, as soon as
Device::FS is declared via a header include or even just forward
declared, that code will cease to compile because FS:: now resolves
to Device::FS if FS::Foo was used in the Device namespace.
It also leads to having to write IOS::ES:: to access ES types and
utilities even for code that is already under the IOS namespace.
The fix for this is simple: rename the device classes and give them
a "device" suffix in their names if the existing ones may be ambiguous.
This makes it clear whether we're referring to the device class or to
something else.
This is not any longer to type, considering it lets us get rid of the
Device namespace, which is now wholly unnecessary.
There are no functional changes in this commit.
A future commit will fix unnecessarily qualified names.
According to the C standard, an offsetof expression must evaluate to an
address constant, otherwise it's undefined behavior.
Fixes https://bugs.dolphin-emu.org/issues/12409
See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95942
There are still improper uses of offsetof (mostly in JitArm64) but
fixing that will take more effort since there's a PPCSTATE_OFF wrapper
macro that is sometimes used with non-array members and sometimes used
with arrays and variable indices... Let's keep that for another PR.
Fixes the expression window being spammed with the first entry in the
Operators or Functions select menus when scrolling the mouse wheel while
hovering over them.
Fixes https://bugs.dolphin-emu.org/issues/12405
The dolphin-redirect.php script seems to have been present since 2012
at least, but we accidentally stopped using it when the "open wiki"
feature was reimplemented in DolphinQt2 in 2016.
<@delroth> dolphin-redirect.php is slightly smarter and tries to find gameid aliases for e.g. same region
<@delroth> uh, I mean different region
PR 9262 added a bunch of Jit64 optimizations, some of
which were already in JitArm64 and some which weren't.
This change ports the latter ones to JitArm64.
Let's reset m_last_used for each register that will be used
in an instruction before we start allocating any of them,
so that one of the earlier allocations doesn't spill a
register that we want in a later allocation. (We must still
also increment/reset m_last_used in R and RW, otherwise we
end up in trouble when emulating lmw/stmw since those access
more guest registers than there are available host registers.)
This should ensure that the asserts added earlier in this
pull request are never triggered.
If the register pressure is high when allocating registers,
Arm64FPRCache may spill a guest register which we are going to
allocate later during the current instruction, which has the
side effect of turning it into double precision. This will have
bad consequences if we are assuming that it is single precision,
so let's add some asserts to detect if that ever happens.
PR #2663 added a Jit64 implementation of dcbX and a fast path to skip JIT cache invalidation. Unfortunately, there is a mismatch between address spaces in this optimization. It tests the effective address (with the top 3 bits cleared) against the valid block bitset which is intended to be indexed by physical address. While this works in the common case, it fails (for example) when the effective address is in the 7E... region (a.k.a. "fake VMEM"). This may also fall apart under more complex memory mapping scenarios requiring full MMU emulation.
The good news is that even without this fast path, the underlying call to JitInterface::InvalidateICache() still does very little work in the common case. It correctly translates the effective address to a physical address which it tests against the valid block bitset, skipping invalidation if it is not necessary. As such, the cost of removing the fast path should not be too high.
The Jit64 implementation is retained, though all it does now is emit a call. This is marginally more efficient than simple interpreter fallback, which involves an extra call. The JitArm64 implementation has also been fixed.
The game Happy Feet is fixed by this change, as it loads code in the 7E... address region and depends upon JIT cache invalidation in reponse to dcbf.
https://bugs.dolphin-emu.org/issues/12133
At least on some CPUs (I found out about this from the
Arm Cortex-A76 Software Optimization Guide), using X30
with BLR is one cycle slower than using another register.
Check return value of calls to File::CreateTempDir() from CoreTiming,
FileSystem, and MMIO test classes to verify the test user directory
exists, and fail the tests otherwise.
Previously, eaddr would only be partially initialized in the ipv6 case.
Even if there's no support for it, we may as well ensure that the
variable always has deterministic initialization.
While we're at it, we can make the parameter a const reference, given no
members are modified.
EmulationActivity has an instance of Settings. If you go to
SettingsActivity from EmulationActivity and change some settings,
the changes get saved to disk, but EmulationActivity's Settings
instance still contains the old settings in its map of all
settings (assuming the EmulationActivity was not killed by the
system to save memory). Then, once you're done playing your
game and exit EmulationActivity, EmulationActivity calls
Settings.saveSettings. This call to saveSettings first overwrites
the entire INI file with its map of all settings (which is
outdated) in order to save any legacy settings that have changed
(which they haven't, since the GUI doesn't let you change legacy
settings while a game is running). Then, it asks the new config
system to write the most up-to-date values available for non-legacy
settings, which should make all the settings be up-to-date again.
The problem here is that the new config system would skip writing
to disk if no settings changes had been made since the last time
we asked it to write to disk (i.e. since SettingsActivity exited).
NB: Calling Settings.loadSettings in EmulationActivity.onResume
is not a working solution. I assume this is because
SettingsActivity saves its settings in onStop and not onPause.
For certain occurrences of nandx/norx, we declare a ReadWrite constraint
on the destination register, even though the value of the destination
register is irrelevant. This false dependency would force the RegCache
to generate a redundant MOV when the destination register wasn't already
assigned to a host register.
Example 1:
BF 00 00 00 00 mov edi,0
8B FE mov edi,esi
F7 D7 not edi
Example 2:
8B 7D 80 mov edi,dword ptr [rbp-80h]
8B FE mov edi,esi
F7 D7 not edi
Also avoid files without a name before the extension (name: ".ini")
from being added to the list because then they wouldn't be saveable
and it would appear with an empty name anyway.
Added Opacity controls for the user to customize the opacity of their touchscreen controls. Also, placed both Scale and Opacity settings into one window/option called Adjust Controls.
This function has been marked as obsolete. In Qt 6.0 it's removed
entirely, so we must use getContentsMargin() explicitly instead
(margin() would do this for us).
Ditto for setMargin(), in which case we use setContentsMargin instead.
setMargin() would just pass its argument to all four parameters of
setContentsMargin(), so we can do the same.
This literal was deprecated in 5.14.0. Not to mention it wasn't
documented as part of the API either: see the 5.14.0 changelog here:
https://code.qt.io/cgit/qt/qtbase.git/tree/dist/changes-5.14.0?h=v5.14.0
On Qt 6.0 this define is removed entirely. To stay forward compatible,
we can make use of QStringLiteral instead.
This skips a potentially costly loop if volume is 100% or 0%,
as for former there is no need for volume adjustment,
while latter can be solved by specifying a AUDCLNT_BUFFERFLAGS_SILENT flag
This fixes numerous resource leaks, as not every return path cleaned every created resource
Now they are all managed automatically and "commited" to WASAPIStream class fields only
after it's certain they initialized properly
When a game is selected, the option to add a shortcut of the game to the desktop is given. Uses native Windows API since Qt lacks support for adding shortcuts.
FinalizeCarryOverflow didn't maintain XER[OV/SO] properly due to an
oversight. Here's the code it would generate:
0: 9c pushf
1: 80 65 3b fe and BYTE PTR [rbp+0x3b],0xfe
5: 71 04 jno b <jno>
7: c6 45 3b 03 mov BYTE PTR [rbp+0x3b],0x3
000000000000000b <jno>:
b: 9d popf
At first glance it seems reasonable. The host flags are carefully
preserved with PUSHF. The AND instruction clears XER[OV]. Next, an
conditional branch checks the host's overflow flag and, if needed, skips
over a MOV that sets XER[OV/SO]. Finally, host flags are restored with
POPF.
However, the AND instruction also clears the host's overflow flag. As a
result, the branch that follows it is always taken and the MOV is always
skipped. The end result is that XER[OV] is always cleared while XER[SO]
is left unchanged.
Putting POPF immediately after the AND would fix this, but we already
have GenerateOverflow doing it correctly (and without the PUSHF/POPF
shenanigans too). So let's just use that instead.
Happens in Super Mario Sunshine. You could probably do something similar
for b == -1 (like we do for subfic), but I couldn't find any titles that
do this.
- Case 1: d == a
Before:
41 8B C7 mov eax,r15d
41 BF 00 00 00 00 mov r15d,0
44 2B F8 sub r15d,eax
After:
41 F7 DF neg r15d
- Case 2: d != a
Before:
BF 00 00 00 00 mov edi,0
41 2B FD sub edi,r13d
After:
41 8B FD mov edi,r13d
F7 DF neg edi
Consider the case where d and a refer to the same PowerPC register,
which is known to hold an immediate value by the RegCache. We place a
ReadWrite constraint on this register and bind it to an x86 register.
The RegCache then allocates a new register, initializes it with the
immediate, and returns a RCX64Reg for both d and a.
At this point information about the immediate value becomes unreachable.
In the case of subfx, this generates suboptimal code:
Before 1:
BF 1E 00 00 00 mov edi,1Eh <- done by RegCache
8B C7 mov eax,edi
8B FE mov edi,esi
2B F8 sub edi,eax
Before 2:
BE 00 AC 3F 80 mov esi,803FAC00h <- done by RegCache
8B C6 mov eax,esi
8B 75 EC mov esi,dword ptr [rbp-14h]
2B F0 sub esi,eax
The solution is to explicitly handle the constant a case before having
the RegCache allocate registers for us.
After 1:
8D 7E E2 lea edi,[rsi-1Eh]
After 2:
8B 75 EC mov esi,dword ptr [rbp-14h]
81 EE 00 AC 3F 80 sub esi,803FAC00h
The special case doesn't appear to make a significant difference in any games, and the current implementation has a (minor, fixable) issue that breaks Super Mario Sunshine (both with a failed assertion (https://bugs.dolphin-emu.org/issues/11742) and a rendering issue (https://bugs.dolphin-emu.org/issues/7476)). Hardware testing wasn't able to reproduce the special case, either, so it may just not exist.
PR #9315 contains a fixed implementation of the special case on all video backends, and can serve as a basis for it being reintroduced if it is found to exist under more specific circumstances. For now, I don't see a reason to keep it present.
-If adding 2 devices with the same name, they their unique id wouldn't be increased, causing a conflict.
-Removing a device wouldn't actually remove it from the internal devices list because the list of devices had already been updated when going through it.
-It was possible to remove devices belonging to other sources by adding a device with the same name and then removing it.
JosJuice