mirror of
https://github.com/RetroPortingToolKit/psxrecomp
synced 2026-10-28 10:59:16 -04:00
Matthew Stanley
eac1a61e43
PR #171 fixed the PGXP hooks, which were space-joined onto emissions that end on a bare #endif (the PSX_ENABLE_BLOCK_CYCLES muldiv stall/latency blocks) and were therefore swallowed as "extra tokens" and silently discarded -- 3,187 hooks per title in Ape Escape alone, plus 1,024 in the BIOS. It fixed the instance. Two same-line appenders of the same class were left behind: 1. strict_translator.cpp -- the deferred-load PGXP_LOAD hook is still space-joined onto c_code_deferred. All five c_code_deferred producers currently end in plain statements, so it is benign TODAY, but the moment a block-cycles annotation lands on deferred loads it breaks exactly the way #171 just did, and just as silently. 2. code_generator.cpp -- the trailing disassembly comment is appended to the emission unconditionally. Reachable whenever append_pgxp_hooks returned early and left the emission ending on a directive, e.g. `mfhi $zero` / `mflo $zero` (rd == 0 takes an early return). The comment is then dropped and -Wendif-labels fires. Both now route through one predicate, emission_ends_on_preprocessor_directive(), so the hazard is closed by construction rather than one site at a time. Site 1 takes an unconditional newline (always valid where the space was); site 2 only breaks the line when the emission actually ends on a directive, so ordinary instructions keep their inline comment and the generated diff stays minimal. Measured effect on emitted code, against merged master: - SCPH1001 BIOS: byte-identical - OpenBIOS: 2 sites, both the deferred PGXP_LOAD moving to its own line, identical token sequence - Ape / Tomba 1 / Tomba 2 / MMX6: every function token-identical (3095/6891/2726/4023 functions each, none added, none removed, zero body changes) Adds emitter_directive_line_test (10 checks, registered with add_test) so the predicate cannot regress: it pins both real emission shapes that end on #endif and, just as importantly, the negative cases -- a false positive would inject stray newlines into every instruction. ctest before and after this change fails the identical 6 pre-existing tests (aot_overlay_discovery, launcher_vulkan_option, mod_load_acceleration, release_zip, sdl3_main_single_include, vk_present_wait_stage); no new failures. full_function_emitter_test and l2_structural_test both pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
72 lines
3.4 KiB
C++
72 lines
3.4 KiB
C++
/*
|
|
* A preprocessor directive owns its whole line. Anything appended after it on
|
|
* that line is swallowed as "extra tokens" and silently DISCARDED -- no build
|
|
* failure, just a -Wendif-labels warning nobody reads. PR #171 was exactly
|
|
* this: `append_pgxp_hooks` space-joined its PGXP_*() call onto emissions that
|
|
* end on a bare `#endif` (the PSX_ENABLE_BLOCK_CYCLES muldiv stall/latency
|
|
* blocks), so 3,187 hooks per title were thrown away.
|
|
*
|
|
* This pins the predicate that now guards the remaining same-line appenders,
|
|
* so the hazard cannot come back by construction.
|
|
*
|
|
* Build/run: ctest -R emitter_directive_line_test
|
|
*/
|
|
|
|
#include "pgxp_hook_emitter.h"
|
|
|
|
#include <cstdio>
|
|
#include <string>
|
|
|
|
static int failures;
|
|
|
|
#define CHECK(cond, msg) \
|
|
do { \
|
|
if (!(cond)) { std::fprintf(stderr, "FAIL: %s\n", (msg)); failures++; } \
|
|
else { std::printf("ok: %s\n", (msg)); } \
|
|
} while (0)
|
|
|
|
using PSXRecomp::emission_ends_on_preprocessor_directive;
|
|
|
|
int main() {
|
|
/* The shape that caused PR #171: translate_mult's real emission. */
|
|
const std::string muldiv =
|
|
"{ int64_t result = (int64_t)(int32_t)cpu->gpr[4] * (int64_t)(int32_t)cpu->gpr[5];"
|
|
" cpu->lo = (uint32_t)result; cpu->hi = (uint32_t)(result >> 32); }"
|
|
"\n#ifdef PSX_ENABLE_BLOCK_CYCLES\n psx_muldiv_set(cpu, 13u);\n#endif";
|
|
CHECK(emission_ends_on_preprocessor_directive(muldiv),
|
|
"a muldiv emission ending on a bare #endif is detected");
|
|
|
|
/* translate_mfhi with rd==$zero: the stall block with no statement. */
|
|
CHECK(emission_ends_on_preprocessor_directive(
|
|
"/* nop: write to $zero */"
|
|
"\n#ifdef PSX_ENABLE_BLOCK_CYCLES\n psx_muldiv_stall(cpu);\n#endif"),
|
|
"a zero-reg mfhi/mflo emission ending on #endif is detected");
|
|
|
|
/* Leading whitespace is legal before a directive. */
|
|
CHECK(emission_ends_on_preprocessor_directive("foo();\n #endif"),
|
|
"an indented directive still owns its line");
|
|
CHECK(emission_ends_on_preprocessor_directive("foo();\n\t#else"),
|
|
"a tab-indented directive still owns its line");
|
|
CHECK(emission_ends_on_preprocessor_directive("#endif"),
|
|
"a single-line emission that is only a directive is detected");
|
|
|
|
/* Ordinary statements must NOT be flagged -- a false positive here would
|
|
* inject stray newlines into every instruction's output. */
|
|
CHECK(!emission_ends_on_preprocessor_directive("cpu->gpr[4] = cpu->hi;"),
|
|
"a plain statement is not a directive line");
|
|
CHECK(!emission_ends_on_preprocessor_directive(
|
|
"\n#ifdef PSX_ENABLE_BLOCK_CYCLES\n psx_gte_stall(cpu);\n#endif\n "),
|
|
"an emission whose directive block is followed by a fresh indented "
|
|
"line is safe to append to");
|
|
CHECK(!emission_ends_on_preprocessor_directive(
|
|
"{ uint32_t _pgxa = cpu->gpr[4]; foo();\n PGXP_LOAD(0u, _pgxa, x); }"),
|
|
"an already-fixed hook emission is not flagged");
|
|
CHECK(!emission_ends_on_preprocessor_directive(""),
|
|
"an empty emission is not flagged");
|
|
CHECK(!emission_ends_on_preprocessor_directive("x = 1; /* #endif in a comment */"),
|
|
"a '#' that is not the line's first token is not a directive");
|
|
|
|
std::printf(failures ? "FAILED (%d)\n" : "ALL PASS\n", failures);
|
|
return failures ? 1 : 0;
|
|
}
|