From ba6fea1c8189cc43431fdf245cb3dd3936dae6d2 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 19 Nov 2023 11:14:43 +0100 Subject: [PATCH 1/2] HLE: Refactor ReplaceFunctionIfPossible --- Source/Core/Core/HLE/HLE.cpp | 17 ++++++++ Source/Core/Core/HLE/HLE.h | 40 ++++++------------- .../CachedInterpreter/CachedInterpreter.cpp | 20 +++++----- .../Core/PowerPC/Interpreter/Interpreter.cpp | 11 +++-- Source/Core/Core/PowerPC/Jit64/Jit.cpp | 20 +++++----- Source/Core/Core/PowerPC/JitArm64/Jit.cpp | 20 +++++----- 6 files changed, 69 insertions(+), 59 deletions(-) diff --git a/Source/Core/Core/HLE/HLE.cpp b/Source/Core/Core/HLE/HLE.cpp index 310aec6620..2d4f3313f2 100644 --- a/Source/Core/Core/HLE/HLE.cpp +++ b/Source/Core/Core/HLE/HLE.cpp @@ -200,6 +200,23 @@ HookFlag GetHookFlagsByIndex(u32 index) return os_patches[index].flags; } +TryReplaceFunctionResult TryReplaceFunction(u32 address) +{ + const u32 hook_index = GetHookByFunctionAddress(address); + if (hook_index == 0) + return {}; + + const HookType type = GetHookTypeByIndex(hook_index); + if (type != HookType::Start && type != HookType::Replace) + return {}; + + const HookFlag flags = GetHookFlagsByIndex(hook_index); + if (!IsEnabled(flags)) + return {}; + + return {type, hook_index}; +} + bool IsEnabled(HookFlag flag) { return flag != HLE::HookFlag::Debug || Config::Get(Config::MAIN_ENABLE_DEBUGGING) || diff --git a/Source/Core/Core/HLE/HLE.h b/Source/Core/Core/HLE/HLE.h index 330cf6a92f..725ef1aa54 100644 --- a/Source/Core/Core/HLE/HLE.h +++ b/Source/Core/Core/HLE/HLE.h @@ -19,9 +19,9 @@ using HookFunction = void (*)(const Core::CPUThreadGuard&); enum class HookType { + None, // Do not hook the function Start, // Hook the beginning of the function and execute the function afterwards Replace, // Replace the function with the HLE version - None, // Do not hook the function }; enum class HookFlag @@ -39,6 +39,14 @@ struct Hook HookFlag flags; }; +struct TryReplaceFunctionResult +{ + HookType type = HookType::None; + u32 hook_index = 0; + + explicit operator bool() const { return type != HookType::None; } +}; + void PatchFixedFunctions(Core::System& system); void PatchFunctions(Core::System& system); void Clear(); @@ -59,32 +67,8 @@ HookFlag GetHookFlagsByIndex(u32 index); bool IsEnabled(HookFlag flag); -// Performs the backend-independent preliminary checking before calling a -// FunctionObject to do the actual replacing. Typically, this callback will -// be in the backend itself, containing the backend-specific portions -// required in replacing a function. -// -// fn may be any object that satisfies the FunctionObject concept in the C++ -// standard library. That is, any lambda, object with an overloaded function -// call operator, or regular function pointer. -// -// fn must return a bool indicating whether or not function replacing occurred. -// fn must also accept a parameter list of the form: fn(u32 function, HLE::HookType type). -template -bool ReplaceFunctionIfPossible(u32 address, FunctionObject fn) -{ - const u32 hook_index = GetHookByFunctionAddress(address); - if (hook_index == 0) - return false; +// Performs the backend-independent preliminary checking for whether a function +// can be HLEd. If it can be, the information needed for HLEing it is returned. +TryReplaceFunctionResult TryReplaceFunction(u32 address); - const HookType type = GetHookTypeByIndex(hook_index); - if (type != HookType::Start && type != HookType::Replace) - return false; - - const HookFlag flags = GetHookFlagsByIndex(hook_index); - if (!IsEnabled(flags)) - return false; - - return fn(hook_index, type); -} } // namespace HLE diff --git a/Source/Core/Core/PowerPC/CachedInterpreter/CachedInterpreter.cpp b/Source/Core/Core/PowerPC/CachedInterpreter/CachedInterpreter.cpp index 8fbed42f06..8a836b6fe5 100644 --- a/Source/Core/Core/PowerPC/CachedInterpreter/CachedInterpreter.cpp +++ b/Source/Core/Core/PowerPC/CachedInterpreter/CachedInterpreter.cpp @@ -269,17 +269,19 @@ bool CachedInterpreter::CheckIdle(CachedInterpreter& cached_interpreter, u32 idl bool CachedInterpreter::HandleFunctionHooking(u32 address) { - return HLE::ReplaceFunctionIfPossible(address, [&](u32 hook_index, HLE::HookType type) { - m_code.emplace_back(WritePC, address); - m_code.emplace_back(Interpreter::HLEFunction, hook_index); + const auto result = HLE::TryReplaceFunction(address); + if (!result) + return false; - if (type != HLE::HookType::Replace) - return false; + m_code.emplace_back(WritePC, address); + m_code.emplace_back(Interpreter::HLEFunction, result.hook_index); - m_code.emplace_back(EndBlock, js.downcountAmount); - m_code.emplace_back(); - return true; - }); + if (result.type != HLE::HookType::Replace) + return false; + + m_code.emplace_back(EndBlock, js.downcountAmount); + m_code.emplace_back(); + return true; } void CachedInterpreter::Jit(u32 address) diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp index 168df8deaf..f3d3619f1b 100644 --- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp +++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp @@ -106,10 +106,13 @@ void Interpreter::Trace(const UGeckoInstruction& inst) bool Interpreter::HandleFunctionHooking(u32 address) { - return HLE::ReplaceFunctionIfPossible(address, [this](u32 hook_index, HLE::HookType type) { - HLEFunction(*this, hook_index); - return type != HLE::HookType::Start; - }); + const auto result = HLE::TryReplaceFunction(address); + if (!result) + return false; + + HLEFunction(*this, result.hook_index); + + return result.type != HLE::HookType::Start; } int Interpreter::SingleStepInner() diff --git a/Source/Core/Core/PowerPC/Jit64/Jit.cpp b/Source/Core/Core/PowerPC/Jit64/Jit.cpp index 9b089196b8..5cd47cd092 100644 --- a/Source/Core/Core/PowerPC/Jit64/Jit.cpp +++ b/Source/Core/Core/PowerPC/Jit64/Jit.cpp @@ -1238,17 +1238,19 @@ void Jit64::IntializeSpeculativeConstants() bool Jit64::HandleFunctionHooking(u32 address) { - return HLE::ReplaceFunctionIfPossible(address, [&](u32 hook_index, HLE::HookType type) { - HLEFunction(hook_index); + const auto result = HLE::TryReplaceFunction(address); + if (!result) + return false; - if (type != HLE::HookType::Replace) - return false; + HLEFunction(result.hook_index); - MOV(32, R(RSCRATCH), PPCSTATE(npc)); - js.downcountAmount += js.st.numCycles; - WriteExitDestInRSCRATCH(); - return true; - }); + if (result.type != HLE::HookType::Replace) + return false; + + MOV(32, R(RSCRATCH), PPCSTATE(npc)); + js.downcountAmount += js.st.numCycles; + WriteExitDestInRSCRATCH(); + return true; } void LogGeneratedX86(size_t size, const PPCAnalyst::CodeBuffer& code_buffer, const u8* normalEntry, diff --git a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp index df782e4de8..a2dc1cb6b7 100644 --- a/Source/Core/Core/PowerPC/JitArm64/Jit.cpp +++ b/Source/Core/Core/PowerPC/JitArm64/Jit.cpp @@ -696,17 +696,19 @@ void JitArm64::WriteConditionalExceptionExit(int exception, ARM64Reg temp_gpr, A bool JitArm64::HandleFunctionHooking(u32 address) { - return HLE::ReplaceFunctionIfPossible(address, [&](u32 hook_index, HLE::HookType type) { - HLEFunction(hook_index); + const auto result = HLE::TryReplaceFunction(address); + if (!result) + return false; - if (type != HLE::HookType::Replace) - return false; + HLEFunction(result.hook_index); - LDR(IndexType::Unsigned, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(npc)); - js.downcountAmount += js.st.numCycles; - WriteExit(DISPATCHER_PC); - return true; - }); + if (result.type != HLE::HookType::Replace) + return false; + + LDR(IndexType::Unsigned, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(npc)); + js.downcountAmount += js.st.numCycles; + WriteExit(DISPATCHER_PC); + return true; } void JitArm64::DumpCode(const u8* start, const u8* end) From 3a00ff625e826bece6825af9a058e3fe97d7d735 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sun, 19 Nov 2023 11:26:38 +0100 Subject: [PATCH 2/2] PPCAnalyst: Don't discard registers across HLE'd functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not sure if this was causing correctness issues – it depends on whether the HLE code was actually reading the discarded registers – but it was at least causing annoying assert messages in one piece of homebrew. --- Source/Core/Core/PowerPC/PPCAnalyst.cpp | 32 ++++++++++++++++++------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/PowerPC/PPCAnalyst.cpp b/Source/Core/Core/PowerPC/PPCAnalyst.cpp index 78a80e80a2..30134420bb 100644 --- a/Source/Core/Core/PowerPC/PPCAnalyst.cpp +++ b/Source/Core/Core/PowerPC/PPCAnalyst.cpp @@ -18,6 +18,7 @@ #include "Core/Config/MainSettings.h" #include "Core/ConfigManager.h" #include "Core/Core.h" +#include "Core/HLE/HLE.h" #include "Core/PowerPC/JitCommon/JitBase.h" #include "Core/PowerPC/MMU.h" #include "Core/PowerPC/PPCSymbolDB.h" @@ -970,15 +971,18 @@ u32 PPCAnalyzer::Analyze(u32 address, CodeBlock* block, CodeBuffer* buffer, fprDiscardable = BitSet32{}; } + const bool hle = !!HLE::TryReplaceFunction(op.address); + const bool may_exit_block = hle || op.canEndBlock || op.canCauseException; + const BitSet8 opWantsCR = op.wantsCR; const bool opWantsFPRF = op.wantsFPRF; const bool opWantsCA = op.wantsCA; - op.wantsCR = wantsCR | BitSet8(op.canEndBlock || op.canCauseException ? 0xFF : 0); - op.wantsFPRF = wantsFPRF || op.canEndBlock || op.canCauseException; - op.wantsCA = wantsCA || op.canEndBlock || op.canCauseException; - wantsCR |= opWantsCR | BitSet8(op.canEndBlock || op.canCauseException ? 0xFF : 0); - wantsFPRF |= opWantsFPRF || op.canEndBlock || op.canCauseException; - wantsCA |= opWantsCA || op.canEndBlock || op.canCauseException; + op.wantsCR = wantsCR | BitSet8(may_exit_block ? 0xFF : 0); + op.wantsFPRF = wantsFPRF || may_exit_block; + op.wantsCA = wantsCA || may_exit_block; + wantsCR |= opWantsCR | BitSet8(may_exit_block ? 0xFF : 0); + wantsFPRF |= opWantsFPRF || may_exit_block; + wantsCA |= opWantsCA || may_exit_block; wantsCR &= ~op.outputCR | opWantsCR; wantsFPRF &= !op.outputFPRF || opWantsFPRF; wantsCA &= !op.outputCA || opWantsCA; @@ -989,7 +993,19 @@ u32 PPCAnalyzer::Analyze(u32 address, CodeBlock* block, CodeBuffer* buffer, op.fprInXmm = fprInXmm; gprInUse |= op.regsIn | op.regsOut; fprInUse |= op.fregsIn | op.GetFregsOut(); - if (op.canEndBlock || op.canCauseException) + + if (strncmp(op.opinfo->opname, "stfd", 4)) + fprInXmm |= op.fregsIn; + + if (hle) + { + gprInUse = BitSet32{}; + fprInUse = BitSet32{}; + fprInXmm = BitSet32{}; + gprDiscardable = BitSet32{}; + fprDiscardable = BitSet32{}; + } + else if (op.canEndBlock || op.canCauseException) { gprDiscardable = BitSet32{}; fprDiscardable = BitSet32{}; @@ -1001,8 +1017,6 @@ u32 PPCAnalyzer::Analyze(u32 address, CodeBlock* block, CodeBuffer* buffer, fprDiscardable |= op.GetFregsOut(); fprDiscardable &= ~op.fregsIn; } - if (strncmp(op.opinfo->opname, "stfd", 4)) - fprInXmm |= op.fregsIn; } // Forward scan, for flags that need the other direction for calculation.