Skip to content

Commit 8aef409

Browse files
JoeMattclaude
andcommitted
acid: timing perf counters + per-test perf delta + first timing/IRQ tests
Builds out the acid framework along the lines requested: comprehensive test categories, perf data capture wired into the runner, first real tests against timing & IRQ delivery (the categories most likely to explain the Doom 2x speed regression in issue #131). Core instrumentation -------------------- Five new PERF_COUNTERs at the timing-critical hot paths so any test or `make benchmark` run can see how often things actually fire (no runtime cost unless built with BENCH_PROFILE=1): * `timing_jaguar_execute_calls` -- once per `retro_run()` * `timing_halfline_callbacks` -- 525 per frame on NTSC * `timing_vblank_irqs` -- 1 per frame * `timing_jerry_irqs` -- JERRY PIT timer 1/2 to 68K * `timing_gpu_irqs_to_68k` -- TOM PIT to 68K Verified against headless Doom benchmark: halflines = 524 * frames exactly; vblank_irqs ~= frames; everything within spec. These counters will surface any future regression where (e.g.) vblank fires twice per frame -- which is the leading hypothesis for the Doom 1.5-2x bug. Acid runner: per-test perf summary ---------------------------------- `test/acid/run.c` now snapshots a fixed set of perf counters before and after each test's frame run and prints the delta, e.g.: [PASS ] tests/timing/vc_per_frame.jag perf: timing_jaguar_execute_calls=600 timing_halfline_callbacks=314400 That lets reviewers see at a glance what each test exercised -- useful for catching tests that PASS while doing nothing, and for attributing a slow blitter test to the right counter (calls vs inner-iter vs phrase-write). Top-level `make acid` now forces BENCH_PROFILE=1 + TEST_EXPORTS=1 so the runner's `dlsym(perf_counters_find)` always works. First real tests ---------------- * `tests/timing/vc_advance.s` [PASS] -- VC counter must change * `tests/timing/vc_per_frame.s` [PASS] -- VC sweeps once per frame * `tests/irq/vblank_delivery.s` [NOT-RUN-YET] -- VBlank IRQ raises in TOM (counter ticks) but our 68K vector-64 patch never fires. Real bug surface, exactly the kind of thing this suite is meant to catch. Left checked in as a known-broken regression gate. Documentation ------------- * `test/acid/README.md` rewritten as a long-form roadmap covering all 13 planned categories (smoke, timing, irq, blitter, op, gpu, dsp, bus, hle, memory, quirks, stress, perf), with status matrix, per-test perf-summary docs, vasm install steps for the prb28/vasm GitHub mirror, and explicit cross-references to Shamus' original `docs/TODO` items per category. * `docs/emulation-bug-hunt-todos.md` gains a final section that lists the still-open accuracy items from the upstream `docs/TODO` (VC behaviour, cycle accuracy, blitter A1<->A2 propagation, bus contention, OP timing) and maps each to its acid-test home. The original `docs/TODO` is left untouched per user direction -- it's the historical record. Status: 3 / 5 tests passing. The 2 NOT-RUN-YET cases are real emulator bugs, surfaced (not introduced) by this work. Co-Authored-By: Claude Opus 4.7 <[email protected]>
1 parent 1950680 commit 8aef409

10 files changed

Lines changed: 737 additions & 110 deletions

File tree

Makefile

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,12 @@ benchmark:
909909
# (see test/acid/README.md). Requires the vasm 68K assembler on $PATH;
910910
# if absent, the assemble step is skipped and only the runner harness
911911
# is built (so CI can still validate the harness compiles).
912-
acid: $(TARGET)
912+
#
913+
# Forces a BENCH_PROFILE=1 + TEST_EXPORTS=1 build of the core so the
914+
# acid runner can dlsym `perf_counters_find` and report a per-test
915+
# delta (halflines, vblank IRQs, blits, inner-loop iters, ...).
916+
acid:
917+
$(MAKE) BENCH_PROFILE=1 TEST_EXPORTS=1 -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 4)
913918
$(MAKE) -C test/acid test CORE=$(abspath $(TARGET))
914919

915920
print-%:

docs/emulation-bug-hunt-todos.md

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,3 +456,35 @@ shipping v2.2.0; capture them so they don't get lost.
456456
/ `const`-correctness audits as a CI step. `clang-tidy` and
457457
`cppcheck` would be good starting points; the codebase already
458458
has a C89 lint, so the infrastructure is there.
459+
460+
## Original `docs/TODO` items still relevant (Shamus / CJ)
461+
462+
The historical `docs/TODO` from the upstream Virtual Jaguar tree
463+
lists several still-open accuracy / feature items. These map onto
464+
the acid-test categories in `test/acid/README.md`; tracking here so
465+
they don't get lost:
466+
467+
- **"Fix VC behavior to match what a real Jaguar does. Still not
468+
sure just what the heck is going on there." [Shamus]**
469+
acid `timing/`. Active suspect for the Doom 1.5-2x speed
470+
regression (issue #131).
471+
- **"Cycle accuracy for GPU/DSP/OP/Blitter." [Shamus]**
472+
cross-cutting; informs every category in `test/acid/`, especially
473+
`bus/` (which can't pass without it).
474+
- **"Need to propagate blitter fixes in the A1 <- A2 direction
475+
to the A1 -> A2 direction and the GPU fixes to various
476+
instructions to the DSP." [Shamus]** — acid `blitter/` (A1↔A2
477+
symmetry tests) and `gpu/` + `dsp/` (shared opcode coverage).
478+
- **"Blitter needs fixing." [Shamus]** — acid `blitter/`.
479+
PR #129 fixed a perf-relevant chunk (`ADDARRAY` etc); accuracy
480+
axis still wide open.
481+
- **"Need to emulate bus contention." [Shamus]** — acid `bus/`.
482+
Almost certainly load-bearing for the Doom regression and the
483+
AvP audio dropouts.
484+
- **"Need to fix timing in the OP. As it is now, it gives a false
485+
impression of how much it's capable of." [Shamus]**
486+
acid `op/`.
487+
488+
The original `docs/TODO` is intentionally left untouched — it's
489+
the authors' historical record and we track our own work via
490+
GitHub issues + this file + `test/acid/`.

src/core/jaguar.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "jaguar.h"
2020

2121
#include "cdrom.h"
22+
#include "perf_counters.h"
2223
#include "dac.h"
2324
#include "dsp.h"
2425
#include "eeprom.h"
@@ -33,6 +34,16 @@
3334

3435
static bool frameDone;
3536

37+
/* Frame-pacing instrumentation (no-op unless built with BENCH_PROFILE).
38+
* Lets the acid runner / benchmark detect timing regressions like the
39+
* Doom 2x speed bug -- e.g. expected 525 halflines/frame NTSC, 60 vblank
40+
* IRQs/sec. See test/acid/README.md and src/core/perf_counters.h.
41+
* Counters that fire from other TUs are declared at their use sites
42+
* (PERF_COUNTER backs each name with a file-scope static). */
43+
PERF_COUNTER(timing_halfline_callbacks);
44+
PERF_COUNTER(timing_vblank_irqs);
45+
PERF_COUNTER(timing_jaguar_execute_calls);
46+
3647
// Platform-independent xorshift32 PRNG for deterministic RAM initialization.
3748
// libc rand() produces different sequences on different platforms (glibc vs
3849
// macOS libsystem), which causes cross-platform baseline mismatches.
@@ -694,7 +705,8 @@ void JaguarInit(void)
694705
// Half line times are, naturally, half of this. :-P
695706
void HalflineCallback(void)
696707
{
697-
uint16_t vc = TOMReadWord(0xF00006, JAGUAR);
708+
uint16_t vc = (PERF_INC(timing_halfline_callbacks),
709+
TOMReadWord(0xF00006, JAGUAR));
698710
uint16_t vp = TOMReadWord(0xF0003E, JAGUAR) + 1;
699711
uint16_t vi = TOMReadWord(0xF0004E, JAGUAR);
700712

@@ -712,7 +724,10 @@ void HalflineCallback(void)
712724

713725
// Time for Vertical Interrupt?
714726
if ((vc & 0x7FF) == vi && (vc & 0x7FF) > 0)
727+
{
728+
PERF_INC(timing_vblank_irqs);
715729
TOMSetPendingVideoInt();
730+
}
716731

717732
TOMExecHalfline(vc, true);
718733

@@ -934,6 +949,7 @@ uint8_t * GetRamPtr(void)
934949
* so the DSP runs alongside the 68K and GPU, matching real hardware timing. */
935950
void JaguarExecuteNew(void)
936951
{
952+
PERF_INC(timing_jaguar_execute_calls);
937953
frameDone = false;
938954

939955
do

src/jerry/jerry.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,9 @@
162162
#include "eeprom.h"
163163
#include "event.h"
164164
#include "jaguar.h"
165+
#include "perf_counters.h"
166+
167+
PERF_COUNTER(timing_jerry_irqs);
165168
#include "joystick.h"
166169
#include "m68000/m68kinterface.h"
167170
#include "memtrack.h"
@@ -250,6 +253,7 @@ void JERRYPIT1Callback(void)
250253
// Not sure, but I think we don't generate another IRQ if one's already going...
251254
// But this seems to work... :-/
252255
jerryPendingInterrupt |= IRQ2_TIMER1;
256+
PERF_INC(timing_jerry_irqs);
253257
m68k_set_irq(2); // Generate 68K IPL 2
254258
}
255259
}
@@ -266,6 +270,7 @@ void JERRYPIT2Callback(void)
266270
if (jerryInterruptMask & IRQ2_TIMER2) // CPU Timer 2 IRQ
267271
{
268272
jerryPendingInterrupt |= IRQ2_TIMER2;
273+
PERF_INC(timing_jerry_irqs);
269274
m68k_set_irq(2); // Generate 68K IPL 2
270275
}
271276
}

src/tom/tom.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,11 @@
262262
#include "jaguar.h"
263263
#include "m68000/m68kinterface.h"
264264
#include "op.h"
265+
#include "perf_counters.h"
265266
#include "settings.h"
266267

268+
PERF_COUNTER(timing_gpu_irqs_to_68k);
269+
267270
// Red Color Values for CrY<->RGB Color Conversion
268271
uint8_t redcv[16][16] = {
269272
// 0 1 2 3 4 5 6 7 8 9 A B C D E F
@@ -1316,7 +1319,10 @@ void TOMExecPIT(uint32_t cycles)
13161319
GPUSetIRQLine(GPUIRQ_TIMER, ASSERT_LINE); // GPUSetIRQLine does the 'IRQ enabled' checking
13171320

13181321
if (TOMIRQEnabled(IRQ_TIMER))
1322+
{
1323+
PERF_INC(timing_gpu_irqs_to_68k);
13191324
m68k_set_irq(2); // Cause a 68000 IPL 2...
1325+
}
13201326

13211327
TOMResetPIT();
13221328
}
@@ -1329,7 +1335,10 @@ void TOMPITCallback(void)
13291335
GPUSetIRQLine(GPUIRQ_TIMER, ASSERT_LINE); // It does the 'IRQ enabled' checking
13301336

13311337
if (TOMIRQEnabled(IRQ_TIMER))
1338+
{
1339+
PERF_INC(timing_gpu_irqs_to_68k);
13321340
m68k_set_irq(2); // Generate a 68K IPL 2...
1341+
}
13331342

13341343
TOMResetPIT();
13351344
}

0 commit comments

Comments
 (0)