Checked on hardware that this bias was not added because I had assumed the other way around would be true, forgot to ask about making a PR for this when I initially had done so
If we're on an x64 CPU that doesn't have the MOVBE extension, trying to
SwapAndStore a host register results in that register's value getting
clobbered with the swapped value. Jit64::stX and Jit64::stXx detect this
case, and if necessary, emit a MOV to a register that's fine to clobber.
This logic was broken by the merge of PR 12134. Jit64::stX and
Jit64::stXx were assuming that if RegCache::IsImm returns true for a
guest register, calling RegCache::Use or RegCache::BindOrImm for that
guest register would result in an immediate. However, PR 12134 made it
possible for a guest register to have both a host register and an
immediate in the register cache at the same time. When this happens,
RegCache::IsImm returns true, yet RegCache::Use and RegCache::BindForImm
return an RCOpArg whose Location returns a host register. (To make it
extra confusing, RCOpArg::IsImm calls RegCache::IsImm if the RCOpArg
came from RegCache, so RCOpArg::IsImm returns true!)
To fix this, in cases where Jit64::stX and Jit64::stXx explicitly need
an immediate to avoid having to emit an extra MOV, let's call
RegCache::Imm32 so that we're certain that we're getting an immediate.
This fixes an issue on older x64 CPUs that manifested as e.g. completely
broken graphics in Spyro: Enter the Dragonfly.
The constant propagation PR made it so that a guest register can be
present in the register cache as both a host register and an immediate
at the same time. If such a guest register is requested from the
register cache, the register cache prefers returning it as a host
register. However, RCOpArg::IsImm still returns true in this case. This
is confusing, especially since OpArg::IsImm does not return true if the
RCOpArg is converted into an OpArg.
This commit makes RCOpArg::IsImm check whether RCOpArg::Location returns
an immediate, so that RCOpArg::IsImm returns false when a host register
is being used. Code that wants to know whether an immediate exists in
the register cache rather than whether an immediate is currently being
used should call RegCache::IsImm instead.
Recently came across a strange issue where Dolphin would hard crash in most games with this error:
```sh
/usr/include/c++/15.2.1/optional:1165: constexpr const _Tp* std::optional<_Tp>::operator->() const [with _Tp = InputCommon::ImagePixelData]: Assertion 'this->_M_is_engaged()' failed.
```
The culprit turned out to be accessing `host_key_image` which is an `std::optional` thay may return `std::nullopt`. I'm not sure why this issue started occuring for me since I've had no issue with my Dynamic Input Textures in the past? But this fixes a crash if the image fails to load.
Aside from being unnecessary, on Windows the flag prevents two instances
of Dolphin (one instance from before 2509-371 when the flag was
introduced and the other after) from running the same ROM
simultaneously.
Attempting to do so generated the false error `"[Rom]" is an invalid
GCM/ISO file, or is not a GC/Wii ISO.` followed by `Failed to init core`
and emulation shutdown on the second instance to start the game. Fixing
the incorrect error message is a task I'm deferring to another PR.
The problem didn't happen when both instances were 2509-371 or later,
but I ran into it while bisecting an issue and it'd be nice to avoid
that problem in the future.
We have an optimization where the guest carry flag is kept in the host
carry flag between certain back-to-back pairs of integer instructions.
If the second instruction falls back to the interpreter, then
FallBackToInterpreter should flush the carry flag to m_ppc_state,
otherwise the interpreter reads a stale carry flag and at some later
point Jit64 trips the "Attempt to modify flags while flags locked!"
assertion.
An alternative solution would be to not store the guest carry flag in
the host carry flag to begin with if we know the next instruction is
going to fall back to the interpreter, but knowing that in advance is
non-trivial. Since interpreter fallbacks aren't exactly intended to be
super optimized, I went for the flushing solution instead, which is how
JitArm64 already works. In most cases, the emitted code shouldn't even
differ between these two solutions.
Note that the problematic situation only happens if the first integer
instruction doesn't fall back to the interpreter but the second one
does. This used to be impossible because there's no "JIT disable"
setting that's granular enough to disable some integer instructions but
not all, but with the constant propagation PR, it's possible if constant
propagation is able to entirely evaluate the first instruction but not
the second.
This makes SetUID take more emulated time giving the host more time to actually do the work.
The Wii menu "Data Management" -> "Save Data" -> "Wii" screen is no longer nearly as hard to emulate at full speed.
Constructing the UIDSys from the filesystem is a major bottleneck in the Wii menu "Data Management" -> "Save Data" -> "Wii" screen and this change makes it about twice as fast.
cristian64
Craig Carnell
Nuh Uh
JMC47andGitHub
Jordan Woyak
JosJuice
Joshua Vandaële
Dentomologist
Mateus B. Cassiano