mirror of
https://github.com/libretro/virtualjaguar-libretro.git
synced 2026-11-07 04:36:00 -10:00
perf(dsp): build the untimed DSP loop without GDB code when nothing is armed (#1057)
* perf(dsp): build the untimed loop without GDB code when nothing is armed DSPExecUntimed() is now compiled twice from dsp_exec_untimed.h: a plain instance with no breakpoint test and no idle-skip gate, and an armed instance used only while gdbArmedDSP is set. DSPExec() picks one per slice, alongside the existing timed/untimed split. The armed instance keeps the breakpoint test where it was, after the IRQ dispatch and the PC-escape bail, so a breakpoint on an ISR's first instruction still fires. Arming is sampled once per slice, as before. Preprocessed, the plain loop is develop's loop with the GDB lines removed and nothing else changed. Refs #1054 Co-Authored-By: Claude Opus 5.5 <[email protected]> * docs(debug): record the #1054 DSP loop specialisation and its layout caveat The parked specialisation now ships, so the design doc's "parked" and "~2% unexplained" text no longer describes the code. Records the A/B/B/A numbers. It also records that code layout moves this loop by about +/-1%: compiling the check out makes real games slower, and the port beats the ceiling on identical C. The claim stays narrow: yarc gains, games are neutral. It also notes that the DSP_HOOK control arm will now read about 0. Refs #1054 Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(dsp): keep the local-RAM opcode fetch inside dsp_ram_8 The six fetch sites (both exec loops and the four delay-slot fetches) read dsp_ram_8[off] and [off + 1] whenever dsp_pc was below base + 0x2000. So an odd PC at base + 0x1FFF read one byte past the array. cppcheck flagged it (arrayIndexOutOfBoundsCond) once the code moved into a changed file. The bound is now base + 0x2000 - 1, the same form gpu.c already uses. That one address now goes through DSPReadWord(), which aligns. Every other PC is byte-identical. clang-tidy now skips src/jerry/dsp_exec_untimed.h the way it skips the *_simd_* fragments: on its own it is only undeclared dsp.c statics. Its code is still linted through dsp.c via HeaderFilterRegex. Refs #1054 Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
2abb032ab5
commit
9c5656e271
4 files changed
+340
-261
No files matched your search
@@ -811,8 +811,15 @@ jobs:
|
||||
# capability macros and includes). They are covered instead by
|
||||
# c89-lint's per-arch pass, which compiles them the way the
|
||||
# build does.
|
||||
#
|
||||
# src/jerry/dsp_exec_untimed.h is the same kind of fragment: a
|
||||
# function body dsp.c includes twice, using dsp.c's statics.
|
||||
# Standalone it is nothing but undeclared identifiers. Its code
|
||||
# is still linted, through dsp.c, because HeaderFilterRegex
|
||||
# covers src/jerry.
|
||||
FILES=$(git diff --name-only --diff-filter=d "$BASE"...HEAD -- '*.c' '*.h' \
|
||||
| grep -Ev '^(src/m68000/|src/bios/jag.*\.c$|libretro-common/|src/core/version\.h$|src/(tom|jerry)/[a-z0-9_]+_simd_(neon|sse2|scalar)\.(c|h)$)' \
|
||||
| grep -v '^src/jerry/dsp_exec_untimed\.h$' \
|
||||
| grep -E '^(src/|libretro\.c$)' || true)
|
||||
if [ -z "$FILES" ]; then
|
||||
echo "No relevant C files changed; skipping clang-tidy."
|
||||
|
||||
+33
-1
@@ -416,6 +416,37 @@ Five layers, in increasing cost:
|
||||
(digest-verified, C89-clean) rather than merged -- 0.7% did not justify
|
||||
an extracted-body header in the emulator's hottest function.
|
||||
|
||||
**Update (issue #1054, 2026-10): the DSP half is now specialised.**
|
||||
By October develop had split DSPExec() into timed and untimed loops,
|
||||
and the untimed loop is now built twice from `src/jerry/dsp_exec_untimed.h`:
|
||||
a plain instance with no GDB code, and an armed instance that DSPExec()
|
||||
enters only while `gdbArmedDSP` is set. Measured on an M4 Max, A/B/B/A,
|
||||
load 2-4 unless noted:
|
||||
|
||||
| Arms | ROM | Median | Best-run | p |
|
||||
|---|---|---|---|---|
|
||||
| develop vs DSP check compiled out (the ceiling) | yarc | +2.7% | +2.3% | 0.0003 |
|
||||
| develop vs port | yarc | +2.6% | +3.5% | 0.0006 |
|
||||
| develop vs port (repeat, 5-min load ~10) | yarc | +1.6% | +3.3% | 0.0092 |
|
||||
| develop vs port, both DSP-armed (load rose 2 -> 17) | yarc | +2.5% | +0.3% | 0.1078 |
|
||||
| develop vs port | Tempest 2000 | -0.2% | +0.2% | 0.7415 |
|
||||
| develop vs port | Doom | -0.8% | -0.4% | 0.6801 |
|
||||
| develop vs ceiling | Tempest 2000 | -1.0% | -1.3% | 0.0007 |
|
||||
| develop vs ceiling | Doom | -0.6% | -1.3% | 0.0168 |
|
||||
| ceiling vs port | yarc | +1.1% | +0.9% | 0.0018 |
|
||||
|
||||
Read the last three rows before quoting a cause. Compiling the check
|
||||
out makes real games about 1% *slower*, and the port beats the ceiling
|
||||
on yarc although its plain loop is the same C. Code layout moves this
|
||||
loop by about +/-1%, the same size as the check. So the claim is
|
||||
narrow: the default DSP path carries no GDB code, yarc gains, and games
|
||||
are neutral. A "the check cost 2%" reading is not supported.
|
||||
|
||||
One consequence for the control arms: after this change
|
||||
`VJ_GDB_STUB_DISABLE_DSP_HOOK=1` compares two nearly identical plain
|
||||
loops, so the DSP row of a future attribution run should read about 0.
|
||||
That does not mean the arm is broken.
|
||||
|
||||
**Next step is real-hardware profiling, not more host measurement.**
|
||||
Every number here comes from an arm64 Mac that never dropped below load
|
||||
5. The residual should be re-attributed on a real target (tvOS/A-series,
|
||||
@@ -429,7 +460,8 @@ Five layers, in increasing cost:
|
||||
confident-looking p-value.
|
||||
|
||||
Shipped-by-default stands for now at a measured ~2% cost, deliberately,
|
||||
pending those numbers.
|
||||
pending those numbers. (Since #1054 the default DSP path carries no GDB
|
||||
code at all; see the update above.)
|
||||
|
||||
|
||||
|
||||
|
||||
+23
-260
@@ -2445,276 +2445,39 @@ static int32_t DSPIdleLoopProbe(int32_t cycles, uint32_t head, uint32_t jrAddr)
|
||||
|
||||
static DSP_NOINLINE void DSPExecTimed(int32_t cycles);
|
||||
static DSP_NOINLINE void DSPExecUntimed(int32_t cycles);
|
||||
#if !defined(VJ_GDB_STUB_DISABLE_HOOKS) && !defined(VJ_GDB_STUB_DISABLE_DSP_HOOK)
|
||||
static DSP_NOINLINE void DSPExecUntimedArmed(int32_t cycles);
|
||||
#endif
|
||||
|
||||
/* Timing models (pipeline or dram_timing) run a separate loop, picked
|
||||
* once per call, so the untimed loop below and the opcode handlers it
|
||||
* dispatches are exactly develop's (issue #313 / #892). Both loops are
|
||||
* out of line so neither can merge into, or re-allocate, the other. */
|
||||
* dispatches are exactly develop's (issue #313 / #892). The untimed loop
|
||||
* is itself built twice, with and without the GDB breakpoint test (issue
|
||||
* #1054, see dsp_exec_untimed.h), so the default path carries no debugger
|
||||
* code. All loops are out of line so none can merge into, or
|
||||
* re-allocate, another. */
|
||||
void DSPExec(int32_t cycles)
|
||||
{
|
||||
if (vjs.gpuPipelineTiming || busArbiter.enabled)
|
||||
DSPExecTimed(cycles);
|
||||
#if !defined(VJ_GDB_STUB_DISABLE_HOOKS) && !defined(VJ_GDB_STUB_DISABLE_DSP_HOOK)
|
||||
else if (gdbArmedDSP)
|
||||
DSPExecUntimedArmed(cycles);
|
||||
#endif
|
||||
else
|
||||
DSPExecUntimed(cycles);
|
||||
}
|
||||
|
||||
static DSP_NOINLINE void DSPExecUntimed(int32_t cycles)
|
||||
{
|
||||
int idleSkipActive;
|
||||
int gdbArmedSlice;
|
||||
/* Issue #811 late load write-back, armed for exactly the next
|
||||
* instruction (see dsp_ldx_slot). */
|
||||
uint32_t *ldxArmed = NULL;
|
||||
uint32_t ldxVal = 0, ldxPc = 0;
|
||||
/* Issue #811 stale indexed-store data after a divide (see
|
||||
* dsp_div_store_fixups), armed for exactly the next instruction. */
|
||||
uint32_t *divArmed = NULL;
|
||||
uint32_t divOld = 0, divPc = 0;
|
||||
|
||||
#ifdef DSP_SINGLE_STEPPING
|
||||
if (dsp_control & 0x18)
|
||||
{
|
||||
cycles = 1;
|
||||
dsp_control &= ~0x10;
|
||||
}
|
||||
#endif
|
||||
/* No return between here and VJP_LEAVE (perf_iface.h). */
|
||||
VJP_ENTER(VJP_DSP);
|
||||
|
||||
dsp_releaseTimeSlice_flag = 0;
|
||||
|
||||
/* Same shape as GPUExec's cached locals (issue #532): a hot global
|
||||
* read per emulated instruction with an opaque opcode call in
|
||||
* between. Measured (issue #652): this removes part of the DSP's
|
||||
* share of the GDB hook cost; ~2%% remains and is NOT explained --
|
||||
* see docs/gdb-stub-design.md. Only GDBHalt() can re-arm inside a
|
||||
* slice. */
|
||||
gdbArmedSlice = gdbArmedDSP;
|
||||
dsp_in_exec++;
|
||||
|
||||
/* Idle-loop fast-forward gates (issue #569). Every one of these adds
|
||||
* per-instruction state the affine extrapolation does not model, so
|
||||
* any of them turns the whole thing off:
|
||||
* - vjs.riscIdleSkip: the user-facing core option itself;
|
||||
* - blitMemoMode / blitMemoRecording: the JaguarRead and JaguarWrite
|
||||
* families note reads and writes for the memo (blit_memo.h:64-65,
|
||||
* src/core/jaguar.c:1139), and skipping loads would change what
|
||||
* the memo records;
|
||||
* - riscClockScalePct is deliberately NOT a gate (#378 overclock
|
||||
* presets): SCALE_RISC_CYCLES is applied where the budget is handed out (jaguar.c), not inside
|
||||
* DSPExec, so `cycles` here is already in the same currency the
|
||||
* measured per-iteration cost was charged in -- cost and opcost
|
||||
* are measured from live `cycles` deltas, never recomputed, and
|
||||
* nothing per-instruction in this function depends on the scale.
|
||||
* Scaling `n` or `cost` again would double-scale. The GPU has
|
||||
* one scale-dependent per-instruction term (gpu_stall_scale_accum);
|
||||
* see the note in GPUExec;
|
||||
* - busArbiter.enabled (the dram_timing option): bus occupancy is
|
||||
* accounted per access. Belt-and-braces here -- dsp.c never
|
||||
* touches busArbiter and JaguarReadLong charges it only for
|
||||
* `who == OP` -- but a DSP-side arbiter charge added later must
|
||||
* not silently invalidate this;
|
||||
* - vjs.gpuPipelineTiming: drives the DSP pipeline/gateway model
|
||||
* (issue #313), which runs in DSPExecTimed() and never reaches
|
||||
* this loop -- kept in the gate as belt-and-braces;
|
||||
* - DSP_CTRL single-step / single-go: the slice is one instruction;
|
||||
* - vjtrace armed or a memory watch installed (VJ_TRACE builds
|
||||
* only): VJT_PCHIST_DSP in this loop and VJT_WATCH_RD on the
|
||||
* DRAM read path are per-instruction / per-read side effects.
|
||||
* This has to be a RUNTIME check, not `#ifdef VJ_TRACE`: the
|
||||
* whole test ABI is built with -DVJ_TRACE (Makefile:158-161),
|
||||
* so a compile-time gate would make every headless harness --
|
||||
* including the A/B that proves this exact -- silently measure
|
||||
* a disabled feature. Both flags default off, so the cost is
|
||||
* two loads per slice on trace builds and nothing at all on
|
||||
* release builds. */
|
||||
idleSkipActive = vjs.riscIdleSkip
|
||||
&& !blitMemoMode && !blitMemoRecording
|
||||
&& !busArbiter.enabled
|
||||
&& !vjs.gpuPipelineTiming
|
||||
&& !(dsp_control & 0x18);
|
||||
#ifdef VJ_TRACE
|
||||
if (vjtrace_armed || vjtrace_nwatch)
|
||||
idleSkipActive = 0;
|
||||
#endif
|
||||
#if !defined(VJ_GDB_STUB_DISABLE_HOOKS) && !defined(VJ_GDB_STUB_DISABLE_IDLE_GATE)
|
||||
/* GDB stub (issue #652): DSPIdleLoopProbe() extrapolates the PC
|
||||
* forward over many idle-loop iterations without visiting each one,
|
||||
* exactly the class of per-instruction side effect the #569 idle-skip
|
||||
* gate list above already disables for (blit memo, busArbiter,
|
||||
* vjtrace watch). A DSP breakpoint or pending step would be stepped
|
||||
* clean over. This is a RUNTIME check, same reasoning as the VJ_TRACE
|
||||
* block above: gdbArmedDSP defaults to 0 and costs one load + branch
|
||||
* when nothing is armed. VJ_GDB_STUB_DISABLE_HOOKS exists only for
|
||||
* the Phase 2 A/B perf measurement. */
|
||||
if (gdbArmedDSP)
|
||||
idleSkipActive = 0;
|
||||
#endif
|
||||
/* Probe + reject memo are re-derived from scratch every call, so
|
||||
* nothing here reaches a savestate and nothing survives a slice. */
|
||||
idleProbeStage = 0;
|
||||
idleMemoCount = 0;
|
||||
idleMemoNext = 0;
|
||||
dsp_ldx_slot = NULL;
|
||||
|
||||
/* An armed late write-back runs one instruction past the budget so a
|
||||
* load/overwrite pair never straddles a slice (see dsp_ldx_slot);
|
||||
* likewise a divide/indexed-store pair (see dsp_div_store_fixups). */
|
||||
while ((cycles > 0 || ldxArmed || divArmed) && DSP_RUNNING)
|
||||
{
|
||||
uint16_t opcode;
|
||||
uint32_t index;
|
||||
uint32_t pcThis;
|
||||
#ifdef VJ_TRACE
|
||||
VJT_PCHIST_DSP(dsp_pc);
|
||||
#endif
|
||||
|
||||
/* If IMASK was cleared, see if any other interrupts are pending --
|
||||
* but not until the D_FLAGS store that cleared it has retired, so
|
||||
* the instruction still in the pipeline behind it (typically the
|
||||
* epilogue's `jump` back to the interrupted code) runs first. */
|
||||
if (IMASKCleared && dspFlagsRetireDelay == 0)
|
||||
{
|
||||
DSPHandleIRQsNP();
|
||||
IMASKCleared = false;
|
||||
}
|
||||
|
||||
/* PC escape bail-out. When the DSP PC has wandered into a
|
||||
* region that doesn't contain executable code -- register
|
||||
* space at $F00000-$F1FFFF outside DSP local SRAM, or the
|
||||
* unmapped territory above $E40000 -- every "fetched opcode"
|
||||
* is bus-default 0xFFFF garbage that decodes to a near-zero-
|
||||
* cost opcode. The inner loop then burns the entire
|
||||
* timeslice without making progress, hanging the frontend.
|
||||
*
|
||||
* Wolf3D headless triage caught this in v2.3.0: the runtime
|
||||
* watchdog logged `dsp_pc_escape pc=$00FFF004E8` (PC top-byte
|
||||
* corrupted) and the harness wedged for 12+ minutes per
|
||||
* frame. An earlier dsp-diag snapshot showed PC=$0006EE in
|
||||
* RAM at frame 48 -- that's the upstream bug (separate from
|
||||
* this bail-out: $0006EE is *valid* RAM and decodes to real
|
||||
* opcodes; it's where the DSP eventually drifts INTO bad
|
||||
* territory that triggers the wedge). Drain cycles here and
|
||||
* let the runtime watchdog (src/core/crash_detect.c) log the
|
||||
* actual escape PC. DSP_RUNNING is left alone so games that
|
||||
* legitimately stop the DSP via DSPGO=0 are unaffected.
|
||||
*
|
||||
* Valid execution regions match JaguarReadX address decoding
|
||||
* (src/core/jaguar.c): anything <= $E3FFFF (main RAM mirrored
|
||||
* 4x for the bottom 8MB, cart ROM, boot ROM) plus DSP local
|
||||
* SRAM. Earlier versions of this check used `<= 0x1FFFFF`
|
||||
* and would have false-flagged DSP code running from a RAM
|
||||
* mirror at $200000-$7FFFFF or from cart ROM at $800000+.
|
||||
* Caught by Copilot review on PR #182. */
|
||||
if (!((dsp_pc <= 0x00E3FFFF) ||
|
||||
(dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)))
|
||||
{
|
||||
cycles = 0;
|
||||
break;
|
||||
}
|
||||
|
||||
pcThis = dsp_pc;
|
||||
#define DSP_UNTIMED_NAME DSPExecUntimed
|
||||
#define DSP_UNTIMED_GDB 0
|
||||
#include "dsp_exec_untimed.h"
|
||||
|
||||
#if !defined(VJ_GDB_STUB_DISABLE_HOOKS) && !defined(VJ_GDB_STUB_DISABLE_DSP_HOOK)
|
||||
/* GDB stub (issue #652): one load of a hot global, one
|
||||
* never-taken branch when no DSP breakpoint/step is armed -- see
|
||||
* docs/gdb-stub-design.md "Breakpoint detection". GDBHalt()
|
||||
* blocks in place (does not unwind this loop) until the client
|
||||
* continues, steps, or disconnects. VJ_GDB_STUB_DISABLE_HOOKS
|
||||
* exists only for the Phase 2 A/B perf measurement -- see the
|
||||
* comment in src/core/jaguar.c's M68KInstructionHook(). */
|
||||
if (gdbArmedSlice && GDBCheckPC(GDB_TGT_DSP, pcThis))
|
||||
{
|
||||
GDBHalt(GDB_TGT_DSP, GDB_STOP_BREAKPOINT, pcThis);
|
||||
gdbArmedSlice = gdbArmedDSP;
|
||||
}
|
||||
#define DSP_UNTIMED_NAME DSPExecUntimedArmed
|
||||
#define DSP_UNTIMED_GDB 1
|
||||
#include "dsp_exec_untimed.h"
|
||||
#endif
|
||||
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
}
|
||||
else
|
||||
opcode = DSPReadWord(dsp_pc, DSP);
|
||||
index = opcode >> 10;
|
||||
dsp_opcode_first_parameter = (opcode >> 5) & 0x1F;
|
||||
dsp_opcode_second_parameter = opcode & 0x1F;
|
||||
dsp_pc += 2;
|
||||
dsp_exec_opcode_count++;
|
||||
if (divArmed && pcThis == divPc
|
||||
&& &dsp_reg[dsp_opcode_second_parameter] == divArmed)
|
||||
{
|
||||
/* Issue #811: indexed store right behind a divide into its
|
||||
* data register -- the quotient has not been written back
|
||||
* yet, so the store sees the old contents. */
|
||||
uint32_t quot = *divArmed;
|
||||
|
||||
*divArmed = divOld;
|
||||
dsp_executeOpcode(index);
|
||||
*divArmed = quot;
|
||||
dsp_div_store_fixups++;
|
||||
divArmed = NULL;
|
||||
}
|
||||
else if (index == 21)
|
||||
{
|
||||
uint32_t *dst = &dsp_reg[dsp_opcode_second_parameter];
|
||||
uint32_t old = *dst;
|
||||
|
||||
dsp_executeOpcode(index);
|
||||
divArmed = DSPDivStorePeek(dst, &divPc);
|
||||
divOld = old;
|
||||
}
|
||||
else
|
||||
{
|
||||
dsp_executeOpcode(index);
|
||||
divArmed = NULL;
|
||||
}
|
||||
cycles -= dsp_opcode_cycles[index];
|
||||
|
||||
/* Issue #811: the previous instruction was an external load and
|
||||
* this one overwrote its target -- the load data lands last. */
|
||||
if (ldxArmed)
|
||||
{
|
||||
if (pcThis == ldxPc
|
||||
&& &dsp_reg[dsp_opcode_second_parameter] == ldxArmed)
|
||||
{
|
||||
*ldxArmed = ldxVal;
|
||||
dsp_load_wb_fixups++;
|
||||
}
|
||||
ldxArmed = NULL;
|
||||
}
|
||||
if (dsp_ldx_slot)
|
||||
{
|
||||
ldxArmed = DSPLoadOverwritePeek(&ldxVal, &ldxPc);
|
||||
dsp_ldx_slot = NULL;
|
||||
}
|
||||
|
||||
/* Idle-loop fast-forward (issue #569). A taken `jr` that landed
|
||||
* on or behind its own address, at most 8 words back, is the only
|
||||
* candidate; the unsigned difference rejects a not-taken jr
|
||||
* (pc = pcThis + 2) and every forward branch in one compare. */
|
||||
if (idleSkipActive && index == 53
|
||||
&& (uint32_t)(pcThis - dsp_pc) <= 14)
|
||||
cycles = DSPIdleLoopProbe(cycles, dsp_pc, pcThis);
|
||||
|
||||
/* Age out a D_FLAGS store once the instruction that was already
|
||||
* behind it in the pipeline has run (see DSPWriteLong, D_FLAGS
|
||||
* case). Retiring re-opens interrupt recognition and commits the
|
||||
* register-bank switch the store selected. */
|
||||
if (dspFlagsRetireDelay)
|
||||
{
|
||||
dspFlagsRetireDelay--;
|
||||
if (dspFlagsRetireDelay == 0)
|
||||
DSPUpdateRegisterBanks();
|
||||
}
|
||||
}
|
||||
|
||||
dsp_in_exec--;
|
||||
|
||||
VJP_LEAVE(VJP_DSP);
|
||||
}
|
||||
|
||||
INLINE static void dsp_executeOpcode(uint32_t index)
|
||||
{
|
||||
#ifdef __GNUC__
|
||||
@@ -2929,7 +2692,7 @@ INLINE static void dsp_opcode_jump(void)
|
||||
* PC before applying the branch target. This replaces the old
|
||||
* recursive DSPExec(1) call, avoiding full function-call overhead,
|
||||
* redundant IRQ checks, and pipeline-state save/restore. */
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000 - 1)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
ds_opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
@@ -2976,7 +2739,7 @@ INLINE static void dsp_opcode_jr(void)
|
||||
/* Inline delay-slot: fetch-decode-execute one instruction at current
|
||||
* PC before applying the branch target. Same rationale as in
|
||||
* dsp_opcode_jump above. */
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000 - 1)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
ds_opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
@@ -4811,7 +4574,7 @@ static void dsp_opcode_jump_t(void)
|
||||
dspFlagsRetireDelay = 0;
|
||||
DSPUpdateRegisterBanks();
|
||||
}
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000 - 1)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
ds_opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
@@ -4860,7 +4623,7 @@ static void dsp_opcode_jr_t(void)
|
||||
dspFlagsRetireDelay = 0;
|
||||
DSPUpdateRegisterBanks();
|
||||
}
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000 - 1)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
ds_opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
@@ -4964,7 +4727,7 @@ static DSP_NOINLINE void DSPExecTimed(int32_t cycles)
|
||||
}
|
||||
#endif
|
||||
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000 - 1)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
|
||||
@@ -0,0 +1,277 @@
|
||||
/* dsp_exec_untimed.h -- DSPExecUntimed(), the default DSP inner loop.
|
||||
*
|
||||
* Included TWICE from dsp.c (issue #1054), once with DSP_UNTIMED_GDB 0
|
||||
* and once with 1, to make two separate out-of-line functions:
|
||||
*
|
||||
* DSPExecUntimed no GDB code at all -- the path every user runs;
|
||||
* DSPExecUntimedArmed per-instruction breakpoint test, idle skip off.
|
||||
*
|
||||
* DSPExec() picks one per slice from gdbArmedDSP, so a DSP breakpoint
|
||||
* set while a plain slice is on the stack (from a watchpoint halt, say)
|
||||
* takes effect at the next slice. That is the same once-per-slice
|
||||
* sampling the single loop already had: it cached gdbArmedDSP in a local
|
||||
* at slice entry and re-read it only after its own GDBHalt().
|
||||
*
|
||||
* The includer defines DSP_UNTIMED_NAME and DSP_UNTIMED_GDB; this file
|
||||
* #undefs both. dsp.c is the only includer; no include guard, by design.
|
||||
*/
|
||||
static DSP_NOINLINE void DSP_UNTIMED_NAME(int32_t cycles)
|
||||
{
|
||||
int idleSkipActive;
|
||||
#if DSP_UNTIMED_GDB
|
||||
int gdbArmedSlice;
|
||||
#endif
|
||||
/* Issue #811 late load write-back, armed for exactly the next
|
||||
* instruction (see dsp_ldx_slot). */
|
||||
uint32_t *ldxArmed = NULL;
|
||||
uint32_t ldxVal = 0, ldxPc = 0;
|
||||
/* Issue #811 stale indexed-store data after a divide (see
|
||||
* dsp_div_store_fixups), armed for exactly the next instruction. */
|
||||
uint32_t *divArmed = NULL;
|
||||
uint32_t divOld = 0, divPc = 0;
|
||||
|
||||
#ifdef DSP_SINGLE_STEPPING
|
||||
if (dsp_control & 0x18)
|
||||
{
|
||||
cycles = 1;
|
||||
dsp_control &= ~0x10;
|
||||
}
|
||||
#endif
|
||||
/* No return between here and VJP_LEAVE (perf_iface.h). */
|
||||
VJP_ENTER(VJP_DSP);
|
||||
|
||||
dsp_releaseTimeSlice_flag = 0;
|
||||
|
||||
#if DSP_UNTIMED_GDB
|
||||
/* Same shape as GPUExec's cached locals (issue #532): a hot global
|
||||
* read per emulated instruction with an opaque opcode call in
|
||||
* between. Only GDBHalt() can re-arm inside a slice. */
|
||||
gdbArmedSlice = gdbArmedDSP;
|
||||
#endif
|
||||
dsp_in_exec++;
|
||||
|
||||
/* Idle-loop fast-forward gates (issue #569). Every one of these adds
|
||||
* per-instruction state the affine extrapolation does not model, so
|
||||
* any of them turns the whole thing off:
|
||||
* - vjs.riscIdleSkip: the user-facing core option itself;
|
||||
* - blitMemoMode / blitMemoRecording: the JaguarRead and JaguarWrite
|
||||
* families note reads and writes for the memo (blit_memo.h:64-65,
|
||||
* src/core/jaguar.c:1139), and skipping loads would change what
|
||||
* the memo records;
|
||||
* - riscClockScalePct is deliberately NOT a gate (#378 overclock
|
||||
* presets): SCALE_RISC_CYCLES is applied where the budget is handed out (jaguar.c), not inside
|
||||
* DSPExec, so `cycles` here is already in the same currency the
|
||||
* measured per-iteration cost was charged in -- cost and opcost
|
||||
* are measured from live `cycles` deltas, never recomputed, and
|
||||
* nothing per-instruction in this function depends on the scale.
|
||||
* Scaling `n` or `cost` again would double-scale. The GPU has
|
||||
* one scale-dependent per-instruction term (gpu_stall_scale_accum);
|
||||
* see the note in GPUExec;
|
||||
* - busArbiter.enabled (the dram_timing option): bus occupancy is
|
||||
* accounted per access. Belt-and-braces here -- dsp.c never
|
||||
* touches busArbiter and JaguarReadLong charges it only for
|
||||
* `who == OP` -- but a DSP-side arbiter charge added later must
|
||||
* not silently invalidate this;
|
||||
* - vjs.gpuPipelineTiming: drives the DSP pipeline/gateway model
|
||||
* (issue #313), which runs in DSPExecTimed() and never reaches
|
||||
* this loop -- kept in the gate as belt-and-braces;
|
||||
* - DSP_CTRL single-step / single-go: the slice is one instruction;
|
||||
* - vjtrace armed or a memory watch installed (VJ_TRACE builds
|
||||
* only): VJT_PCHIST_DSP in this loop and VJT_WATCH_RD on the
|
||||
* DRAM read path are per-instruction / per-read side effects.
|
||||
* This has to be a RUNTIME check, not `#ifdef VJ_TRACE`: the
|
||||
* whole test ABI is built with -DVJ_TRACE (Makefile:158-161),
|
||||
* so a compile-time gate would make every headless harness --
|
||||
* including the A/B that proves this exact -- silently measure
|
||||
* a disabled feature. Both flags default off, so the cost is
|
||||
* two loads per slice on trace builds and nothing at all on
|
||||
* release builds. */
|
||||
idleSkipActive = vjs.riscIdleSkip
|
||||
&& !blitMemoMode && !blitMemoRecording
|
||||
&& !busArbiter.enabled
|
||||
&& !vjs.gpuPipelineTiming
|
||||
&& !(dsp_control & 0x18);
|
||||
#ifdef VJ_TRACE
|
||||
if (vjtrace_armed || vjtrace_nwatch)
|
||||
idleSkipActive = 0;
|
||||
#endif
|
||||
#if DSP_UNTIMED_GDB && !defined(VJ_GDB_STUB_DISABLE_IDLE_GATE)
|
||||
/* GDB stub (issue #652): DSPIdleLoopProbe() extrapolates the PC
|
||||
* forward over many idle-loop iterations without visiting each one,
|
||||
* exactly the class of per-instruction side effect the #569 idle-skip
|
||||
* gate list above already disables for (blit memo, busArbiter,
|
||||
* vjtrace watch). A DSP breakpoint or pending step would be stepped
|
||||
* clean over. DSPExec() only enters this instance with gdbArmedDSP
|
||||
* set, so the gate is unconditional here and absent from the plain
|
||||
* instance. */
|
||||
idleSkipActive = 0;
|
||||
#endif
|
||||
/* Probe + reject memo are re-derived from scratch every call, so
|
||||
* nothing here reaches a savestate and nothing survives a slice. */
|
||||
idleProbeStage = 0;
|
||||
idleMemoCount = 0;
|
||||
idleMemoNext = 0;
|
||||
dsp_ldx_slot = NULL;
|
||||
|
||||
/* An armed late write-back runs one instruction past the budget so a
|
||||
* load/overwrite pair never straddles a slice (see dsp_ldx_slot);
|
||||
* likewise a divide/indexed-store pair (see dsp_div_store_fixups). */
|
||||
while ((cycles > 0 || ldxArmed || divArmed) && DSP_RUNNING)
|
||||
{
|
||||
uint16_t opcode;
|
||||
uint32_t index;
|
||||
uint32_t pcThis;
|
||||
#ifdef VJ_TRACE
|
||||
VJT_PCHIST_DSP(dsp_pc);
|
||||
#endif
|
||||
|
||||
/* If IMASK was cleared, see if any other interrupts are pending --
|
||||
* but not until the D_FLAGS store that cleared it has retired, so
|
||||
* the instruction still in the pipeline behind it (typically the
|
||||
* epilogue's `jump` back to the interrupted code) runs first. */
|
||||
if (IMASKCleared && dspFlagsRetireDelay == 0)
|
||||
{
|
||||
DSPHandleIRQsNP();
|
||||
IMASKCleared = false;
|
||||
}
|
||||
|
||||
/* PC escape bail-out. When the DSP PC has wandered into a
|
||||
* region that doesn't contain executable code -- register
|
||||
* space at $F00000-$F1FFFF outside DSP local SRAM, or the
|
||||
* unmapped territory above $E40000 -- every "fetched opcode"
|
||||
* is bus-default 0xFFFF garbage that decodes to a near-zero-
|
||||
* cost opcode. The inner loop then burns the entire
|
||||
* timeslice without making progress, hanging the frontend.
|
||||
*
|
||||
* Wolf3D headless triage caught this in v2.3.0: the runtime
|
||||
* watchdog logged `dsp_pc_escape pc=$00FFF004E8` (PC top-byte
|
||||
* corrupted) and the harness wedged for 12+ minutes per
|
||||
* frame. An earlier dsp-diag snapshot showed PC=$0006EE in
|
||||
* RAM at frame 48 -- that's the upstream bug (separate from
|
||||
* this bail-out: $0006EE is *valid* RAM and decodes to real
|
||||
* opcodes; it's where the DSP eventually drifts INTO bad
|
||||
* territory that triggers the wedge). Drain cycles here and
|
||||
* let the runtime watchdog (src/core/crash_detect.c) log the
|
||||
* actual escape PC. DSP_RUNNING is left alone so games that
|
||||
* legitimately stop the DSP via DSPGO=0 are unaffected.
|
||||
*
|
||||
* Valid execution regions match JaguarReadX address decoding
|
||||
* (src/core/jaguar.c): anything <= $E3FFFF (main RAM mirrored
|
||||
* 4x for the bottom 8MB, cart ROM, boot ROM) plus DSP local
|
||||
* SRAM. Earlier versions of this check used `<= 0x1FFFFF`
|
||||
* and would have false-flagged DSP code running from a RAM
|
||||
* mirror at $200000-$7FFFFF or from cart ROM at $800000+.
|
||||
* Caught by Copilot review on PR #182. */
|
||||
if (!((dsp_pc <= 0x00E3FFFF) ||
|
||||
(dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000)))
|
||||
{
|
||||
cycles = 0;
|
||||
break;
|
||||
}
|
||||
|
||||
pcThis = dsp_pc;
|
||||
|
||||
#if DSP_UNTIMED_GDB
|
||||
/* GDB stub (issue #652): checked after the IRQ dispatch and the
|
||||
* PC-escape bail, so a breakpoint on an ISR's first instruction
|
||||
* still fires. GDBHalt() blocks in place (does not unwind this
|
||||
* loop) until the client continues, steps, or disconnects; the
|
||||
* client may disarm meanwhile, which the refresh below picks up. */
|
||||
if (gdbArmedSlice && GDBCheckPC(GDB_TGT_DSP, pcThis))
|
||||
{
|
||||
GDBHalt(GDB_TGT_DSP, GDB_STOP_BREAKPOINT, pcThis);
|
||||
gdbArmedSlice = gdbArmedDSP;
|
||||
}
|
||||
#endif
|
||||
|
||||
/* `- 1`: the fetch reads off and off + 1, so the last byte of
|
||||
* local RAM (only reachable at an odd PC) goes through
|
||||
* DSPReadWord(), which aligns, instead of reading past
|
||||
* dsp_ram_8 (cppcheck arrayIndexOutOfBoundsCond). */
|
||||
if (dsp_pc >= DSP_WORK_RAM_BASE && dsp_pc < DSP_WORK_RAM_BASE + 0x2000 - 1)
|
||||
{
|
||||
uint32_t off = dsp_pc - DSP_WORK_RAM_BASE;
|
||||
opcode = ((uint16_t)dsp_ram_8[off] << 8) | (uint16_t)dsp_ram_8[off + 1];
|
||||
}
|
||||
else
|
||||
opcode = DSPReadWord(dsp_pc, DSP);
|
||||
index = opcode >> 10;
|
||||
dsp_opcode_first_parameter = (opcode >> 5) & 0x1F;
|
||||
dsp_opcode_second_parameter = opcode & 0x1F;
|
||||
dsp_pc += 2;
|
||||
dsp_exec_opcode_count++;
|
||||
if (divArmed && pcThis == divPc
|
||||
&& &dsp_reg[dsp_opcode_second_parameter] == divArmed)
|
||||
{
|
||||
/* Issue #811: indexed store right behind a divide into its
|
||||
* data register -- the quotient has not been written back
|
||||
* yet, so the store sees the old contents. */
|
||||
uint32_t quot = *divArmed;
|
||||
|
||||
*divArmed = divOld;
|
||||
dsp_executeOpcode(index);
|
||||
*divArmed = quot;
|
||||
dsp_div_store_fixups++;
|
||||
divArmed = NULL;
|
||||
}
|
||||
else if (index == 21)
|
||||
{
|
||||
uint32_t *dst = &dsp_reg[dsp_opcode_second_parameter];
|
||||
uint32_t old = *dst;
|
||||
|
||||
dsp_executeOpcode(index);
|
||||
divArmed = DSPDivStorePeek(dst, &divPc);
|
||||
divOld = old;
|
||||
}
|
||||
else
|
||||
{
|
||||
dsp_executeOpcode(index);
|
||||
divArmed = NULL;
|
||||
}
|
||||
cycles -= dsp_opcode_cycles[index];
|
||||
|
||||
/* Issue #811: the previous instruction was an external load and
|
||||
* this one overwrote its target -- the load data lands last. */
|
||||
if (ldxArmed)
|
||||
{
|
||||
if (pcThis == ldxPc
|
||||
&& &dsp_reg[dsp_opcode_second_parameter] == ldxArmed)
|
||||
{
|
||||
*ldxArmed = ldxVal;
|
||||
dsp_load_wb_fixups++;
|
||||
}
|
||||
ldxArmed = NULL;
|
||||
}
|
||||
if (dsp_ldx_slot)
|
||||
{
|
||||
ldxArmed = DSPLoadOverwritePeek(&ldxVal, &ldxPc);
|
||||
dsp_ldx_slot = NULL;
|
||||
}
|
||||
|
||||
/* Idle-loop fast-forward (issue #569). A taken `jr` that landed
|
||||
* on or behind its own address, at most 8 words back, is the only
|
||||
* candidate; the unsigned difference rejects a not-taken jr
|
||||
* (pc = pcThis + 2) and every forward branch in one compare. */
|
||||
if (idleSkipActive && index == 53
|
||||
&& (uint32_t)(pcThis - dsp_pc) <= 14)
|
||||
cycles = DSPIdleLoopProbe(cycles, dsp_pc, pcThis);
|
||||
|
||||
/* Age out a D_FLAGS store once the instruction that was already
|
||||
* behind it in the pipeline has run (see DSPWriteLong, D_FLAGS
|
||||
* case). Retiring re-opens interrupt recognition and commits the
|
||||
* register-bank switch the store selected. */
|
||||
if (dspFlagsRetireDelay)
|
||||
{
|
||||
dspFlagsRetireDelay--;
|
||||
if (dspFlagsRetireDelay == 0)
|
||||
DSPUpdateRegisterBanks();
|
||||
}
|
||||
}
|
||||
|
||||
dsp_in_exec--;
|
||||
|
||||
VJP_LEAVE(VJP_DSP);
|
||||
}
|
||||
|
||||
#undef DSP_UNTIMED_NAME
|
||||
#undef DSP_UNTIMED_GDB
|
||||
Reference in new issue
Block a user