From 75544fa9db8a1e6cf7ec228afca2c75f766afad0 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 1 May 2026 21:01:03 +0200 Subject: [PATCH] fix(cpu): PPCBUG-046 PPCBUG-561 add mb_md() accessor; fix all 6 rld* mb fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PPCBUG-561: Add DecodedInstr::mb_md() to decoder.rs — the correct MD-form 6-bit mask-begin reconstruction (MB[4:0] at PPC bits 21-25, MB[5] at PPC bit 26). The disassembler already had the correct local formula; this promotes it to a single source of truth on DecodedInstr. PPCBUG-046: All 6 doubleword-rotate arms (rldicl, rldicr, rldic, rldimi, rldcl, rldcr) inlined "(instr.mb() << 1) | ((instr.raw >> 1) & 1)" which reads SH5 (host bit 1) instead of MB5 (host bit 5). For the canonical "clrldi r3, r4, 32" zero-extend idiom (mb=32 → MB5=1, MB[4:0]=0), the wrong formula produced mb=0, making the instruction a no-op and leaving upper 32 bits of the GPR polluted. Replace all 6 sites with instr.mb_md(). Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/xenia-cpu/src/decoder.rs | 6 ++++ crates/xenia-cpu/src/interpreter.rs | 56 +++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/crates/xenia-cpu/src/decoder.rs b/crates/xenia-cpu/src/decoder.rs index 28e55dc..80a5a7e 100644 --- a/crates/xenia-cpu/src/decoder.rs +++ b/crates/xenia-cpu/src/decoder.rs @@ -92,6 +92,12 @@ impl DecodedInstr { (extract_bits(self.raw, 30, 30) << 5) | extract_bits(self.raw, 16, 20) } + /// MB/ME field for MD-form and MDS-form instructions (6-bit field, split encoding). + /// MB[4:0] at PPC bits 21-25; MB[5] at PPC bit 26. + #[inline] pub fn mb_md(&self) -> u32 { + extract_bits(self.raw, 21, 25) | (extract_bits(self.raw, 26, 26) << 5) + } + /// SPR field (bits 11-20, swapped halves) #[inline] pub fn spr(&self) -> u32 { let spr_raw = extract_bits(self.raw, 11, 20); diff --git a/crates/xenia-cpu/src/interpreter.rs b/crates/xenia-cpu/src/interpreter.rs index c22cd0b..90177f2 100644 --- a/crates/xenia-cpu/src/interpreter.rs +++ b/crates/xenia-cpu/src/interpreter.rs @@ -693,7 +693,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::rldiclx => { let rs = ctx.gpr[instr.rs()]; let sh = instr.sh64(); - let mb = (instr.mb() << 1) | ((instr.raw >> 1) & 1); // 6-bit mb + let mb = instr.mb_md(); let rotated = rs.rotate_left(sh); let mask = rld_mask_left(mb); ctx.gpr[instr.ra()] = rotated & mask; @@ -703,7 +703,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::rldicrx => { let rs = ctx.gpr[instr.rs()]; let sh = instr.sh64(); - let me = (instr.mb() << 1) | ((instr.raw >> 1) & 1); // 6-bit me + let me = instr.mb_md(); let rotated = rs.rotate_left(sh); let mask = rld_mask_right(me); ctx.gpr[instr.ra()] = rotated & mask; @@ -713,7 +713,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::rldicx => { let rs = ctx.gpr[instr.rs()]; let sh = instr.sh64(); - let mb = (instr.mb() << 1) | ((instr.raw >> 1) & 1); + let mb = instr.mb_md(); let rotated = rs.rotate_left(sh); let mask = rld_mask_left(mb) & rld_mask_right(63 - sh); ctx.gpr[instr.ra()] = rotated & mask; @@ -723,7 +723,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::rldimix => { let rs = ctx.gpr[instr.rs()]; let sh = instr.sh64(); - let mb = (instr.mb() << 1) | ((instr.raw >> 1) & 1); + let mb = instr.mb_md(); let rotated = rs.rotate_left(sh); let mask = rld_mask_left(mb) & rld_mask_right(63 - sh); ctx.gpr[instr.ra()] = (rotated & mask) | (ctx.gpr[instr.ra()] & !mask); @@ -733,7 +733,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::rldclx => { let rs = ctx.gpr[instr.rs()]; let sh = ctx.gpr[instr.rb()] & 0x3F; - let mb = (instr.mb() << 1) | ((instr.raw >> 1) & 1); + let mb = instr.mb_md(); let rotated = rs.rotate_left(sh as u32); let mask = rld_mask_left(mb); ctx.gpr[instr.ra()] = rotated & mask; @@ -743,7 +743,7 @@ fn execute(ctx: &mut PpcContext, mem: &dyn MemoryAccess, instr: &DecodedInstr) - PpcOpcode::rldcrx => { let rs = ctx.gpr[instr.rs()]; let sh = ctx.gpr[instr.rb()] & 0x3F; - let me = (instr.mb() << 1) | ((instr.raw >> 1) & 1); + let me = instr.mb_md(); let rotated = rs.rotate_left(sh as u32); let mask = rld_mask_right(me); ctx.gpr[instr.ra()] = rotated & mask; @@ -6252,4 +6252,48 @@ mod tests { expected[4] = 0xAB; assert_eq!(ctx.vr[3].as_bytes(), expected); } + + // ===== PPCBUG-046 / PPCBUG-561: rldicl / clrldi mb_md fix ===== + + /// Encode rldicl (MD-form, opcode=30, XO=0) in host bit notation. + /// rs: source register, ra: dest register, sh: shift amount (6-bit), + /// mb: mask-begin (6-bit), rc: record bit. + fn encode_rldicl(rs: u32, ra: u32, sh: u32, mb: u32, rc: u32) -> u32 { + (30 << 26) + | (rs << 21) + | (ra << 16) + | ((sh & 0x1F) << 11) + | ((mb & 0x1F) << 6) + | (((mb >> 5) & 1) << 5) + | (((sh >> 5) & 1) << 1) + | (rc & 1) + } + + #[test] + fn clrldi_zero_extends_low_32_bits() { + // clrldi r3, r4, 32 = rldicl r3, r4, 0, 32, 0 + // After PPCBUG-046 fix: mask must be 0x00000000_FFFFFFFF (mb=32 → mask from bit 32 to 63) + // If mb=32 was decoded as mb=0, the mask would be all-ones and the result would be 0xDEAD_BEEF_CAFE_BABE (no-op) + let mut ctx = PpcContext::new(); + let mem = TestMem::new(); + ctx.gpr[4] = 0xDEAD_BEEF_CAFE_BABE_u64; + let raw = encode_rldicl(4, 3, 0, 32, 0); // sh=0, mb=32 + write_instr(&mem, 0x100, raw); + ctx.pc = 0x100; + step(&mut ctx, &mem); + assert_eq!(ctx.gpr[3], 0x0000_0000_CAFE_BABE, "clrldi must zero-extend low 32 bits"); + } + + #[test] + fn rldicl_mb32_leaves_low_32_clean() { + // Same as above but verify upper 32 are zeroed + let mut ctx = PpcContext::new(); + let mem = TestMem::new(); + ctx.gpr[5] = 0xFFFF_FFFF_1234_5678_u64; + let raw = encode_rldicl(5, 6, 0, 32, 0); + write_instr(&mem, 0x100, raw); + ctx.pc = 0x100; + step(&mut ctx, &mem); + assert_eq!(ctx.gpr[6], 0x0000_0000_1234_5678_u64); + } }