Sio.cpp, MemoryCardProtocol.cpp, MultitapProtocol.cpp and MemoryCardFile.cpp
become C89 units, and their headers C ones every unit can read. Sio0 and
Sio2 are plain structs with functions (sio0_set_tx_data, sio2_write and so
on) in place of methods; a state carries both whole, so their members keep
their order and types and a build check holds them to the layout states
have had on 64-bit hosts. The byte FIFOs move to SioFifo.c with their
state freeze, which tests/savestate now exercises directly instead of a
copy. The memory card images keep their slots in plain arrays, and read
what they need of the configuration through three functions in
Pcsx2Config.cpp; the card type the configuration carried at run time is
gone with them, since an open card file is what makes a card present. The
pads' three SIO entry points get C linkage (PAD/PADsio.h), and
sioConvertPadToPortAndSlot hands its port and slot back through pointers.
tests/sio drives SIO0, SIO2, the cards and the multitap through every
command a game sends, PS2 and PS1 cards, keyed and unkeyed MagicGate, DMA
blocks, auto-eject, the Python 1 slot latch and a state saved mid-command,
and compares every byte that comes back, the card files and the SIO state
blocks with the transcript the C++ made: they are the same. The tests that
built the C++ files now build the C ones.
The vertex kick handlers come in two sets, plain and auto-flush, and
which one a draw kicks through is chosen when a register that decides it
is written -- TEX0 and TEX2, TEST, FRAME and ZBUF of the current context,
PRIM and PRMODE -- and changes only when the answer does: the draw state
reads what it draws. Every other draw kicks through the plain handlers,
which do not ask, where before every kick asked whenever auto flush was
on (by default in the software renderer, and in the hardware renderer
for every game the database turned it on for).
Auto flush is now always on, for every primitive, in both renderers.
The hardware level setting, the software switch, their core options and
the GameDB fix and its entries go.
tests/gsautoflush/autoflush_select pins the choice against the state
after every register write.
With the automatic game fixes off, an entry that sets eeDivRoundMode
and not eeRoundMode was reported by eeRoundMode's name: the mode left
at MaxCount indexed the four-entry name table one past its end, and
the pointer read there was printed as a string. The message for
applying it printed eeRoundMode's number the same way. Both now name
the division mode.
tests/patch's dbload lane applies every entry's fixes with the
automatic fixes on and off, as a VM start does, so a log line that
indexes a table with a mode the entry left unset shows up in the
sanitizer runs; it reported this one under UBSan.
The database was a std::unordered_map<std::string, GameEntry>, each
entry holding std::strings, std::vectors of fixes, hacks, memory card
filters and dynamic patches, and its own std::unordered_map of patch
blocks, with member functions over them and std::optional through the
parser.
Now the file is read into a few pools shared by every entry (text,
game fixes, speed hacks, GS fixes, patch blocks, dynamic patch
entries); while it is read an entry names its runs by offset, and
once the pools are final the entries are made with pointers into them.
Serials are found through an open-addressed index, built as the file
is read so a serial repeated in another case is still refused before
it is parsed. Memory card filters are stored already joined with '/'.
GameDatabase_FindGame/FindPatch/ApplyGameFixes/ApplyGSHardwareFixes
replace the namespace and members; DynamicPatch is a pair of pointer
and count runs; ParseSpeedHackName takes a pointer and a length. The
GS fixes skipped under manual hacks were also collected into a string
nothing read, which is gone.
Checked by dumping every one of the 12807 entries through both the old
and the new code, every field plus what both apply functions do with
the automatic fixes on and off: byte for byte the same, and clean under
ASan, UBSan and LeakSan across an unload and reload. tests/patch's
dbload lane now also loads a small database whose every field is known,
including the eight kinds of line that must be refused, and checks
what each field reads back as, lookups in any case, the default patch
block as fallback only when there is one, and unloading.
The patch store was a std::vector<IniPatch> exported to Counters and
VMManager, each walking it for the places it wanted; a 'bytes'
payload was a std::vector<u8> inside each patch; the dynamic patches
were copies of the game database's vectors; the parser split lines
into std::vector<std::string_view>; the loaders took std::string and
went through std::string buffers, std::optional and FileSystem's
std::vector of results.
Now:
- The patches are a plain array in Patch.cpp, read through
LoadedPatchCount/LoadedPatch, and applied by ApplyLoadedPatches with
a mask of places, in load order as before. A payload is a malloc'd
run with its length, freed by ForgetLoadedPatches, and only
allocated once the line has passed every check.
- The dynamic patches' entries are copied into one array of their
own, a pattern run then a replacement run per patch, so a reload of
the game database cannot leave them dangling.
- The parser splits into five (pointer, length) fields on the stack,
with the stripping and empty-field rules SplitString had, and the
command table takes the same.
- LoadPatchesFromZip and LoadPatchesFromDir take C strings. The zip
member is inflated into a malloc'd buffer. The folder is walked with
retro_opendir directly, with FindFiles's filter (files, hidden ones
included, "*.pnach", sizable) and its order, and each file is read
with filestream_read_file, which terminates it. The "not found"
message no longer has a newline in the middle of the path.
tests/patch gains a lane that links Patch.cpp against the real
libretro-common directory, file and inflate code: pnach files for the
CRC in either case and with suffixes beside another CRC's, a .txt, a
hidden file, a directory named like a pnach and an empty folder; and
archives written by Python's zipfile with the member deflated, stored
under an upper-case name, only under a subfolder, and behind a
comment, plus every truncation and every one-byte corruption of one.
It fails if the CRC match turns case-sensitive.
pcsx2/StringView.h/.cpp held prefix/suffix tests, a lower-casing copy
and the pnach "key = value" split, all on std::string_view. Each had
one or two callers:
- MemoryCardFile's ".bin" checks and pcap_io's device-name prefix are
libretro-common's string_ends_with/string_starts_with, on strings
that are NUL-terminated anyway.
- VMManager::IsElfFileName compares the last four characters to
".elf" without case.
- The game database lower-cases its serial keys with an ASCII loop.
- The pnach reader splits at '=' itself, with the stripping it already
had for the comma-separated fields; that stripping now passes
isspace an unsigned char, as it must for bytes past 0x7f.
The files are gone from the Makefile, CMake and the test builds that
linked them.
Each byte of a 'bytes' payload was parsed as two characters in base
16, and the parse stops at the first non-hex character, so "0g" was
taken as a byte of 0 and the line loaded, where the error branch for
a payload that is not hex was meant to refuse it. Both characters
are now checked.
tests/patch only fed the parser the game database's own lines, which
are all well formed. It now also loads eighteen pnach lines that are
not: spacing around '=' and the commas, a trailing comment, no '=',
a place out of range, signed or not a number, a payload not hex or
not whole bytes, an unknown CPU or operand size, too few fields, and
an address or value past its field. Each must load or be refused as
before, and what loads must carry the place, address, value and
payload written.
The game-database lane had stopped linking: its stubs for the three
GS lookup functions still had the std::string_view/std::optional
signatures from before GSHwHack.cpp took a pointer and length and
returned s16, so the suite failed on every run.
It also set SANFLAGS to empty before reading SANITIZER, discarding
the flags the CI matrix passes, so the ASan and UBSan jobs ran it
unsanitized. SANFLAGS is now kept and SANITIZER added to it.
With the flags honoured, the TSan build no longer linked:
libretro-common's .c files went through the C++ driver, which gives
retro_procbarrier's __tsan_acquire/__tsan_release declarations C++
linkage. They are built by the C compiler now, as in the core.
ingame.c carries the calls and the checks; ingame_glue.cpp holds only
what has to be C++: the forwarding call into patches.cpp and the stubs
the patch parser links against.
Every block tests the serial of the game it was written for, as its
comment and CRC name it, so each one is reachable and none lands on
another game:
Dark Angel (PAL) SLES-51333
Mega Man X7 (NTSC-U) SLUS-20487
WipeOut Fusion (NTSC-U) SLUS-20462
Gradius V (PAL) SLES-52095
Michigan: Report From Hell (PAL) SLES-53073
Simple 2000 Vol. 101 (NTSC-J) SLPS-20466, in the SLPS- chain
Vampire Darkstalkers Collection SLPM-65998
Street Fighter EX3 (NTSC-U) SLUS-20130
Street Fighter EX3 (NTSC-J) SLPS-20003
The Alias, SSX On Tour, Kinetica and Kingdom Hearts II blocks log the
game they patch. A message is a format string, so the percent signs in
"130% EE" and the like are written %%.
tests/patch/ingame.cpp links patches.cpp with the real patch parser and
calls it for every serial and CRC the file names, under every renderer
and hint setting. It requires that every message in the file is logged
by some call, that no call logs two games, that every patch line parses,
and that no message carries a conversion other than the CRC's. It runs
from tests/patch/build.sh.
The last helper header in common/ was eleven functions with sixty-two
callers, and what they are splits four ways.
Four went to their one owner. StrideMemCpy copies rows between buffers
of differing pitch, which is a texture upload and nothing else, and is
GSStrideMemCpy in GSTexture.h. SplitString and the StripWhitespace it
calls had two users that parse tables -- the DEV9 adapter reader and the
patch reader -- and each has its own static copy, which is what a parse
helper with two callers is. WideStringToUTF8String is Windows-only and
converts what the Windows adapter APIs return, so it lives in those two
DEV9 files under their _WIN32 guard.
Two became small headers named for what they do rather than for being
string utilities. pcsx2/ParseNumber.h is FromChars: a number out of a
string view or nothing, the bool spelling included, used by the core
option parsing and the patch and cheat readers. pcsx2/FormatString.h is
printf into a std::string, for the twenty callers that need the text
rather than a log line.
The prefix and suffix predicates stay the tree's own, in
pcsx2/StringView.h, and this is the reason: libretro-common's
string_starts_with and string_ends_with take a NUL-terminated char*, and
every caller here has a string_view -- a serial from a YAML document,
an ELF path from SYSTEM.CNF, a window into a larger buffer. Comparing by
length and content is correct; handing .data() to a char* function is
not, and it would have read past the view. toLower and
ParseAssignmentString go with them. Strncasecmp, which StringUtil.h
defined and four other files used through it, is defined in both new
headers under a guard.
Full default build links; all thirteen suites pass; every touched and
new file is clean for Windows, macOS and iOS shapes with
-Winfinite-recursion; the added includes sit at depth zero; no file
references anything removed. common/ is HostSys.cpp and headers.
The logging facade in common/ -- IConsoleWriter and IDevConWriter,
Console and DevCon -- formatted every message into a heap std::string
and then handed it to the frontend's log_cb with "%s\n". One thousand
and six call sites call log_cb themselves now, with the level the
facade's method mapped to, read from its source rather than assumed:
Console.WriteLn is INFO and DevCon.WriteLn is DEBUG, but DevCon.Error
and DevCon.Warning were ERROR and WARN, and stay so. The newline the
facade appended is in each format string; the eighteen literals that
already ended in one keep the blank line they produced. The colour
overloads discarded their colour, and the argument is gone with them.
log_cb is one symbol with C linkage, declared in Pcsx2Defs.h for the
C++ translation units and in ATA.h for the C ones, which called the
facade through a C shim that appended nothing -- and whose callers,
despite its comment, put no newline in either; they have one now. It
was also called unguarded by every log line, and a frontend that offers
no log interface left it null, a crash at the first message: main.cpp
installs a stderr fallback in that case.
Ninety-four files had Pcsx2Defs.h only through Console.h and include it
directly, by the tree's rooted spelling. The patch harnesses, which
counted errors and warnings through the facade's methods, count them
through a log_cb that dispatches on level, and the settings harness
provides one.
Full default build links; all thirteen suites pass; every touched C++
file is clean for Windows, macOS and iOS shapes with
-Winfinite-recursion -- the iOS shape caught a colour variable the arm64
recompiler left behind, which no Linux build compiles -- and every
touched C file under the C89 lane's flags; the added includes sit at
depth zero; no file references anything removed.
Combine, IsAbsolute, ReplaceExtension, GetDirectory and GetFileName leave
common/. Their seventeen sites call libretro-common's file_path
directly: fill_pathname_join into a PATH_MAX_LENGTH buffer where Combine
built a std::string, path_is_absolute, fill_pathname with the dot in the
replacement where ReplaceExtension kept the path's own, fill_pathname_
basedir, and path_basename. Two things were kept exact rather than
approximated: a BIOS path with no extension is returned unchanged, as
ReplaceExtension did, where fill_pathname would append; and the
directory GetDirectory produced only ever fed a directory listing, so
fill_pathname_basedir's trailing separator changes nothing.
SplitWindowsPath had one user, the ISO9660 filesystem, and is a static
function in IsoFS.cpp now; it is that filesystem's parsing, not a path
utility. Canonicalize and SplitNativePath had none.
The EmuFolders paths are char arrays, not strings, and so is
ThreadedFileReader's filename: the first conversion put .c_str() on
them, and the build said so. PATH_MAX_LENGTH is retro_miscellaneous.h's
and file_path.h does not bring it, so each file that gained a buffer
includes it. The patch harness that links Patch.cpp gains file_path.c
and what it pulls in.
Full default build links; all twelve suites pass; every touched file is
clean for Windows, macOS and iOS shapes; every added include sits at
depth zero; and no file of any extension references anything removed.
The last of common/Threading.h. What it held was KernelSemaphore and
UserspaceSemaphore, and they left together, because on x86 the second
parks on the first and the second's one user is the VU1-to-GS packet
handoff, semaXGkick.
semaXGkick counted: the VU1 thread posted once per path-1 packet, the GS
took one credit per packet it popped. Every post sits next to a push
into gsPackQueue, on both paths that push, so the count was a second
copy of that queue's occupancy -- one a reset could let drift, where the
queue cannot. The handoff is retro_asym_eventcount now, with
GetPendingGSPackets as the condition and the pop as the consume: the VU1
side pays a release store per packet instead of a lock xadd, and the GS
side spins its budget on the occupancy, then registers, re-checks and
parks. Measured per handoff pair on x86: 15.8 ns to 1.5 ns. aarch64
gives up its WFE/SEV park for the eventcount's; the GS reaches the park
only after the spin budget, so that is the rare case, and it was
measured on x86 only.
With the header deleted, twenty-eight files that included it -- most by
relative path -- lost what they had been getting through it, and the
build named each: __POSIX__ from Pcsx2Defs.h in sockets.cpp, ordered
after the block that tests it; retro_atomic.h in HostSys.cpp; and
<unordered_map> in IopBios.cpp. Threads.cpp went with it, and
Semaphore.cpp.
The unordered_map was the IOP import-stub cache, hit millions of times a
minute, and it is a fixed open-addressing table now, C89-shaped: an
array of records, Fibonacci hashing on pc >> 2, linear probing, no
allocation, no pointer chasing. libstdc++'s find for a u32 key divides
by the bucket count -- a 64-bit DIV, twice on the general path, 110
instructions -- and walks a heap-allocated node list; the replacement is
22 instructions, one imul, one indexed load. Erase was only ever
followed by re-inserting the same key, so it is an overwrite; a full
probe window overwrites its first slot, which a cache may. On 300 stubs
laid out as an import table lays them: zero collisions, 3.3 ns to 1.6 ns
per hit, 23 ns to 1.4 ns per stale overwrite.
The resolve path behind that cache took the module name as a
std::string built from the table's 8-byte field and compared it against
each module name in a chain. The name is two words now -- irxImportName
reads the field, zeroes everything after the first NUL as the old string
read did, and packs it little-endian -- and each MODULE(n) compares them
against constants folded from the literal at compile time: 'sysm',
'load', 'ioma', 'intr', 'sifc' are immediates in the object, and no
string-compare call remains in the lookups. IopBios.h no longer names
std::string, and it includes what it uses. The JIT's psxRecompileIrxImport
calls the word form directly, so irxImportHLECh goes; irxImportFuncname,
irxImportLog and irxImportLog_rec had no callers and go with it, and
IopModuleNames.cpp, which only irxImportFuncname included, is deleted.
Every touched TU was compiled for Linux through the Makefile,
syntax-checked for Windows with MinGW x86_64 and in an Apple shape, and
every added include sits at preprocessor depth zero. The full default
build links; all twelve suites pass, the patch harness having lost its
Semaphore.cpp link entry; and a whole-tree search finds no reference to
anything removed. One thing for anyone building incrementally: the
multi-ISA rasterizer objects and the .d files do not track the deleted
header, and need a clean.
Timeslice was sched_yield or SwitchToThread behind a name in common/,
and its seven callers now call sthread_yield, resynced from RetroArch
master. Six are yields still: the fastmem spinlock in vtlb.cpp, which
is taken inside the page-fault handler where a mutex is not signal-safe;
the bounded backoff in Gif_Unit; the EE's wait for MTVU ring space, whose
comment records a deliberate choice next to a FIXME a wait lock would
hide; the GS's wait for rasterizer job-queue space, which will get its
own patch; and two network retries. Each is what it was, with the
libretro-common spelling.
The seventh, retro_load_game's poll for the EE's boot result, is a real
wait now. The result is set once by the EE and waited for once by the
frontend, which is retro_eventcount's shape exactly: the frontend
registers, re-checks, and parks, and the EE notifies after it stores.
The same primitive replaces the KernelSemaphore the EE parked on while
Paused: the predicate is the state word, every waker stores it and then
notifies, and prepare/commit provides what the counted post did -- a
store landing between the check and the park is a wake, not a loss. That
was KernelSemaphore's only direct user. The class itself stays for now:
on x86 UserspaceSemaphore embeds one for its park, and that is
semaXGkick, which is measured before it moves.
Threads.cpp is empty with Timeslice gone and is deleted, with its
Makefile.common and CMakeLists entries; a struct that served the deleted
Linux tid trick went with it. Threading.h keeps KernelSemaphore and
UserspaceSemaphore, and nothing else.
Every touched TU was compiled for Linux through the Makefile, for
Windows through MinGW x86_64, and syntax-checked in an Apple shape; the
added includes sit at preprocessor depth zero, checked. The full default
build links, all twelve suites pass -- the ee and patch harnesses gain
the rthreads object their link lines now need -- and a whole-tree search
finds no reference to anything removed.
Three things, and the middle one is the reason for the other two.
libretro-common is brought level with RetroArch master. Five files
differed and one of them stays as it is -- include/compat/ifaddrs.h keeps
the extern "C" guard ps2 added for DEV9, which is not upstream and without
which the core fails to link; the other four are RetroArch being ahead,
checked line by line. Three files are new: retro_procbarrier.c, the
process-wide barrier, retro_eventcount.c, and retro_asym_eventcount.c,
which is built on both. Makefile.common builds all three.
WorkSema is rewritten on two of those. It was a hand-rolled state
machine whose NotifyOfWork -- what the EE pays per GS packet with the GS
thread awake, and what MTVU's own comment already named as the fetch_add
on its hot path -- measured 6.4 ns. It is now retro_asym_eventcount's
notify, a release store and a relaxed load with no lock prefix, measured
1.2 ns in the same binary and the same run. The symmetric retro_eventcount
was measured too, at 9.6 ns, and would have been a regression: it pays an
RMW and a seq_cst load to serve many producers, and each WorkSema here
has one. The ordering the hot side drops is paid where WaitForWork parks,
by retro_procbarrier; where no barrier exists the object runs the
symmetric protocol for its whole life, decided at init, and MTGS never
needs to know. WaitForEmpty's direction -- worker to producer, once a
frame -- is the plain retro_eventcount, whose prepare/commit contract is
what STATE_FLAG_WAITING_EMPTY was reinventing.
The seven operations keep their contract exactly, and tests/spsc/
worksema.cpp pins each: CheckForWork consumes a burst of notifies as one;
WaitForWork returns at once with work pending and parks without; a timed
wait is false only on a timeout with nothing pending; Kill unblocks every
waiter and makes WaitForEmpty false; Reset revives. One subtlety is
pinned because it bit the test first: "empty" means the worker went idle
after consuming, not merely consumed, and a WaitForEmpty issued between
the two blocks -- the old state machine did the same, checked. The
producer/consumer pair runs 50000 items with WaitForEmpty interleaved
2858 times and never sees it return with work outstanding. Plain, TSan,
ASan and UBSan are all clean; TSan sees the eventcounts' atomics, not the
barrier, so the lost-wake half is only load-bearing on more than one
core.
The last part is what the rewrite made dead, plus what an audit of every
common/ object against every importer in the finished build found
alongside it: KernelSemaphore::WaitFor, whose only caller was the old
timed wait; Mutex::TryLock; both StringUtil::ReplaceAll overloads; and
DecompressBlockBC4 and BC5 with their private helper and the two
Int8ToFloat conversions that served only them -- the replacement loader
dispatches BC1, BC2, BC3 and BC7 and nothing else. Every header in
common/ is still included by something, so nothing is removed at file
level; the audit was at symbol level and discounted vtable dispatch,
which is why SettingsInterface's methods, which nm cannot see a caller
for, are untouched.
The core links, all twelve suites pass, and the patch harness gains the
three objects its Semaphore.cpp link now needs.
The HostFS migration removed common/FileSystem.h; dbparse stubbed the
old namespace against that header and has not compiled since. Point
the stubs at HostFS.h, which is where the surviving FindFiles surface
lives, and stub filestream_read_file for the pnach path the test never
enters.
Companion to dbparse, which covers the patch lines. This covers the rest
of GameIndex.yaml -- the game fixes, speed hacks, GS hardware fixes,
rounding and clamping modes and the memory card filters -- by linking
pcsx2/GameDatabase.cpp and the real yaml reader and requiring the load to
produce no errors and no warnings. It spot-checks three serials afterwards
so an empty parse cannot pass by reporting nothing.
It catches the failure that motivated it: renaming a gamefix so the parser
no longer knows it -- XGKickHackk -- is reported and fails the run. That
is the WRC-class shape, a name in the database the code has stopped
recognising, dropped with a console error and no other consequence.
Two silent failures it does not catch, both checked rather than assumed
and both in the header. Renaming a key -- speedHacks to speedHackss --
makes the entire block invisible with nothing reported, because an
unrecognised key is skipped rather than flagged. And an out-of-range
value, autoFlush: 99, loads without complaint; the numeric gsHWFixes are
range-checked nowhere. Closing either means the parser reporting unknown
keys and validating ranges, which is a change to GameDatabase.cpp, not to
a test.
One more gap is structural: the three gsHWFixes fields whose values are
function names are looked up in tables in GSHwHack.cpp, and linking that
pulls in the GS renderer -- sixty symbols for three fields. They are
stubbed, so 349 lines in the database go unvalidated by this, and the
header says which.
The "bytes" fix landed because someone reported WRC 4 as broken. Nothing
in the tree would have found it: a pnach line the parser does not
understand is rejected with a console error, and then nothing happens --
no failure, no test, and the game quietly misbehaves. Six serials carried
patches in an operand size this parser did not implement, and the only
sign was a line in a log.
So this feeds GameIndex.yaml's 854 patch lines to LoadPatchesFromString
and requires every one to be accepted. It links pcsx2/Patch.cpp rather
than reimplementing the format, so it cannot drift from what the emulator
parses, and it applies what it loaded, which is how a line that parses
into an empty payload is caught as well as one that is refused.
It would have found the original bug: deleting "bytes" from the parser
drops it to 842 of 854, exactly the twelve WRC 4 lines. Deleting
"extended" drops it to 826.
Its limit is worth stating and is in the header. Deleting "ledouble"
leaves it at 854, because nothing in the database is written in that size
-- the test tracks what the database contains rather than the whole pnach
format, so it catches a type going missing only once some entry depends
on it. That is the point at which it starts to matter, but it does mean a
green run is not a claim about the format.
Twelve suites now. Clean under ASan and UBSan.
libretroadmin