Merge pull request #10038 from Pokechu22/dsp-rti-srs

DSPLLE: Split SRS into SRS and SRSH and implement conditional variants of RTI

Source/Core/Core/DSP/DSPCore.cpp

@@ -33,25 +33,28 @@ static bool VerifyRoms(const SDSP& dsp)
     u32 hash_drom;  // dsp_coef.bin
   };
 
-  static const std::array<DspRomHashes, 6> known_roms = {{
+  static const std::array<DspRomHashes, 7> known_roms = {{
       // Official Nintendo ROM
       {0x66f334fe, 0xf3b93527},
 
-      // LM1234 replacement ROM (Zelda UCode only)
+      // v0.1: LM1234 replacement ROM (Zelda UCode only)
       {0x9c8f593c, 0x10000001},
 
-      // delroth's improvement on LM1234 replacement ROM (Zelda and AX only,
+      // v0.2: delroth's improvement on LM1234 replacement ROM (Zelda and AX only,
       // IPL/Card/GBA still broken)
       {0xd9907f71, 0xb019c2fb},
 
-      // above with improved resampling coefficients
+      // v0.2.1: above with improved resampling coefficients
       {0xd9907f71, 0xdb6880c1},
 
-      // above with support for GBA ucode
+      // v0.3: above with support for GBA ucode
       {0x3aa4a793, 0xa4a575f5},
 
-      // above with fix to skip bootucode_ax when running from ROM entrypoint
+      // v0.3.1: above with fix to skip bootucode_ax when running from ROM entrypoint
       {0x128ea7a2, 0xa4a575f5},
+
+      // v0.4: above with fixes for invalid use of SRS instruction
+      {0xe789b5a5, 0xa4a575f5},
   }};
 
   const u32 hash_irom =
@@ -69,28 +72,30 @@ static bool VerifyRoms(const SDSP& dsp)
 
   if (rom_idx < 0)
   {
-    if (AskYesNoFmtT("Your DSP ROMs have incorrect hashes.\n"
+    if (AskYesNoFmtT("Your DSP ROMs have incorrect hashes.\n\n"
+                     "Delete the dsp_rom.bin and dsp_coef.bin files in the GC folder in the Global "
+                     "User Directory to use the free DSP ROM, or replace them with good dumps from "
+                     "a real GameCube/Wii.\n\n"
                      "Would you like to stop now to fix the problem?\n"
                      "If you select \"No\", audio might be garbled."))
+    {
       return false;
+    }
   }
 
-  if (rom_idx == 1)
-  {
-    Host::OSD_AddMessage("You are using an old free DSP ROM made by the Dolphin Team.", 6000);
-    Host::OSD_AddMessage("Only games using the Zelda UCode will work correctly.", 6000);
-  }
-  else if (rom_idx == 2 || rom_idx == 3)
+  if (rom_idx >= 1 && rom_idx <= 5)
   {
-    Host::OSD_AddMessage("You are using a free DSP ROM made by the Dolphin Team.", 8000);
-    Host::OSD_AddMessage("All Wii games will work correctly, and most GameCube games", 8000);
-    Host::OSD_AddMessage("should also work fine, but the GBA/CARD UCodes will not work.", 8000);
-  }
-  else if (rom_idx == 4)
-  {
-    Host::OSD_AddMessage("You are using a free DSP ROM made by the Dolphin Team.", 8000);
-    Host::OSD_AddMessage("All Wii games will work correctly, and most GameCube games", 8000);
-    Host::OSD_AddMessage("should also work fine, but the CARD UCode will not work.", 8000);
+    if (AskYesNoFmtT(
+            "You are using an old free DSP ROM made by the Dolphin Team.\n"
+            "Due to emulation accuracy improvements, this ROM no longer works correctly.\n\n"
+            "Delete the dsp_rom.bin and dsp_coef.bin files in the GC folder in the Global "
+            "User Directory to use the most recent free DSP ROM, or replace them with "
+            "good dumps from a real GameCube/Wii.\n\n"
+            "Would you like to stop now to fix the problem?\n"
+            "If you select \"No\", audio might be garbled."))
+    {
+      return false;
+    }
   }
 
   return true;

Source/Core/Core/DSP/DSPTables.cpp

@@ -18,7 +18,7 @@
 namespace DSP
 {
 // clang-format off
-const std::array<DSPOPCTemplate, 214> s_opcodes =
+const std::array<DSPOPCTemplate, 230> s_opcodes =
 {{
   //              # of parameters----+   {type, size, loc, lshift, mask}                                                               branch        reads PC       // instruction approximation
   // name      opcode  mask  size-V  V   param 1                       param 2                       param 3                    extendable    uncond.       updates SR
@@ -48,7 +48,22 @@ const std::array<DSPOPCTemplate, 214> s_opcodes =
   {"RETO",     0x02de, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return if overflow
   {"RET",      0x02df, 0xffff,    1, 0, {},                                                                                     false, true, true, false, false}, // unconditional return
 
-  {"RTI",      0x02ff, 0xffff,    1, 0, {},                                                                                     false, true, true, false, false}, // return from interrupt
+  {"RTIGE",    0x02f0, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if greater or equal
+  {"RTIL",     0x02f1, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if less
+  {"RTIG",     0x02f2, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if greater
+  {"RTILE",    0x02f3, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if less or equal
+  {"RTINZ",    0x02f4, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if not zero
+  {"RTIZ",     0x02f5, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if zero
+  {"RTINC",    0x02f6, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if not carry
+  {"RTIC",     0x02f7, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if carry
+  {"RTIx8",    0x02f8, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if TODO
+  {"RTIx9",    0x02f9, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if TODO
+  {"RTIxA",    0x02fa, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if TODO
+  {"RTIxB",    0x02fb, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if TODO
+  {"RTILNZ",   0x02fc, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if logic not zero
+  {"RTILZ",    0x02fd, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if logic zero
+  {"RTIO",     0x02fe, 0xffff,    1, 0, {},                                                                                     false, true, false, true, false}, // return from interrupt if overflow
+  {"RTI",      0x02ff, 0xffff,    1, 0, {},                                                                                     false, true, true, false, false}, // return from interrupt unconditionally
 
   {"CALLGE",   0x02b0, 0xffff,    2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}},                                                          false, true, false, true, false}, // call if greater or equal
   {"CALLL",    0x02b1, 0xffff,    2, 1, {{P_ADDR_I, 2, 1, 0, 0xffff}},                                                          false, true, false, true, false}, // call if less
@@ -192,7 +207,8 @@ const std::array<DSPOPCTemplate, 214> s_opcodes =
 
   //2
   {"LRS",      0x2000, 0xf800,    1, 2, {{P_REG18, 1, 0, 8, 0x0700},   {P_MEM, 1, 0, 0, 0x00ff}},                               false, false, false, false, false}, // $(D+24) = MEM[($cr[0-7] << 8) | I]
-  {"SRS",      0x2800, 0xf800,    1, 2, {{P_MEM,   1, 0, 0, 0x00ff},   {P_REG18, 1, 0, 8, 0x0700}},                             false, false, false, false, false}, // MEM[($cr[0-7] << 8) | I] = $(S+24)
+  {"SRSH",     0x2800, 0xfe00,    1, 2, {{P_MEM,   1, 0, 0, 0x00ff},   {P_ACCH, 1, 0, 8, 0x0100}},                              false, false, false, false, false}, // MEM[($cr[0-7] << 8) | I] = $acS.h
+  {"SRS",      0x2c00, 0xfc00,    1, 2, {{P_MEM,   1, 0, 0, 0x00ff},   {P_REG1C, 1, 0, 8, 0x0300}},                             false, false, false, false, false}, // MEM[($cr[0-7] << 8) | I] = $(S+24)
 
   // opcodes that can be extended
 

Source/Core/Core/DSP/DSPTables.h

@@ -44,16 +44,13 @@ enum partype_t
   P_ACCM = P_REG | 0x1e00,   // used for mid part of accum
   // The following are not in gcdsptool
   P_ACCM_D = P_REG | 0x1e80,
-  P_ACC = P_REG | 0x2000,  // used for full accum.
+  P_ACC = P_REG | 0x2000,   // used for full accum.
+  P_ACCH = P_REG | 0x1000,  // used for high part of accum
   P_ACC_D = P_REG | 0x2080,
   P_AX = P_REG | 0x2200,
   P_REGS_MASK = 0x03f80,  // gcdsptool's value = 0x01f80
   P_REF = P_REG | 0x4000,
   P_PRG = P_REF | P_REG,
-
-  // The following seem like junk:
-  // P_REG10     = P_REG | 0x1000,
-  // P_AX_D      = P_REG | 0x2280,
 };
 
 struct param2_t

Source/Core/Core/DSP/Interpreter/DSPIntBranch.cpp

@@ -104,13 +104,17 @@ void Interpreter::ret(const UDSPInstruction opc)
   state.pc = state.PopStack(StackRegister::Call);
 }
 
-// RTI
+// RTIcc
 // 0000 0010 1111 1111
 // Return from exception. Pops stored status register $sr from data stack
 // $st1 and program counter PC from call stack $st0 and sets $pc to this
 // location.
-void Interpreter::rti(const UDSPInstruction)
+// This instruction has a conditional form, but it is not used by any official ucode.
+void Interpreter::rti(const UDSPInstruction opc)
 {
+  if (!CheckCondition(opc & 0xf))
+    return;
+
   auto& state = m_dsp_core.DSPState();
   state.r.sr = state.PopStack(StackRegister::Data);
   state.pc = state.PopStack(StackRegister::Call);

Source/Core/Core/DSP/Interpreter/DSPIntLoadStore.cpp

@@ -8,15 +8,29 @@
 
 namespace DSP::Interpreter
 {
-// SRS @M, $(0x18+S)
-// 0010 1sss mmmm mmmm
-// Move value from register $(0x18+S) to data memory pointed by address
+// SRSH @M, $acS.h
+// 0010 10ss mmmm mmmm
+// Move value from register $acS.h to data memory pointed by address
+// CR[0-7] | M. That is, the upper 8 bits of the address are the
+// bottom 8 bits from CR, and the lower 8 bits are from the 8-bit immediate.
+void Interpreter::srsh(const UDSPInstruction opc)
+{
+  auto& state = m_dsp_core.DSPState();
+  const auto reg = static_cast<u8>(((opc >> 8) & 0x1) + DSP_REG_ACH0);
+  const auto addr = static_cast<u16>((state.r.cr << 8) | (opc & 0xFF));
+
+  state.WriteDMEM(addr, OpReadRegister(reg));
+}
+
+// SRS @M, $(0x1C+S)
+// 0010 11ss mmmm mmmm
+// Move value from register $(0x1C+S) to data memory pointed by address
 // CR[0-7] | M. That is, the upper 8 bits of the address are the
 // bottom 8 bits from CR, and the lower 8 bits are from the 8-bit immediate.
 void Interpreter::srs(const UDSPInstruction opc)
 {
   auto& state = m_dsp_core.DSPState();
-  const auto reg = static_cast<u8>(((opc >> 8) & 0x7) + 0x18);
+  const auto reg = static_cast<u8>(((opc >> 8) & 0x3) + DSP_REG_ACL0);
   const auto addr = static_cast<u16>((state.r.cr << 8) | (opc & 0xFF));
 
   if (reg >= DSP_REG_ACM0)

Source/Core/Core/DSP/Interpreter/DSPIntTables.cpp

@@ -19,7 +19,7 @@ struct InterpreterOpInfo
 };
 
 // clang-format off
-constexpr std::array<InterpreterOpInfo, 124> s_opcodes
+constexpr std::array<InterpreterOpInfo, 125> s_opcodes
 {{
   {0x0000, 0xfffc, &Interpreter::nop},
 
@@ -32,7 +32,7 @@ constexpr std::array<InterpreterOpInfo, 124> s_opcodes
 
   {0x02d0, 0xfff0, &Interpreter::ret},
 
-  {0x02ff, 0xffff, &Interpreter::rti},
+  {0x02f0, 0xfff0, &Interpreter::rti},
 
   {0x02b0, 0xfff0, &Interpreter::call},
 
@@ -101,7 +101,8 @@ constexpr std::array<InterpreterOpInfo, 124> s_opcodes
 
   // 2
   {0x2000, 0xf800, &Interpreter::lrs},
-  {0x2800, 0xf800, &Interpreter::srs},
+  {0x2800, 0xfe00, &Interpreter::srsh},
+  {0x2c00, 0xfc00, &Interpreter::srs},
 
   // opcodes that can be extended
 

Source/Core/Core/DSP/Interpreter/DSPInterpreter.h

@@ -149,6 +149,7 @@ class Interpreter
   void srri(UDSPInstruction opc);
   void srrn(UDSPInstruction opc);
   void srs(UDSPInstruction opc);
+  void srsh(UDSPInstruction opc);
   void sub(UDSPInstruction opc);
   void subarn(UDSPInstruction opc);
   void subax(UDSPInstruction opc);

Source/Core/Core/DSP/Jit/x64/DSPEmitter.h

@@ -88,6 +88,7 @@ class DSPEmitter final : public JIT::DSPEmitter, public Gen::X64CodeBlock
   void bloopi(UDSPInstruction opc);
 
   // Load/Store
+  void srsh(UDSPInstruction opc);
   void srs(UDSPInstruction opc);
   void lrs(UDSPInstruction opc);
   void lr(UDSPInstruction opc);
@@ -220,6 +221,7 @@ class DSPEmitter final : public JIT::DSPEmitter, public Gen::X64CodeBlock
   void r_callr(UDSPInstruction opc);
   void r_ifcc(UDSPInstruction opc);
   void r_ret(UDSPInstruction opc);
+  void r_rti(UDSPInstruction opc);
 
   void Update_SR_Register(Gen::X64Reg val = Gen::EAX, Gen::X64Reg scratch = Gen::EDX);
 

Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp

@@ -255,19 +255,28 @@ void DSPEmitter::ret(const UDSPInstruction opc)
   ReJitConditional(opc, &DSPEmitter::r_ret);
 }
 
-// RTI
-// 0000 0010 1111 1111
-// Return from exception. Pops stored status register $sr from data stack
-// $st1 and program counter PC from call stack $st0 and sets $pc to this
-// location.
-void DSPEmitter::rti(const UDSPInstruction opc)
+void DSPEmitter::r_rti(const UDSPInstruction opc)
 {
   //	g_dsp.r[DSP_REG_SR] = dsp_reg_load_stack(StackRegister::Data);
   dsp_reg_load_stack(StackRegister::Data);
   dsp_op_write_reg(DSP_REG_SR, RDX);
   //	g_dsp.pc = dsp_reg_load_stack(StackRegister::Call);
   dsp_reg_load_stack(StackRegister::Call);
   MOV(16, M_SDSP_pc(), R(DX));
+  WriteBranchExit();
+}
+
+// RTIcc
+// 0000 0010 1111 1111
+// Return from exception. Pops stored status register $sr from data stack
+// $st1 and program counter PC from call stack $st0 and sets $pc to this
+// location.
+// This instruction has a conditional form, but it is not used by any official ucode.
+// NOTE: Cannot use FallBackToInterpreter(opc) here because of the need to write branch exit
+void DSPEmitter::rti(const UDSPInstruction opc)
+{
+  MOV(16, M_SDSP_pc(), Imm16(m_compile_pc + 1));
+  ReJitConditional(opc, &DSPEmitter::r_rti);
 }
 
 // HALT

Source/Core/Core/DSP/Jit/x64/DSPJitLoadStore.cpp

@@ -12,14 +12,35 @@ using namespace Gen;
 
 namespace DSP::JIT::x64
 {
-// SRS @M, $(0x18+S)
+// SRSH @M, $acS.h
 // 0010 1sss mmmm mmmm
-// Move value from register $(0x18+S) to data memory pointed by address
+// Move value from register $acS.h to data memory pointed by address
+// CR[0-7] | M. That is, the upper 8 bits of the address are the
+// bottom 8 bits from CR, and the lower 8 bits are from the 8-bit immediate.
+void DSPEmitter::srsh(const UDSPInstruction opc)
+{
+  u8 reg = ((opc >> 8) & 0x1) + DSP_REG_ACH0;
+  // u16 addr = (g_dsp.r.cr << 8) | (opc & 0xFF);
+
+  X64Reg tmp1 = m_gpr.GetFreeXReg();
+
+  dsp_op_read_reg(reg, tmp1, RegisterExtension::Zero);
+  dsp_op_read_reg(DSP_REG_CR, RAX, RegisterExtension::Zero);
+  SHL(16, R(EAX), Imm8(8));
+  OR(16, R(EAX), Imm16(opc & 0xFF));
+  dmem_write(tmp1);
+
+  m_gpr.PutXReg(tmp1);
+}
+
+// SRS @M, $(0x1C+S)
+// 0010 1sss mmmm mmmm
+// Move value from register $(0x1C+S) to data memory pointed by address
 // CR[0-7] | M. That is, the upper 8 bits of the address are the
 // bottom 8 bits from CR, and the lower 8 bits are from the 8-bit immediate.
 void DSPEmitter::srs(const UDSPInstruction opc)
 {
-  u8 reg = ((opc >> 8) & 0x7) + 0x18;
+  u8 reg = ((opc >> 8) & 0x3) + DSP_REG_ACL0;
   // u16 addr = (g_dsp.r.cr << 8) | (opc & 0xFF);
 
   X64Reg tmp1 = m_gpr.GetFreeXReg();

Source/Core/Core/DSP/Jit/x64/DSPJitTables.cpp

@@ -19,7 +19,7 @@ struct JITOpInfo
 };
 
 // clang-format off
-const std::array<JITOpInfo, 124> s_opcodes =
+const std::array<JITOpInfo, 125> s_opcodes =
 {{
   {0x0000, 0xfffc, &DSPEmitter::nop},
 
@@ -32,7 +32,7 @@ const std::array<JITOpInfo, 124> s_opcodes =
 
   {0x02d0, 0xfff0, &DSPEmitter::ret},
 
-  {0x02ff, 0xffff, &DSPEmitter::rti},
+  {0x02f0, 0xfff0, &DSPEmitter::rti},
 
   {0x02b0, 0xfff0, &DSPEmitter::call},
 
@@ -101,7 +101,8 @@ const std::array<JITOpInfo, 124> s_opcodes =
 
   // 2
   {0x2000, 0xf800, &DSPEmitter::lrs},
-  {0x2800, 0xf800, &DSPEmitter::srs},
+  {0x2800, 0xfe00, &DSPEmitter::srsh},
+  {0x2c00, 0xfc00, &DSPEmitter::srs},
 
   // opcodes that can be extended
 

Source/DSPSpy/tests/accelerator_loop_test.ds

@@ -1,6 +1,8 @@
 incdir  "tests"
 include "dsp_base.inc"
 
+test_main:
+
 ; Test parameters
 lri $AC0.M, #0x0000 ; start
 lri $AC0.L, #0x0000 ; start

Source/DSPSpy/tests/accelerator_test.ds

@@ -2,7 +2,6 @@
 ; See https://github.com/dolphin-emu/dolphin/pull/5997
 incdir  "tests"
 include "dsp_base.inc"
-jmp test_main
 
 ; Writes the passed format, start and end addresses to the accelerator registers,
 ; then reads them back to registers.

Source/DSPSpy/tests/andc_ls_test.ds

@@ -5,7 +5,7 @@ include "dsp_base.inc"
 ;    
 ;    3d80      andc'ls     $AC1.M : $AX0.L, $AC0.M
 
-
+test_main:
 	clr $ACC0
 	clr $ACC1
 	lri $AX0.L, #0x0000
@@ -14,7 +14,7 @@ include "dsp_base.inc"
 
 	lri $AC0.M, #0x0004
 	lri $AC1.M, #0x1234
-	andc'ls $AC1.M : $AX0.L, $AC0.M
+	cw 0x3d80 ; andc'ls $AC1.M : $AX0.L, $AC0.M
 
 	call send_back
 

Source/DSPSpy/tests/arith_test.ds

@@ -2,6 +2,8 @@
 incdir  "tests"
 include "dsp_base.inc"
 
+test_main:
+
 clr $acc0
 tst $acc0