From 3137d4f75f1eac20786a2627508542988ef81aa7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Mar 2018 02:36:38 -0400 Subject: [PATCH 1/3] BreakPoints: Use std algorithms where applicable --- Source/Core/Core/PowerPC/BreakPoints.cpp | 101 +++++++++++------------ 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index e87143bcf8..977d9425bd 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -88,16 +88,15 @@ void BreakPoints::Add(u32 address, bool temp) void BreakPoints::Remove(u32 address) { - for (auto i = m_breakpoints.begin(); i != m_breakpoints.end(); ++i) - { - if (i->address == address) - { - m_breakpoints.erase(i); - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(address, 4, true); - return; - } - } + const auto iter = std::find_if(m_breakpoints.begin(), m_breakpoints.end(), + [address](const auto& bp) { return bp.address == address; }); + + if (iter == m_breakpoints.cend()) + return; + + m_breakpoints.erase(iter); + if (g_jit) + g_jit->GetBlockCache()->InvalidateICache(address, 4, true); } void BreakPoints::Clear() @@ -187,52 +186,49 @@ void MemChecks::Add(const TMemCheck& memory_check) void MemChecks::Remove(u32 address) { - for (auto i = m_mem_checks.begin(); i != m_mem_checks.end(); ++i) - { - if (i->start_address == address) - { - Core::RunAsCPUThread([&] { - m_mem_checks.erase(i); - if (!HasAny() && g_jit) - g_jit->ClearCache(); - PowerPC::DBATUpdated(); - }); - return; - } - } + const auto iter = + std::find_if(m_mem_checks.cbegin(), m_mem_checks.cend(), + [address](const auto& check) { return check.start_address == address; }); + + if (iter == m_mem_checks.cend()) + return; + + Core::RunAsCPUThread([&] { + m_mem_checks.erase(iter); + if (!HasAny() && g_jit) + g_jit->ClearCache(); + PowerPC::DBATUpdated(); + }); } TMemCheck* MemChecks::GetMemCheck(u32 address, size_t size) { - for (TMemCheck& mc : m_mem_checks) - { - if (mc.end_address >= address && address + size - 1 >= mc.start_address) - { - return &mc; - } - } + const auto iter = + std::find_if(m_mem_checks.begin(), m_mem_checks.end(), [address, size](const auto& mc) { + return mc.end_address >= address && address + size - 1 >= mc.start_address; + }); - // none found - return nullptr; + // None found + if (iter == m_mem_checks.cend()) + return nullptr; + + return &*iter; } bool MemChecks::OverlapsMemcheck(u32 address, u32 length) { if (!HasAny()) return false; - u32 page_end_suffix = length - 1; - u32 page_end_address = address | page_end_suffix; - for (TMemCheck memcheck : m_mem_checks) - { - if (((memcheck.start_address | page_end_suffix) == page_end_address || - (memcheck.end_address | page_end_suffix) == page_end_address) || - ((memcheck.start_address | page_end_suffix) < page_end_address && - (memcheck.end_address | page_end_suffix) > page_end_address)) - { - return true; - } - } - return false; + + const u32 page_end_suffix = length - 1; + const u32 page_end_address = address | page_end_suffix; + + return std::any_of(m_mem_checks.cbegin(), m_mem_checks.cend(), [&](const auto& mc) { + return ((mc.start_address | page_end_suffix) == page_end_address || + (mc.end_address | page_end_suffix) == page_end_address) || + ((mc.start_address | page_end_suffix) < page_end_address && + (mc.end_address | page_end_suffix) > page_end_address); + }); } bool TMemCheck::Action(DebugInterface* debug_interface, u32 value, u32 addr, bool write, @@ -318,14 +314,13 @@ void Watches::UpdateName(int count, const std::string name) void Watches::Remove(u32 address) { - for (auto i = m_watches.begin(); i != m_watches.end(); ++i) - { - if (i->address == address) - { - m_watches.erase(i); - return; - } - } + const auto iter = std::find_if(m_watches.cbegin(), m_watches.cend(), + [address](const auto& watch) { return watch.address == address; }); + + if (iter == m_watches.cend()) + return; + + m_watches.erase(iter); } void Watches::Clear() From b4cd11c7c9839f7a6573de6960f6becabfb2b444 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Mar 2018 02:51:59 -0400 Subject: [PATCH 2/3] BreakPoints: Invert if statements where reasonable Puts the early-exit condition first, unindenting code that doesn't need to be. --- Source/Core/Core/PowerPC/BreakPoints.cpp | 81 ++++++++++++------------ 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index 977d9425bd..412250ed54 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -62,28 +62,30 @@ void BreakPoints::AddFromStrings(const TBreakPointsStr& bp_strings) void BreakPoints::Add(const TBreakPoint& bp) { - if (!IsAddressBreakPoint(bp.address)) - { - m_breakpoints.push_back(bp); - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(bp.address, 4, true); - } + if (IsAddressBreakPoint(bp.address)) + return; + + m_breakpoints.push_back(bp); + + if (g_jit) + g_jit->GetBlockCache()->InvalidateICache(bp.address, 4, true); } void BreakPoints::Add(u32 address, bool temp) { - if (!IsAddressBreakPoint(address)) // only add new addresses - { - TBreakPoint bp; // breakpoint settings - bp.is_enabled = true; - bp.is_temporary = temp; - bp.address = address; + // Only add new addresses + if (IsAddressBreakPoint(address)) + return; - m_breakpoints.push_back(bp); + TBreakPoint bp; // breakpoint settings + bp.is_enabled = true; + bp.is_temporary = temp; + bp.address = address; - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(address, 4, true); - } + m_breakpoints.push_back(bp); + + if (g_jit) + g_jit->GetBlockCache()->InvalidateICache(address, 4, true); } void BreakPoints::Remove(u32 address) @@ -170,18 +172,18 @@ void MemChecks::AddFromStrings(const TMemChecksStr& mc_strings) void MemChecks::Add(const TMemCheck& memory_check) { - if (GetMemCheck(memory_check.start_address) == nullptr) - { - bool had_any = HasAny(); - Core::RunAsCPUThread([&] { - m_mem_checks.push_back(memory_check); - // If this is the first one, clear the JIT cache so it can switch to - // watchpoint-compatible code. - if (!had_any && g_jit) - g_jit->ClearCache(); - PowerPC::DBATUpdated(); - }); - } + if (GetMemCheck(memory_check.start_address) != nullptr) + return; + + bool had_any = HasAny(); + Core::RunAsCPUThread([&] { + m_mem_checks.push_back(memory_check); + // If this is the first one, clear the JIT cache so it can switch to + // watchpoint-compatible code. + if (!had_any && g_jit) + g_jit->ClearCache(); + PowerPC::DBATUpdated(); + }); } void MemChecks::Remove(u32 address) @@ -284,22 +286,23 @@ void Watches::AddFromStrings(const TWatchesStr& watch_strings) void Watches::Add(const TWatch& watch) { - if (!IsAddressWatch(watch.address)) - { - m_watches.push_back(watch); - } + if (IsAddressWatch(watch.address)) + return; + + m_watches.push_back(watch); } void Watches::Add(u32 address) { - if (!IsAddressWatch(address)) // only add new addresses - { - TWatch watch; // watch settings - watch.is_enabled = true; - watch.address = address; + // Only add new addresses + if (IsAddressWatch(address)) + return; - m_watches.push_back(watch); - } + TWatch watch; // watch settings + watch.is_enabled = true; + watch.address = address; + + m_watches.push_back(watch); } void Watches::Update(int count, u32 address) From a0164e14bc44ff2e6d35c2641d20c43c9a0fcb13 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 19 Mar 2018 02:55:54 -0400 Subject: [PATCH 3/3] BreakPoints: Avoid direct use of the JIT global Trims the direct usages of the global by making the code go through the JIT interface (where it should have been going in the first place). This also removes direct JIT header dependencies from the breakpoints as well. Now, no code uses the JIT global other than JIT code itself, and the unit tests. --- Source/Core/Core/PowerPC/BreakPoints.cpp | 31 +++++++++--------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index 412250ed54..74dd6e9f56 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -12,9 +12,9 @@ #include "Common/CommonTypes.h" #include "Common/DebugInterface.h" +#include "Common/Logging/Log.h" #include "Core/Core.h" -#include "Core/PowerPC/JitCommon/JitBase.h" -#include "Core/PowerPC/JitCommon/JitCache.h" +#include "Core/PowerPC/JitInterface.h" #include "Core/PowerPC/PowerPC.h" bool BreakPoints::IsAddressBreakPoint(u32 address) const @@ -67,8 +67,7 @@ void BreakPoints::Add(const TBreakPoint& bp) m_breakpoints.push_back(bp); - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(bp.address, 4, true); + JitInterface::InvalidateICache(bp.address, 4, true); } void BreakPoints::Add(u32 address, bool temp) @@ -84,8 +83,7 @@ void BreakPoints::Add(u32 address, bool temp) m_breakpoints.push_back(bp); - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(address, 4, true); + JitInterface::InvalidateICache(address, 4, true); } void BreakPoints::Remove(u32 address) @@ -97,18 +95,14 @@ void BreakPoints::Remove(u32 address) return; m_breakpoints.erase(iter); - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(address, 4, true); + JitInterface::InvalidateICache(address, 4, true); } void BreakPoints::Clear() { - if (g_jit) + for (const TBreakPoint& bp : m_breakpoints) { - for (const TBreakPoint& bp : m_breakpoints) - { - g_jit->GetBlockCache()->InvalidateICache(bp.address, 4, true); - } + JitInterface::InvalidateICache(bp.address, 4, true); } m_breakpoints.clear(); @@ -121,8 +115,7 @@ void BreakPoints::ClearAllTemporary() { if (bp->is_temporary) { - if (g_jit) - g_jit->GetBlockCache()->InvalidateICache(bp->address, 4, true); + JitInterface::InvalidateICache(bp->address, 4, true); bp = m_breakpoints.erase(bp); } else @@ -180,8 +173,8 @@ void MemChecks::Add(const TMemCheck& memory_check) m_mem_checks.push_back(memory_check); // If this is the first one, clear the JIT cache so it can switch to // watchpoint-compatible code. - if (!had_any && g_jit) - g_jit->ClearCache(); + if (!had_any) + JitInterface::ClearCache(); PowerPC::DBATUpdated(); }); } @@ -197,8 +190,8 @@ void MemChecks::Remove(u32 address) Core::RunAsCPUThread([&] { m_mem_checks.erase(iter); - if (!HasAny() && g_jit) - g_jit->ClearCache(); + if (!HasAny()) + JitInterface::ClearCache(); PowerPC::DBATUpdated(); }); }