From 8f4c6ad7b102244e056875b30c2fb2243b70f74f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 09:59:45 -0500 Subject: [PATCH 1/7] DSPAnalyzer: Add basic class skeleton Adds the non-functional skeleton for the to-be Analyzer interface with deglobalized components. --- Source/Core/Core/DSP/DSPAnalyzer.h | 42 +++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index dab41baadc..5cf80b88fe 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -4,6 +4,7 @@ #pragma once +#include #include "Common/CommonTypes.h" namespace DSP @@ -17,8 +18,9 @@ namespace DSP::Analyzer // Useful things to detect: // * Loop endpoints - so that we can avoid checking for loops every cycle. -enum +enum CodeFlags : u8 { + CODE_NONE = 0, CODE_START_OF_INST = 1, CODE_IDLE_SKIP = 2, CODE_LOOP_START = 4, @@ -27,6 +29,44 @@ enum CODE_CHECK_INT = 32, }; +class Analyzer +{ +public: + explicit Analyzer(const SDSP& dsp); + ~Analyzer(); + + Analyzer(const Analyzer&) = default; + Analyzer& operator=(const Analyzer&) = delete; + + Analyzer(Analyzer&&) = default; + Analyzer& operator=(Analyzer&&) = delete; + + // 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(); + + // Retrieves the flags set during analysis for code in memory. + [[nodiscard]] u8 GetCodeFlags(u16 address) const { return m_code_flags[address]; } + +private: + // Flushes all analyzed state. + void Reset(); + + // Analyzes a region of DSP memory. + // Note: start is inclusive, end is exclusive. + void AnalyzeRange(u16 start_addr, u16 end_addr); + + // Holds data about all instructions in RAM. + std::array m_code_flags{}; + + // DSP context for analysis to be run under. + const SDSP& m_dsp; +}; + // 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 From 5756ece7cee89263b78ab8fe0ad0819d1e752ecd Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 10:51:19 -0500 Subject: [PATCH 2/7] DSPAnalyzer: Implement DSP analyzer skeleton and use it Attempts to simply make use of the interface. Cleanup will follow in subsequent commits to make for nicer review. --- Source/Core/Core/DSP/DSPAnalyzer.cpp | 67 +++++++++---------- Source/Core/Core/DSP/DSPAnalyzer.h | 12 ---- Source/Core/Core/DSP/DSPCore.cpp | 4 +- Source/Core/Core/DSP/DSPCore.h | 6 ++ .../Core/DSP/Interpreter/DSPInterpreter.cpp | 16 +++-- Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp | 15 +++-- Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp | 3 +- Source/Core/Core/HW/DSPLLE/DSPHost.cpp | 3 +- 8 files changed, 59 insertions(+), 67 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.cpp b/Source/Core/Core/DSP/DSPAnalyzer.cpp index 7a88b99773..107645d6c5 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.cpp +++ b/Source/Core/Core/DSP/DSPAnalyzer.cpp @@ -14,13 +14,6 @@ namespace DSP::Analyzer { -namespace -{ -constexpr size_t ISPACE = 65536; - -// Holds data about all instructions in RAM. -std::array 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,14 +58,28 @@ 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}, +}; -void Reset() +Analyzer::Analyzer(const SDSP& dsp) : m_dsp{dsp} { - code_flags.fill(0); } -void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr) +Analyzer::~Analyzer() = default; + +void Analyzer::Analyze() +{ + Reset(); + AnalyzeRange(0x0000, 0x1000); // IRAM + AnalyzeRange(0x8000, 0x9000); // IROM +} + +void Analyzer::Reset() +{ + m_code_flags.fill(0); +} + +void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) { // First we run an extremely simplified version of a disassembler to find // where all instructions start. @@ -82,27 +89,27 @@ void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr) u16 last_arithmetic = 0; for (u16 addr = start_addr; addr < end_addr;) { - const UDSPInstruction inst = dsp.ReadIMEM(addr); + const UDSPInstruction inst = m_dsp.ReadIMEM(addr); const DSPOPCTemplate* opcode = GetOpTemplate(inst); if (!opcode) { 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; + const u16 loop_end = m_dsp.ReadIMEM(addr + 1); + 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(addr + 1u)] |= CODE_LOOP_END; + m_code_flags[addr] |= CODE_LOOP_START; + m_code_flags[static_cast(addr + 1u)] |= CODE_LOOP_END; } // Mark the last arithmetic/multiplier instruction before a branch. @@ -114,7 +121,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,7 +129,9 @@ 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(addr + opcode->size)] |= CODE_CHECK_INT; + { + m_code_flags[static_cast(addr + opcode->size)] |= CODE_CHECK_INT; + } addr += opcode->size; } @@ -139,30 +148,16 @@ void AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr) found = true; if (idle_skip_sigs[s][i] == 0xFFFF) continue; - if (idle_skip_sigs[s][i] != dsp.ReadIMEM(static_cast(addr + i))) + if (idle_skip_sigs[s][i] != m_dsp.ReadIMEM(static_cast(addr + i))) break; } 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 diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index 5cf80b88fe..ce599cffc2 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -66,16 +66,4 @@ private: // DSP context for analysis to be run under. const SDSP& m_dsp; }; - -// 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); - -// Retrieves the flags set during analysis for code in memory. -u8 GetCodeFlags(u16 address); - } // namespace DSP::Analyzer diff --git a/Source/Core/Core/DSP/DSPCore.cpp b/Source/Core/Core/DSP/DSPCore.cpp index 39618d5621..7be3350ae6 100644 --- a/Source/Core/Core/DSP/DSPCore.cpp +++ b/Source/Core/Core/DSP/DSPCore.cpp @@ -115,7 +115,7 @@ private: SDSP& m_dsp; }; -SDSP::SDSP(DSPCore& core) : m_dsp_core{core} +SDSP::SDSP(DSPCore& core) : m_dsp_core{core}, m_analyzer{*this} { } @@ -487,7 +487,7 @@ void DSPCore::Step() void DSPCore::Reset() { m_dsp.Reset(); - Analyzer::Analyze(m_dsp); + m_dsp.GetAnalyzer().Analyze(); } void DSPCore::ClearIRAM() diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h index 719e6a02c8..f25c302a70 100644 --- a/Source/Core/Core/DSP/DSPCore.h +++ b/Source/Core/Core/DSP/DSPCore.h @@ -12,6 +12,7 @@ #include #include "Common/Event.h" +#include "Core/DSP/DSPAnalyzer.h" #include "Core/DSP/DSPBreakpoints.h" #include "Core/DSP/DSPCaptureLogger.h" @@ -396,6 +397,10 @@ struct SDSP // Saves and loads any necessary state. void DoState(PointerWrap& p); + // DSP static analyzer. + Analyzer::Analyzer& GetAnalyzer() { return m_analyzer; } + const Analyzer::Analyzer& GetAnalyzer() const { return m_analyzer; } + DSP_Regs r{}; u16 pc = 0; @@ -449,6 +454,7 @@ private: u16 ReadIFXImpl(u16 address); DSPCore& m_dsp_core; + Analyzer::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 70ea39b8f2..57662e1fb3 100644 --- a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp +++ b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp @@ -42,14 +42,16 @@ void Interpreter::ExecuteInstruction(const UDSPInstruction inst) void Interpreter::Step() { - m_dsp_core.CheckExceptions(); - m_dsp_core.DSPState().step_counter++; + auto& state = m_dsp_core.DSPState(); - const u16 opc = m_dsp_core.DSPState().FetchInstruction(); + m_dsp_core.CheckExceptions(); + state.step_counter++; + + const u16 opc = state.FetchInstruction(); ExecuteInstruction(UDSPInstruction{opc}); - const auto pc = m_dsp_core.DSPState().pc; - if ((Analyzer::GetCodeFlags(static_cast(pc - 1)) & Analyzer::CODE_LOOP_END) != 0) + const auto pc = state.pc; + if ((state.GetAnalyzer().GetCodeFlags(static_cast(pc - 1)) & Analyzer::CODE_LOOP_END) != 0) HandleLoop(); } @@ -114,7 +116,7 @@ int Interpreter::RunCyclesDebug(int cycles) } // Idle skipping. - if ((Analyzer::GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0) + if ((state.GetAnalyzer().GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0) return 0; Step(); @@ -170,7 +172,7 @@ int Interpreter::RunCycles(int cycles) return 0; // Idle skipping. - if ((Analyzer::GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0) + if ((state.GetAnalyzer().GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0) 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..dce1fb52ab 100644 --- a/Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp +++ b/Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp @@ -128,7 +128,7 @@ void DSPEmitter::checkExceptions(u32 retval) bool DSPEmitter::FlagsNeeded() const { - const u8 flags = Analyzer::GetCodeFlags(m_compile_pc); + const u8 flags = m_dsp_core.DSPState().GetAnalyzer().GetCodeFlags(m_compile_pc); return !(flags & Analyzer::CODE_START_OF_INST) || (flags & Analyzer::CODE_UPDATE_SR); } @@ -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.GetCodeFlags(m_compile_pc) & Analyzer::CODE_CHECK_INT) 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(m_compile_pc - 1u)) & Analyzer::CODE_LOOP_END) + if ((analyzer.GetCodeFlags(static_cast(m_compile_pc - 1u)) & Analyzer::CODE_LOOP_END) != 0) { 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.GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP) != 0) { 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.GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP) != 0) { 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.GetCodeFlags(m_compile_pc) & Analyzer::CODE_IDLE_SKIP) != 0) { 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.GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP) != 0) { 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 664ce367c6..ca864c9943 100644 --- a/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp +++ b/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp @@ -82,7 +82,8 @@ 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().GetCodeFlags(m_start_address) & + Analyzer::CODE_IDLE_SKIP) != 0) { 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..b0e06edca5 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(); } void UpdateDebugger() From 2ff4d047851d1c3b40dff32d7ce648669d9db5fe Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 11:15:55 -0500 Subject: [PATCH 3/7] DSPAnalyzer: Add convenience functions over bit tests Makes it harder to accidentally misuse and increases readability. --- Source/Core/Core/DSP/DSPAnalyzer.cpp | 2 +- Source/Core/Core/DSP/DSPAnalyzer.h | 42 +++++++++++++++++-- .../Core/DSP/Interpreter/DSPInterpreter.cpp | 8 ++-- Source/Core/Core/DSP/Jit/x64/DSPEmitter.cpp | 16 +++---- Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp | 3 +- 5 files changed, 52 insertions(+), 19 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.cpp b/Source/Core/Core/DSP/DSPAnalyzer.cpp index 107645d6c5..a86b9c2c3e 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.cpp +++ b/Source/Core/Core/DSP/DSPAnalyzer.cpp @@ -130,7 +130,7 @@ void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) opcode->opcode == 0x1900 || opcode->opcode == 0x1980 || opcode->opcode == 0x2000 || opcode->extended) { - m_code_flags[static_cast(addr + opcode->size)] |= CODE_CHECK_INT; + m_code_flags[static_cast(addr + opcode->size)] |= CODE_CHECK_EXC; } addr += opcode->size; diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index ce599cffc2..e2663adfd5 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -26,7 +26,7 @@ enum CodeFlags : u8 CODE_LOOP_START = 4, CODE_LOOP_END = 8, CODE_UPDATE_SR = 16, - CODE_CHECK_INT = 32, + CODE_CHECK_EXC = 32, }; class Analyzer @@ -49,8 +49,41 @@ public: // some pretty expensive analysis if necessary. void Analyze(); - // Retrieves the flags set during analysis for code in memory. - [[nodiscard]] u8 GetCodeFlags(u16 address) const { return m_code_flags[address]; } + // 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; + } + + // Whether or not the address indicates the end of a loop. + [[nodiscard]] bool IsLoopEnd(u16 address) const + { + return (GetCodeFlags(address) & CODE_LOOP_END) != 0; + } + + // 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; + } + + // 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: // Flushes all analyzed state. @@ -60,6 +93,9 @@ private: // Note: start is inclusive, end is exclusive. void AnalyzeRange(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 m_code_flags{}; diff --git a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp index 57662e1fb3..acde5bc721 100644 --- a/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp +++ b/Source/Core/Core/DSP/Interpreter/DSPInterpreter.cpp @@ -51,7 +51,7 @@ void Interpreter::Step() ExecuteInstruction(UDSPInstruction{opc}); const auto pc = state.pc; - if ((state.GetAnalyzer().GetCodeFlags(static_cast(pc - 1)) & Analyzer::CODE_LOOP_END) != 0) + if (state.GetAnalyzer().IsLoopEnd(static_cast(pc - 1))) HandleLoop(); } @@ -115,8 +115,7 @@ int Interpreter::RunCyclesDebug(int cycles) return cycles; } - // Idle skipping. - if ((state.GetAnalyzer().GetCodeFlags(state.pc) & Analyzer::CODE_IDLE_SKIP) != 0) + if (state.GetAnalyzer().IsIdleSkip(state.pc)) return 0; Step(); @@ -171,8 +170,7 @@ int Interpreter::RunCycles(int cycles) if ((state.cr & CR_HALT) != 0) return 0; - // Idle skipping. - if ((state.GetAnalyzer().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 dce1fb52ab..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 = m_dsp_core.DSPState().GetAnalyzer().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) @@ -245,7 +245,7 @@ void DSPEmitter::Compile(u16 start_addr) 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); @@ -263,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(m_compile_pc - 1u)) & Analyzer::CODE_LOOP_END) != 0) + if (analyzer.IsLoopEnd(static_cast(m_compile_pc - 1u))) { MOVZX(32, 16, EAX, M_SDSP_r_st(2)); TEST(32, R(EAX), R(EAX)); @@ -284,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) != 0) + if (!Host::OnThread() && analyzer.IsIdleSkip(start_addr)) { MOV(16, R(EAX), Imm16(DSP_IDLE_SKIP_CYCLES)); } @@ -320,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) != 0) + if (!Host::OnThread() && analyzer.IsIdleSkip(start_addr)) { MOV(16, R(EAX), Imm16(DSP_IDLE_SKIP_CYCLES)); } @@ -337,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) != 0) + if (analyzer.IsIdleSkip(m_compile_pc)) { break; } @@ -383,7 +383,7 @@ void DSPEmitter::Compile(u16 start_addr) } m_gpr.SaveRegs(); - if (!Host::OnThread() && (analyzer.GetCodeFlags(start_addr) & Analyzer::CODE_IDLE_SKIP) != 0) + 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 ca864c9943..34204d4c07 100644 --- a/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp +++ b/Source/Core/Core/DSP/Jit/x64/DSPJitBranch.cpp @@ -82,8 +82,7 @@ void DSPEmitter::WriteBranchExit() { DSPJitRegCache c(m_gpr); m_gpr.SaveRegs(); - if ((m_dsp_core.DSPState().GetAnalyzer().GetCodeFlags(m_start_address) & - Analyzer::CODE_IDLE_SKIP) != 0) + if (m_dsp_core.DSPState().GetAnalyzer().IsIdleSkip(m_start_address)) { MOV(16, R(EAX), Imm16(0x1000)); } From 9d1c8fe49239242002ffe1000a1a1f1807d6a767 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 11:31:26 -0500 Subject: [PATCH 4/7] DSPAnalyzer: Make CodeFlags private to the analyzer Now that we have the convenience functions around the flag bit manipulations, there's no external usages of the flags, so we can make these private to the analyzer implementation. Now the Analyzer namespace is largely unnecessary and can be merged with the DSP namespace in the next commit. --- Source/Core/Core/DSP/DSPAnalyzer.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index e2663adfd5..21dc740908 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -18,17 +18,6 @@ namespace DSP::Analyzer // Useful things to detect: // * Loop endpoints - so that we can avoid checking for loops every cycle. -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, -}; - class Analyzer { public: @@ -86,6 +75,17 @@ public: } 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(); From f9c488f0d9cdfe2bc9115c23425a27db9d3075ef Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 11:39:37 -0500 Subject: [PATCH 5/7] DSPAnalyzer: Merge Analyzer namespace into DSP namespace Now that the Analyzer class fully encapsulates all analyzer state, the namespace is no longer necessary. --- Source/Core/Core/DSP/DSPAnalyzer.cpp | 4 ++-- Source/Core/Core/DSP/DSPAnalyzer.h | 5 ++--- Source/Core/Core/DSP/DSPCore.h | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.cpp b/Source/Core/Core/DSP/DSPAnalyzer.cpp index a86b9c2c3e..ccafaebc51 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.cpp +++ b/Source/Core/Core/DSP/DSPAnalyzer.cpp @@ -12,7 +12,7 @@ #include "Core/DSP/DSPCore.h" #include "Core/DSP/DSPTables.h" -namespace DSP::Analyzer +namespace DSP { // 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 @@ -160,4 +160,4 @@ void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) } INFO_LOG_FMT(DSPLLE, "Finished analysis."); } -} // namespace DSP::Analyzer +} // namespace DSP diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index 21dc740908..d3077fc48b 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -12,8 +12,7 @@ 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. @@ -102,4 +101,4 @@ private: // DSP context for analysis to be run under. const SDSP& m_dsp; }; -} // namespace DSP::Analyzer +} // namespace DSP diff --git a/Source/Core/Core/DSP/DSPCore.h b/Source/Core/Core/DSP/DSPCore.h index f25c302a70..e3ed8f6f76 100644 --- a/Source/Core/Core/DSP/DSPCore.h +++ b/Source/Core/Core/DSP/DSPCore.h @@ -398,8 +398,8 @@ struct SDSP void DoState(PointerWrap& p); // DSP static analyzer. - Analyzer::Analyzer& GetAnalyzer() { return m_analyzer; } - const Analyzer::Analyzer& GetAnalyzer() const { return m_analyzer; } + Analyzer& GetAnalyzer() { return m_analyzer; } + const Analyzer& GetAnalyzer() const { return m_analyzer; } DSP_Regs r{}; u16 pc = 0; @@ -454,7 +454,7 @@ private: u16 ReadIFXImpl(u16 address); DSPCore& m_dsp_core; - Analyzer::Analyzer m_analyzer; + Analyzer m_analyzer; }; enum class State From cc512a75241352929101c6d31fd10b285362f4f5 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 11:51:25 -0500 Subject: [PATCH 6/7] DSPAnalyzer: Break tight coupling to SDSP Allows the analyzer to exist independently of the DSP structure. This allows for unit-tests to be created in a nicer manner. SDSP is only necessary during the analysis phase, so we only need to keep a reference around to it then as opposed to the entire lifecycle of the analyzer. This also allows the copy/move assignment operators to be defaulted, as a reference member variable prevents that. --- Source/Core/Core/DSP/DSPAnalyzer.cpp | 19 ++++++++----------- Source/Core/Core/DSP/DSPAnalyzer.h | 13 +++++-------- Source/Core/Core/DSP/DSPCore.cpp | 4 ++-- Source/Core/Core/HW/DSPLLE/DSPHost.cpp | 2 +- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.cpp b/Source/Core/Core/DSP/DSPAnalyzer.cpp index ccafaebc51..ae81357b19 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.cpp +++ b/Source/Core/Core/DSP/DSPAnalyzer.cpp @@ -61,17 +61,14 @@ constexpr u16 idle_skip_sigs[NUM_IDLE_SIGS][MAX_IDLE_SIG_SIZE + 1] = { 0, 0}, }; -Analyzer::Analyzer(const SDSP& dsp) : m_dsp{dsp} -{ -} - +Analyzer::Analyzer() = default; Analyzer::~Analyzer() = default; -void Analyzer::Analyze() +void Analyzer::Analyze(const SDSP& dsp) { Reset(); - AnalyzeRange(0x0000, 0x1000); // IRAM - AnalyzeRange(0x8000, 0x9000); // IROM + AnalyzeRange(dsp, 0x0000, 0x1000); // IRAM + AnalyzeRange(dsp, 0x8000, 0x9000); // IROM } void Analyzer::Reset() @@ -79,7 +76,7 @@ void Analyzer::Reset() m_code_flags.fill(0); } -void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) +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. @@ -89,7 +86,7 @@ void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) u16 last_arithmetic = 0; for (u16 addr = start_addr; addr < end_addr;) { - const UDSPInstruction inst = m_dsp.ReadIMEM(addr); + const UDSPInstruction inst = dsp.ReadIMEM(addr); const DSPOPCTemplate* opcode = GetOpTemplate(inst); if (!opcode) { @@ -101,7 +98,7 @@ void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) if ((inst & 0xffe0) == 0x0060 || (inst & 0xff00) == 0x1100) { // BLOOP, BLOOPI - const u16 loop_end = m_dsp.ReadIMEM(addr + 1); + const u16 loop_end = dsp.ReadIMEM(addr + 1); m_code_flags[addr] |= CODE_LOOP_START; m_code_flags[loop_end] |= CODE_LOOP_END; } @@ -148,7 +145,7 @@ void Analyzer::AnalyzeRange(u16 start_addr, u16 end_addr) found = true; if (idle_skip_sigs[s][i] == 0xFFFF) continue; - if (idle_skip_sigs[s][i] != m_dsp.ReadIMEM(static_cast(addr + i))) + if (idle_skip_sigs[s][i] != dsp.ReadIMEM(static_cast(addr + i))) break; } if (found) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index d3077fc48b..c48199aa24 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -20,14 +20,14 @@ namespace DSP class Analyzer { public: - explicit Analyzer(const SDSP& dsp); + explicit Analyzer(); ~Analyzer(); Analyzer(const Analyzer&) = default; - Analyzer& operator=(const Analyzer&) = delete; + Analyzer& operator=(const Analyzer&) = default; Analyzer(Analyzer&&) = default; - Analyzer& operator=(Analyzer&&) = delete; + 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, @@ -35,7 +35,7 @@ public: // 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(); + void Analyze(const SDSP& dsp); // Whether or not the given address indicates the start of an instruction. [[nodiscard]] bool IsStartOfInstruction(u16 address) const @@ -90,15 +90,12 @@ private: // Analyzes a region of DSP memory. // Note: start is inclusive, end is exclusive. - void AnalyzeRange(u16 start_addr, u16 end_addr); + void AnalyzeRange(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 m_code_flags{}; - - // DSP context for analysis to be run under. - const SDSP& m_dsp; }; } // namespace DSP diff --git a/Source/Core/Core/DSP/DSPCore.cpp b/Source/Core/Core/DSP/DSPCore.cpp index 7be3350ae6..40c7c20cb4 100644 --- a/Source/Core/Core/DSP/DSPCore.cpp +++ b/Source/Core/Core/DSP/DSPCore.cpp @@ -115,7 +115,7 @@ private: SDSP& m_dsp; }; -SDSP::SDSP(DSPCore& core) : m_dsp_core{core}, m_analyzer{*this} +SDSP::SDSP(DSPCore& core) : m_dsp_core{core} { } @@ -487,7 +487,7 @@ void DSPCore::Step() void DSPCore::Reset() { m_dsp.Reset(); - m_dsp.GetAnalyzer().Analyze(); + m_dsp.GetAnalyzer().Analyze(m_dsp); } void DSPCore::ClearIRAM() diff --git a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp index b0e06edca5..65e26ce238 100644 --- a/Source/Core/Core/HW/DSPLLE/DSPHost.cpp +++ b/Source/Core/Core/HW/DSPLLE/DSPHost.cpp @@ -93,7 +93,7 @@ void CodeLoaded(DSPCore& dsp, const u8* ptr, size_t size) UpdateDebugger(); dsp.ClearIRAM(); - state.GetAnalyzer().Analyze(); + state.GetAnalyzer().Analyze(state); } void UpdateDebugger() From 8aecaf784c43aec914226fec08a7226eed6cba6d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 28 Dec 2020 12:09:31 -0500 Subject: [PATCH 7/7] DSPAnalyzer: Separate instruction searching and idle skip finding Places them into their own function to keep their functionality isolated and self-documenting. --- Source/Core/Core/DSP/DSPAnalyzer.cpp | 14 ++++++++++++-- Source/Core/Core/DSP/DSPAnalyzer.h | 8 ++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Source/Core/Core/DSP/DSPAnalyzer.cpp b/Source/Core/Core/DSP/DSPAnalyzer.cpp index ae81357b19..2b2d70e2e6 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.cpp +++ b/Source/Core/Core/DSP/DSPAnalyzer.cpp @@ -80,7 +80,16 @@ 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; @@ -132,8 +141,10 @@ void Analyzer::AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr) 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++) @@ -155,6 +166,5 @@ void Analyzer::AnalyzeRange(const SDSP& dsp, u16 start_addr, u16 end_addr) } } } - INFO_LOG_FMT(DSPLLE, "Finished analysis."); } } // namespace DSP diff --git a/Source/Core/Core/DSP/DSPAnalyzer.h b/Source/Core/Core/DSP/DSPAnalyzer.h index c48199aa24..84f1c1cd97 100644 --- a/Source/Core/Core/DSP/DSPAnalyzer.h +++ b/Source/Core/Core/DSP/DSPAnalyzer.h @@ -92,6 +92,14 @@ private: // 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]; }