Merge pull request #9377 from lioncash/analyzer

DSPAnalyzer: Migrate off file-scope state

8 files changed, 153 insertions, 77 deletions

diff --git a/Source/Core/Core/DSP/DSPAnalyzer.cpp b/Source/Core/Core/DSP/DSPAnalyzer.cpp
index 7a88b99773..2b2d70e2e6 100644
--- a/Source/Core/Core/DSP/DSPAnalyzer.cpp
+++ b/Source/Core/Core/DSP/DSPAnalyzer.cpp
@@ -12,15 +12,8 @@
 #include "Core/DSP/DSPCore.h"
 #include "Core/DSP/DSPTables.h"
 
-namespace DSP::Analyzer
+namespace DSP
 {
-namespace
-{
-constexpr size_t ISPACE = 65536;
-
-// Holds data about all instructions in RAM.
-std::array<u8, ISPACE> code_flags;
-
 // Good candidates for idle skipping is mail wait loops. If we're time slicing
 // between the main CPU and the DSP, if the DSP runs into one of these, it might
 // as well give up its time slice immediately, after executing once.
@@ -65,18 +58,38 @@ constexpr u16 idle_skip_sigs[NUM_IDLE_SIGS][MAX_IDLE_SIG_SIZE + 1] = {
     {0x00da, 0x0352,  // LR     $AX0.H, @0x0352
      0x8600,          // TSTAXH $AX0.H
      0x0295, 0xFFFF,  // JZ    0x????
-     0, 0}};
+     0, 0},
+};
+
+Analyzer::Analyzer() = default;
+Analyzer::~Analyzer() = default;
 
-void Reset()
+void Analyzer::Analyze(const SDSP& dsp)
 {
-  code_flags.fill(0);
+  Reset();
+  AnalyzeRange(dsp, 0x0000, 0x1000);  // IRAM
+  AnalyzeRange(dsp, 0x8000, 0x9000);  // IROM
 }
 
-void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr)
+void Analyzer::Reset()
+{
+  m_code_flags.fill(0);
+}
+
+void Analyzer::AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr)
 {
   // First we run an extremely simplified version of a disassembler to find
   // where all instructions start.
+  FindInstructionStarts(dsp, start_addr, end_addr);
+
+  // Next, we'll scan for potential idle skips.
+  FindIdleSkips(dsp, start_addr, end_addr);
 
+  INFO_LOG_FMT(DSPLLE, "Finished analysis.");
+}
+
+void Analyzer::FindInstructionStarts(const SDSP& dsp, u16 start_addr, u16 end_addr)
+{
   // This may not be 100% accurate in case of jump tables!
   // It could get desynced, which would be bad. We'll see if that's an issue.
   u16 last_arithmetic = 0;
@@ -89,20 +102,20 @@ void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr)
       addr++;
       continue;
     }
-    code_flags[addr] |= CODE_START_OF_INST;
+    m_code_flags[addr] |= CODE_START_OF_INST;
     // Look for loops.
     if ((inst & 0xffe0) == 0x0060 || (inst & 0xff00) == 0x1100)
     {
       // BLOOP, BLOOPI
       const u16 loop_end = dsp.ReadIMEM(addr + 1);
-      code_flags[addr] |= CODE_LOOP_START;
-      code_flags[loop_end] |= CODE_LOOP_END;
+      m_code_flags[addr] |= CODE_LOOP_START;
+      m_code_flags[loop_end] |= CODE_LOOP_END;
     }
     else if ((inst & 0xffe0) == 0x0040 || (inst & 0xff00) == 0x1000)
     {
       // LOOP, LOOPI
-      code_flags[addr] |= CODE_LOOP_START;
-      code_flags[static_cast<u16>(addr + 1u)] |= CODE_LOOP_END;
+      m_code_flags[addr] |= CODE_LOOP_START;
+      m_code_flags[static_cast<u16>(addr + 1u)] |= CODE_LOOP_END;
     }
 
     // Mark the last arithmetic/multiplier instruction before a branch.
@@ -114,7 +127,7 @@ void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr)
 
     if (opcode->branch && !opcode->uncond_branch)
     {
-      code_flags[last_arithmetic] |= CODE_UPDATE_SR;
+      m_code_flags[last_arithmetic] |= CODE_UPDATE_SR;
     }
 
     // If an instruction potentially raises exceptions, mark the following
@@ -122,12 +135,16 @@ void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr)
     if (opcode->opcode == 0x00c0 || opcode->opcode == 0x1800 || opcode->opcode == 0x1880 ||
         opcode->opcode == 0x1900 || opcode->opcode == 0x1980 || opcode->opcode == 0x2000 ||
         opcode->extended)
-      code_flags[static_cast<u16>(addr + opcode->size)] |= CODE_CHECK_INT;
+    {
+      m_code_flags[static_cast<u16>(addr + opcode->size)] |= CODE_CHECK_EXC;
+    }
 
     addr += opcode->size;
   }
+}
 
-  // Next, we'll scan for potential idle skips.
+void Analyzer::FindIdleSkips(const SDSP& dsp, u16 start_addr, u16 end_addr)
+{
   for (size_t s = 0; s < NUM_IDLE_SIGS; s++)
   {
     for (u16 addr = start_addr; addr < end_addr; addr++)
@@ -145,24 +162,9 @@ void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr)
       if (found)
       {
         INFO_LOG_FMT(DSPLLE, "Idle skip location found at {:02x} (sigNum:{})", addr, s + 1);
-        code_flags[addr] |= CODE_IDLE_SKIP;
+        m_code_flags[addr] |= CODE_IDLE_SKIP;
       }
     }
   }
-  INFO_LOG_FMT(DSPLLE, "Finished analysis.");
-}
-}  // Anonymous namespace
-
-void Analyze(const SDSP& dsp)
-{
-  Reset();
-  AnalyzeRange(dsp, 0x0000, 0x1000);  // IRAM
-  AnalyzeRange(dsp, 0x8000, 0x9000);  // IROM
 }
-
-u8 GetCodeFlags(u16 address)
-{
-  return code_flags[address];
-}
-
-}  // namespace DSP::Analyzer
+}  // namespace DSP
diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h
index dab41baadc..84f1c1cd97 100644
--- a/Source/Core/Core/DSP/DSPAnalyzer.h
+++ b/Source/Core/Core/DSP/DSPAnalyzer.h
@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <array>
 #include "Common/CommonTypes.h"
 
 namespace DSP
@@ -11,31 +12,98 @@ namespace DSP
 struct SDSP;
 }
 
-// Basic code analysis.
-namespace DSP::Analyzer
+namespace DSP
 {
 // Useful things to detect:
 // * Loop endpoints - so that we can avoid checking for loops every cycle.
 
-enum
+class Analyzer
 {
-  CODE_START_OF_INST = 1,
-  CODE_IDLE_SKIP = 2,
-  CODE_LOOP_START = 4,
-  CODE_LOOP_END = 8,
-  CODE_UPDATE_SR = 16,
-  CODE_CHECK_INT = 32,
-};
+public:
+  explicit Analyzer();
+  ~Analyzer();
+
+  Analyzer(const Analyzer&) = default;
+  Analyzer& operator=(const Analyzer&) = default;
+
+  Analyzer(Analyzer&&) = default;
+  Analyzer& operator=(Analyzer&&) = default;
+
+  // This one should be called every time IRAM changes - which is basically
+  // every time that a new ucode gets uploaded, and never else. At that point,
+  // we can do as much static analysis as we want - but we should always throw
+  // all old analysis away. Luckily the entire address space is only 64K code
+  // words and the actual code space 8K instructions in total, so we can do
+  // some pretty expensive analysis if necessary.
+  void Analyze(const SDSP& dsp);
+
+  // Whether or not the given address indicates the start of an instruction.
+  [[nodiscard]] bool IsStartOfInstruction(u16 address) const
+  {
+    return (GetCodeFlags(address) & CODE_START_OF_INST) != 0;
+  }
+
+  // Whether or not the address indicates an idle skip location.
+  [[nodiscard]] bool IsIdleSkip(u16 address) const
+  {
+    return (GetCodeFlags(address) & CODE_IDLE_SKIP) != 0;
+  }
+
+  // Whether or not the address indicates the start of a loop.
+  [[nodiscard]] bool IsLoopStart(u16 address) const
+  {
+    return (GetCodeFlags(address) & CODE_LOOP_START) != 0;
+  }
 
-// This one should be called every time IRAM changes - which is basically
-// every time that a new ucode gets uploaded, and never else. At that point,
-// we can do as much static analysis as we want - but we should always throw
-// all old analysis away. Luckily the entire address space is only 64K code
-// words and the actual code space 8K instructions in total, so we can do
-// some pretty expensive analysis if necessary.
-void Analyze(const SDSP& dsp);
+  // Whether or not the address indicates the end of a loop.
+  [[nodiscard]] bool IsLoopEnd(u16 address) const
+  {
+    return (GetCodeFlags(address) & CODE_LOOP_END) != 0;
+  }
 
-// Retrieves the flags set during analysis for code in memory.
-u8 GetCodeFlags(u16 address);
+  // Whether or not the address describes an instruction that requires updating the SR register.
+  [[nodiscard]] bool IsUpdateSR(u16 address) const
+  {
+    return (GetCodeFlags(address) & CODE_UPDATE_SR) != 0;
+  }
 
-}  // namespace DSP::Analyzer
+  // Whether or not the address describes instructions that potentially raise exceptions.
+  [[nodiscard]] bool IsCheckExceptions(u16 address) const
+  {
+    return (GetCodeFlags(address) & CODE_CHECK_EXC) != 0;
+  }
+
+private:
+  enum CodeFlags : u8
+  {
+    CODE_NONE = 0,
+    CODE_START_OF_INST = 1,
+    CODE_IDLE_SKIP = 2,
+    CODE_LOOP_START = 4,
+    CODE_LOOP_END = 8,
+    CODE_UPDATE_SR = 16,
+    CODE_CHECK_EXC = 32,
+  };
+
+  // Flushes all analyzed state.
+  void Reset();
+
+  // Analyzes a region of DSP memory.
+  // Note: start is inclusive, end is exclusive.
+  void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr);
+
+  // Finds addresses in the range [start_addr, end_addr) that are the start of an
+  // instruction. During this process other attributes may be detected as well
+  // for relevant instructions (loop start/end, etc).
+  void FindInstructionStarts(const SDSP& dsp, u16 start_addr, u16 end_addr);
+
+  // Finds locations within the range [start_addr, end_addr) that may contain idle skips.
+  void FindIdleSkips(const SDSP& dsp, u16 start_addr, u16 end_addr);
+
+  // Retrieves the flags set during analysis for code in memory.
+  [[nodiscard]] u8 GetCodeFlags(u16 address) const { return m_code_flags[address]; }
+
+  // Holds data about all instructions in RAM.
+  std::array<u8, 65536> m_code_flags{};
+};
+}  // namespace DSP
diff --git a/Source/Core/Core/DSP/DSPCore.cpp b/Source/Core/Core/DSP/DSPCore.cpp
index ea075f78d9..d06c324118 100644
--- a/Source/Core/Core/DSP/DSPCore.cpp
+++ b/Source/Core/Core/DSP/DSPCore.cpp
@@ -483,7 +483,7 @@ void DSPCore::Step()
 void DSPCore::Reset()
 {
   m_dsp.Reset();
-  Analyzer::Analyze(m_dsp);
+  m_dsp.GetAnalyzer().Analyze(m_dsp);
 }
 
 void DSPCore::ClearIRAM()
diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h
index ea723b580c..fb8af13e38 100644
--- a/Source/Core/Core/DSP/DSPCore.h
+++ b/Source/Core/Core/DSP/DSPCore.h
@@ -12,6 +12,7 @@
 #include <string>
 
 #include "Common/Event.h"
+#include "Core/DSP/DSPAnalyzer.h"
 #include "Core/DSP/DSPBreakpoints.h"
 #include "Core/DSP/DSPCaptureLogger.h"
 
@@ -398,6 +399,10 @@ struct SDSP
   // Saves and loads any necessary state.
   void DoState(PointerWrap& p);
 
+  // DSP static analyzer.
+  Analyzer& GetAnalyzer() { return m_analyzer; }
+  const Analyzer& GetAnalyzer() const { return m_analyzer; }
+
   DSP_Regs r{};
   u16 pc = 0;
 
@@ -451,6 +456,7 @@ private:
   u16 ReadIFXImpl(u16 address);
 
   DSPCore& m_dsp_core;
+  Analyzer m_analyzer;
 };
 
 enum class State
diff --git a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp
index bd45ff46c2..b900d0585c 100644
--- a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp
+++ b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp
@@ -46,14 +46,16 @@ void Interpreter::ExecuteInstruction(const UDSPInstruction inst)
 
 void Interpreter::Step()
 {
+  auto& state = m_dsp_core.DSPState();
+
   m_dsp_core.CheckExceptions();
-  m_dsp_core.DSPState().step_counter++;
+  state.step_counter++;
 
-  const u16 opc = m_dsp_core.DSPState().FetchInstruction();
+  const u16 opc = state.FetchInstruction();
   ExecuteInstruction(UDSPInstruction{opc});
 
-  const auto pc = m_dsp_core.DSPState().pc;
-  if ((Analyzer::GetCodeFlags(static_cast<u16>(pc - 1)) & Analyzer::CODE_LOOP_END) != 0)
+  const auto pc = state.pc;
+  if (state.GetAnalyzer().IsLoopEnd(static_cast<u16>(pc - 1)))
     HandleLoop();
 }
 
@@ -117,8 +119,7 @@ int Interpreter::RunCyclesDebug(int cycles)
         return cycles;
       }
 
-      // Idle skipping.
-      if ((Analyzer::GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0)
+      if (state.GetAnalyzer().IsIdleSkip(state.pc))
         return 0;
 
       Step();
@@ -173,8 +174,7 @@ int Interpreter::RunCycles(int cycles)
       if ((state.cr & CR_HALT) != 0)
         return 0;
 
-      // Idle skipping.
-      if ((Analyzer::GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0)
+      if (state.GetAnalyzer().IsIdleSkip(state.pc))
         return 0;
 
       Step();
diff --git a/Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp b/Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp
index 25f26d0f34..5bb2326019 100644
--- a/Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp
+++ b/Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp
@@ -128,9 +128,9 @@ void DSPEmitter::checkExceptions(u32 retval)
 
 bool DSPEmitter::FlagsNeeded() const
 {
-  const u8 flags = Analyzer::GetCodeFlags(m_compile_pc);
+  const auto& analyzer = m_dsp_core.DSPState().GetAnalyzer();
 
-  return !(flags & Analyzer::CODE_START_OF_INST) || (flags & Analyzer::CODE_UPDATE_SR);
+  return !analyzer.IsStartOfInstruction(m_compile_pc) || analyzer.IsUpdateSR(m_compile_pc);
 }
 
 static void FallbackThunk(Interpreter::Interpreter& interpreter, UDSPInstruction inst)
@@ -242,9 +242,10 @@ void DSPEmitter::Compile(u16 start_addr)
   bool fixup_pc = false;
   m_block_size[start_addr] = 0;
 
+  auto& analyzer = m_dsp_core.DSPState().GetAnalyzer();
   while (m_compile_pc < start_addr + MAX_BLOCK_SIZE)
   {
-    if (Analyzer::GetCodeFlags(m_compile_pc) & Analyzer::CODE_CHECK_INT)
+    if (analyzer.IsCheckExceptions(m_compile_pc))
       checkExceptions(m_block_size[start_addr]);
 
     const UDSPInstruction inst = m_dsp_core.DSPState().ReadIMEM(m_compile_pc);
@@ -262,7 +263,7 @@ void DSPEmitter::Compile(u16 start_addr)
 
     // Handle loop condition, only if current instruction was flagged as a loop destination
     // by the analyzer.
-    if (Analyzer::GetCodeFlags(static_cast<u16>(m_compile_pc - 1u)) & Analyzer::CODE_LOOP_END)
+    if (analyzer.IsLoopEnd(static_cast<u16>(m_compile_pc - 1u)))
     {
       MOVZX(32, 16, EAX, M_SDSP_r_st(2));
       TEST(32, R(EAX), R(EAX));
@@ -283,7 +284,7 @@ void DSPEmitter::Compile(u16 start_addr)
       DSPJitRegCache c(m_gpr);
       HandleLoop();
       m_gpr.SaveRegs();
-      if (!Host::OnThread() && Analyzer::GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP)
+      if (!Host::OnThread() && analyzer.IsIdleSkip(start_addr))
       {
         MOV(16, R(EAX), Imm16(DSP_IDLE_SKIP_CYCLES));
       }
@@ -319,7 +320,7 @@ void DSPEmitter::Compile(u16 start_addr)
         DSPJitRegCache c(m_gpr);
         // don't update g_dsp.pc -- the branch insn already did
         m_gpr.SaveRegs();
-        if (!Host::OnThread() && Analyzer::GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP)
+        if (!Host::OnThread() && analyzer.IsIdleSkip(start_addr))
         {
           MOV(16, R(EAX), Imm16(DSP_IDLE_SKIP_CYCLES));
         }
@@ -336,7 +337,7 @@ void DSPEmitter::Compile(u16 start_addr)
     }
 
     // End the block if we're before an idle skip address
-    if (Analyzer::GetCodeFlags(m_compile_pc) & Analyzer::CODE_IDLE_SKIP)
+    if (analyzer.IsIdleSkip(m_compile_pc))
     {
       break;
     }
@@ -382,7 +383,7 @@ void DSPEmitter::Compile(u16 start_addr)
   }
 
   m_gpr.SaveRegs();
-  if (!Host::OnThread() && Analyzer::GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP)
+  if (!Host::OnThread() && analyzer.IsIdleSkip(start_addr))
   {
     MOV(16, R(EAX), Imm16(DSP_IDLE_SKIP_CYCLES));
   }
diff --git a/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp b/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp
index 6a132285fe..de0b266a9a 100644
--- a/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp
+++ b/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp
@@ -82,7 +82,7 @@ void DSPEmitter::WriteBranchExit()
 {
   DSPJitRegCache c(m_gpr);
   m_gpr.SaveRegs();
-  if (Analyzer::GetCodeFlags(m_start_address) & Analyzer::CODE_IDLE_SKIP)
+  if (m_dsp_core.DSPState().GetAnalyzer().IsIdleSkip(m_start_address))
   {
     MOV(16, R(EAX), Imm16(0x1000));
   }
diff --git a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp
index f42510b874..65e26ce238 100644
--- a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp
+++ b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp
@@ -93,8 +93,7 @@ void CodeLoaded(DSPCore& dsp, const u8* ptr, size_t size)
   UpdateDebugger();
 
   dsp.ClearIRAM();
-
-  Analyzer::Analyze(state);
+  state.GetAnalyzer().Analyze(state);
 }
 
 void UpdateDebugger()