fix(dsp): D_FLAGS store retires a pipeline stage late (Wolfenstein 3D missing audio)

Wolfenstein 3D lost ALL audio -- not just music -- from frame 48 onward.

LAYER DIAGNOSIS
The DSP runs, but its I2S handler never returns, so it stops feeding
LTXD/RTXD.  test/tools/test_dsp_audio_diag before: DSP PC escape at frame
48 (PC=$0006EE), LTXD non-zero ratio 0.0%, bank 1 with 1/32 registers
initialised, audio callbacks stopping at frame 48, every sample silent
across a 1200-frame capture.

A DSP instruction trace ring (last 96 PCs, per-instruction active bank,
both register banks, dumped when the PC leaves DSP RAM) puts the fault in
the handler epilogue at

    F1B24E  store  r0,(r1)     ; r1 = $F1A100 -- writes D_FLAGS, clears IMASK
    F1B250  jump   (r3)        ; r3 = bank-0 return address $F1B080

with the main polling loop at $F1B080 running in bank 1 (REGPAGE set).
Because "IMASK set forces bank 0" (DSPUpdateRegisterBanks), clearing IMASK
in the store switched the live bank to 1 *inside* the store, so
`jump (r3)` read bank 1's uninitialised r3 == 0 and the DSP returned to PC
$000000.  Register dump at the escape: bank 0 r3 = $00F1B080 (correct),
bank 1 r3 = 0.

MODEL
docs/jtrm-gpu-dsp.md ("Pipeline") gives the RISC core four stages --
Decode, Read Operands, Compute, Write-back.  A D_FLAGS store's write-back
therefore lands a stage after the following instruction has latched its
source registers, and a stage after a branch has latched the target
register it needs to steer the fetch.  Three effects, all modelled here:

  - a `jump` in the slot behind the store reads its target register from
    the pre-store bank (dspPreStoreBank);
  - IMASK is not really clear until the store retires, so that same slot
    still runs masked and cannot be preempted.  DSPExec holds off its
    IMASKCleared dispatch, and DSPHandleIRQsNP re-checks because
    DSPSetIRQLine reaches it asynchronously and cannot see that hold-off.
    Without the second guard Wolf3D still died at frame 2553: an I2S IRQ
    landed between $F1B24E and $F1B250, the nested handler returned to
    $F1B250 a slot late, and `jump (r3)` read the post-store bank (PC
    $000005);
  - refilling the pipeline from a taken branch target lets an outstanding
    store -- including one the delay slot itself issued -- retire first,
    so jump/jr clear the delay at the redirect.

That last point is what Doom depends on: its epilogue puts the D_FLAGS
store in the delay slot of `jump (r15)` at $F1B028, and the branch target
at $F1B1FC is a `jump (r25)` that must read the NEW bank.  Both shipped
epilogue shapes now work.  The bank pointer still swaps immediately, so
nothing outside the two-slot window changes.

The branch-refill part of the derivation is inference, not spec: no
docs/jtrm-*.md text describes it and dsp_opcode_cycles[] charges jump/jr
1 cycle.  It is the only composite rule satisfying both epilogue shapes,
with two independent code bases as the evidence.  The in-flight delay is
not in the save-state format (two instruction slots wide); DSPStateLoad
retires it and re-points dspPreStoreBank, so STATE_VERSION is unchanged.

EVIDENCE (stock 0b15377 vs this commit, same host)

Target title, 15 s headless capture at 48 kHz stereo:
  Wolfenstein 3D   non-silent 0.0% -> 54.5%, peak 0 -> 14858,
                   sustained 0/15 -> 11/15 s
                   dsp-diag 3 fail -> 6 pass; LTXD 0.0% -> 82.0%;
                   bank1 init 1/32 -> 28/32
                   clean over 5400 frames (90 s) x2 runs, no PC escape
  PCM character (mono, 2048-pt Welch), vs Tempest 2000 as the known-good
  music control and Doom as the SFX-only control:
                   spectral flatness  Wolf3D 0.0015 | T2K 0.0012 | Doom 0.0037
                   top-12-peak energy Wolf3D 0.74   | T2K 0.89   | Doom 0.26
                   median pitch jump  Wolf3D 0.00st | T2K 1.33st | Doom 3.60st
  In-level capture (scripted input into level 1, HUD visible):
                   18 s at RMS 4324, 43/45 s sustained, flatness 0.0015,
                   envelope autocorrelation peak 0.42 at a 2.68 s lag, and
                   a recurring 375/398 Hz fundamental with 1031/1055 Hz
                   harmonics across non-adjacent seconds -- pitched,
                   repeating content, i.e. music, not broadband SFX.

No-change titles -- byte-identical WAV captures: Doom, Tempest 2000,
Alien vs Predator, Iron Soldier.  AvP is unchanged, so this does NOT
explain its separate slow/wrong-pitch bug.

40-title cartridge A/B digest sweep (framebuffer hash + non-silent sample
count, 1200 frames each): 39/40 identical, 1 differing -- Wolfenstein 3D,
ns 0 -> 840392.  (A 41st ROM, Brutal Sports Football, is rejected by the
content loader on both builds; pre-existing, unrelated.)

CD titles (share the DSP/DAC path), cd_visual_verify in both HLE and real-
BIOS modes, 1500 frames: Myst hle RMS 2698 / bios 2548, Battle Morph
1379/1747, Iron Soldier 2 1683/1913, Primal Rage 859/980, Baldies 0/1020
-- every figure identical to stock, matching the documented baselines.
CD boot harnesses at 3000 frames, per title x mode, stock vs this: Battle
Morph and BrainDead 13 identical in both modes (same stage, same
final_pc); Dragon's Lair hle identical.  Dragon's Lair BIOS mode differed
in one run -- final_pc $004C12 (BIOS service band $0-$3FFF) vs $009462
(game-owned RAM), which reads as a forward move -- but n=1 per side and
that row oscillates between BIOS_INTRO and "? (BIOS service band)" across
historical sections of docs/cd-boot-matrix.md, so treat it as unconfirmed
rather than a second win.  No backward moves anywhere.  Highlander, Hover
Strike and Space Ace were not reached: test/roms/private disappeared from
the host part-way through the sweep, so those three rows are unverified.

The `make TEST_EXPORTS=1 test` exit 0 above was captured at a
code-identical parent (only later delta is this comment block); it cannot
be re-run until the ROM set is restored.

Suite: `make TEST_EXPORTS=1 test` exits 0.  test_audio_clipping passes
(Skyhammer window RMS 3079.8, Iron Soldier 2 2599.3, 0.000% saturated in
both); test_audio_presence Iron Soldier 1 window RMS 1175.7, inside its
documented [200, 25000] band.  c89-lint clean.  Benchmark unchanged within
noise -- this host is contended, and interleaved A/B runs put the fixed
build at or above stock (yarc mean 151.0 vs 141.6 fps, Tempest 2000 125.0
vs 110.8 fps, n=4 each).

NOT A DEFECT: Doom's own "no music" report.  Its embedded IWAD (1855
lumps) carries no D_* music lump and no MUS/SNG lump at all -- only the
DS_START/DS_END sound markers -- so the Jaguar build ships with no music
data to play, and its measured audio is SFX (flatness 0.0037, top-12
energy 0.26, erratic 3.60-semitone pitch jumps).  Wolf3D's IWAD by
contrast carries 13 song lumps between SONGS and SONGEND (TIPTOEF,
SEARCHF, SUSPENF, TOWERF, FUNKIEF, NAZIRPF, JAZNAZF, DEATHF, GETTHEF,
GETYOUF, GOINAFF, ULTIMAF, EVILF), which is what this fix restores.

Headless RMS cannot distinguish music from structured noise at the right
level, so a human RetroArch listen on Wolfenstein 3D is still required.

Co-Authored-By: Claude Fable 5 <[email protected]>

src/jerry/dsp.c

@@ -306,6 +306,18 @@ static uint32_t dspgo_poll_count;
 static uint32_t dsp_in_exec = 0;
 static uint32_t dsp_releaseTimeSlice_flag = 0;
 
+/* Instruction slots a DSP-issued D_FLAGS store waits out before it
+ * retires: the slot holding the store itself, plus the one instruction
+ * already past Read Operands behind it in the pipeline.  While the delay
+ * is outstanding a cleared IMASK cannot yet let an interrupt in, and a
+ * `jump` still reads its target register from dspPreStoreBank.  See the
+ * D_FLAGS case in DSPWriteLong for the hardware citation. */
+#define DSP_FLAGS_RETIRE_DELAY 2
+static uint32_t dspFlagsRetireDelay = 0;
+static uint32_t * dspPreStoreBank = NULL;
+
+
+
 // Private function prototypes
 
 void FlushDSPPipeline(void);
@@ -672,12 +684,70 @@ void DSPWriteLong(uint32_t offset, uint32_t data, uint32_t who/*=UNKNOWN*/)
       {
          case 0x00:
             {
+               uint32_t * preWriteBank = dsp_reg;
                IMASKCleared = (dsp_flags & IMASK) && !(data & IMASK);
                dsp_flags = (data & ~IMASK) | ((data & IMASK) ? (dsp_flags & IMASK) : 0);
                dsp_flag_z = dsp_flags & 0x01;
                dsp_flag_c = (dsp_flags >> 1) & 0x01;
                dsp_flag_n = (dsp_flags >> 2) & 0x01;
                DSPUpdateRegisterBanks();
+               /* A D_FLAGS store issued by the DSP itself does not retire
+                * until the instruction behind it is already past Read
+                * Operands.  JTRM (docs/jtrm-gpu-dsp.md, "Pipeline") gives
+                * the RISC core four stages -- Decode, Read Operands,
+                * Compute, Write-back -- so this store's write-back lands
+                * a stage after the next instruction has latched its source
+                * registers, and a whole stage after a branch has latched
+                * the target register it needs to steer the fetch.  Two
+                * consequences the register-bank pointer alone cannot
+                * express, both recorded here and honoured by
+                * dsp_opcode_jump and DSPExec:
+                *
+                *  - A `jump (Rn)` in the slot behind the store reads Rn
+                *    from the PRE-store bank.  This is the ISR epilogue
+                *    Wolfenstein 3D's I2S handler uses at $F1B24E:
+                *        store  r0,(r1)     ; r1 = $F1A100, clears IMASK
+                *        jump   (r3)        ; r3 = bank-0 return address
+                *    with the main loop running in bank 1 (REGPAGE set).
+                *    Swapping banks inside the store made `jump (r3)` read
+                *    bank 1's uninitialised r3 == 0, so the DSP returned to
+                *    PC $000000, left RAM and stopped feeding LTXD/RTXD --
+                *    the game lost all audio from frame 48 on.
+                *
+                *  - IMASK is not really clear until the store retires
+                *    either, so the instruction in that same slot still
+                *    runs masked and cannot be preempted.  Wolf3D's outer
+                *    epilogue at $F1B128-$F1B12A has the same store/jump
+                *    shape; letting an I2S interrupt in between the two
+                *    made the handler push the store's own address as the
+                *    return address, so `jump (r4)` ran a slot later than
+                *    it should have and read the wrong bank anyway.
+                *
+                * The bank pointer itself still swaps immediately, which is
+                * what Doom's epilogue needs: it puts the D_FLAGS store in
+                * the delay slot of `jump (r15)` at $F1B028, so the branch
+                * target at $F1B6B2 -- a `movei` whose write-back follows
+                * the store's -- must land in the NEW bank.
+                *
+                * CAVEAT on the derivation: the four-stage pipeline is
+                * documented, but nothing in docs/jtrm-*.md describes the
+                * taken-branch refill cost that lets a delay-slot store
+                * retire before the branch target reads registers, and this
+                * emulator's own dsp_opcode_cycles[] charges jump/jr only 1
+                * cycle.  The rule above is inferred from behaviour: it is
+                * the only composite that satisfies BOTH shipped epilogue
+                * shapes -- Wolf3D's `store; jump (Rn)` needs the old bank
+                * at the jump, Doom's `jump (Rn); store` needs the new bank
+                * at the target -- with two independent code bases as the
+                * evidence.  Revisit if primary JTRM pipeline timing ever
+                * contradicts it. */
+               if (who == DSP)
+               {
+                  dspPreStoreBank = preWriteBank;
+                  dspFlagsRetireDelay = DSP_FLAGS_RETIRE_DELAY;
+               }
+               else
+                  dspFlagsRetireDelay = 0;
                dsp_control &= ~((dsp_flags & CINT04FLAGS) >> 3);
                dsp_control &= ~((dsp_flags & CINT5FLAG) >> 1);
                break;
@@ -774,6 +844,21 @@ void DSPHandleIRQsNP(void)
 	if (dsp_flags & IMASK) 							// Bail if we're already inside an interrupt
 		return;
 
+	/* IMASK reads clear the moment the D_FLAGS store is decoded, but the
+	 * store has not retired yet -- the instruction behind it is already
+	 * past Read Operands and still runs masked (see DSPWriteLong).  This
+	 * entry point is also reached asynchronously from DSPSetIRQLine, which
+	 * cannot see DSPExec's own hold-off, so re-check here.  Leave the
+	 * latch standing in dsp_control and IMASKCleared unconsumed: DSPExec
+	 * dispatches it once the delay expires, at most two slots later.
+	 *
+	 * Without this, Wolfenstein 3D's I2S handler was re-entered between
+	 * `store r0,(r1)` at $F1B24E and `jump (r3)` at $F1B250, so the nested
+	 * handler returned to $F1B250 a slot late and `jump (r3)` read the
+	 * post-store bank -- PC $000005, DSP off the rails at frame 2553. */
+	if (dspFlagsRetireDelay && IMASKCleared)
+		return;
+
 	// Get the active interrupt bits (latches) & interrupt mask (enables)
 	// INT_LAT5 is at dsp_control bit 16 (non-contiguous with LAT0-4 at bits 6-10)
 	bits = ((dsp_control >> 11) & 0x20) | ((dsp_control >> 6) & 0x1F);
@@ -798,7 +883,12 @@ void DSPHandleIRQsNP(void)
 		which = 5;
 
 	dsp_flags |= IMASK;		// Force Bank #0
+	/* Taking an interrupt flushes the pipeline, so an un-retired D_FLAGS
+	 * store from before the vector fetch has no in-flight successor left
+	 * to shadow -- retire it rather than let it reach into the handler. */
+	dspFlagsRetireDelay = 0;
 	DSPUpdateRegisterBanks();
+	dspPreStoreBank = dsp_reg;
 
 
 	dsp_reg[31] -= 4;
@@ -870,6 +960,8 @@ void DSPReset(void)
 
 	CLR_ZNC;
 	IMASKCleared = false;
+	dspFlagsRetireDelay = 0;
+	dspPreStoreBank = dsp_reg;
 	FlushDSPPipeline();
 	dsp_reset_stats();
 
@@ -913,9 +1005,13 @@ void DSPExec(int32_t cycles)
       uint16_t opcode;
       uint32_t index;
 
-		if (IMASKCleared)						// If IMASK was cleared,
+		/* 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();					// See if any other interrupts are pending!
+			DSPHandleIRQsNP();
 			IMASKCleared = false;
 		}
 
@@ -966,6 +1062,13 @@ void DSPExec(int32_t cycles)
 		dsp_pc += 2;
 		dsp_executeOpcode(index);
 		cycles -= dsp_opcode_cycles[index];
+
+		/* 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 stops
+		 * dsp_opcode_jump reaching for the pre-store bank. */
+		if (dspFlagsRetireDelay)
+			dspFlagsRetireDelay--;
 	}
 
 	dsp_in_exec--;
@@ -1166,7 +1269,14 @@ INLINE static void dsp_opcode_jump(void)
 
 	if (BRANCH_CONDITION(IMM_2))
 	{
-		uint32_t delayed_pc = RM;
+		/* The target register is latched a pipeline stage before a
+		 * D_FLAGS store in the slot ahead of us can retire, so when one
+		 * is still outstanding read it from the bank that was live when
+		 * that store issued -- not from the bank it selected.  See the
+		 * D_FLAGS case in DSPWriteLong. */
+		uint32_t delayed_pc = dspFlagsRetireDelay
+		                      ? dspPreStoreBank[dsp_opcode_first_parameter]
+		                      : RM;
 		uint16_t ds_opcode;
 		uint32_t ds_index;
 		/* Inline delay-slot: fetch-decode-execute one instruction at current
@@ -1186,6 +1296,11 @@ INLINE static void dsp_opcode_jump(void)
 		dsp_pc += 2;
 		dsp_executeOpcode(ds_index);
 		dsp_pc = delayed_pc;
+		/* Refilling the pipeline from the branch target costs enough
+		 * cycles that an outstanding D_FLAGS store -- including one the
+		 * delay slot just issued -- reaches Write-back first, so the
+		 * target instruction sees the new bank.  See DSPWriteLong. */
+		dspFlagsRetireDelay = 0;
 	}
 }
 
@@ -1217,6 +1332,8 @@ INLINE static void dsp_opcode_jr(void)
 		dsp_pc += 2;
 		dsp_executeOpcode(ds_index);
 		dsp_pc = delayed_pc;
+		/* Same branch-target pipeline refill as dsp_opcode_jump. */
+		dspFlagsRetireDelay = 0;
 	}
 }
 
@@ -2970,6 +3087,12 @@ size_t DSPStateLoad(const uint8_t *buf)
       dsp_reg = dsp_reg_bank_1;
       dsp_alternate_reg = dsp_reg_bank_0;
    }
+   /* The state format carries no slot for an un-retired D_FLAGS store
+    * (see DSP_FLAGS_RETIRE_DELAY); retire it on load.  The window is two
+    * instruction slots wide, and the register banks themselves are saved
+    * in full either way. */
+   dspFlagsRetireDelay = 0;
+   dspPreStoreBank = dsp_reg;
 
    STATE_LOAD_VAR(buf, dsp_opcode_first_parameter);
    STATE_LOAD_VAR(buf, dsp_opcode_second_parameter);