From d8063e9c543b34ddd646556cf7148c69bffbd816 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:05:00 -0500 Subject: [PATCH 1/8] VideoCommon/OpcodeDecoding: Normalize variable naming Provides consistent naming of variables within the translation unit. While we're at it, we can mark them const where applicable. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 104 +++++++++++---------- 1 file changed, 56 insertions(+), 48 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index 2e40b6d863..39bb0a5765 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -34,26 +34,26 @@ bool g_bRecordFifoData = false; namespace OpcodeDecoder { -static bool s_bFifoErrorSeen = false; +static bool s_is_fifo_error_seen = false; static u32 InterpretDisplayList(u32 address, u32 size) { - u8* startAddress; + u8* start_address; if (Fifo::UseDeterministicGPUThread()) - startAddress = (u8*)Fifo::PopFifoAuxBuffer(size); + start_address = static_cast(Fifo::PopFifoAuxBuffer(size)); else - startAddress = Memory::GetPointer(address); + start_address = Memory::GetPointer(address); u32 cycles = 0; // Avoid the crash if Memory::GetPointer failed .. - if (startAddress != nullptr) + if (start_address != nullptr) { // temporarily swap dl and non-dl (small "hack" for the stats) g_stats.SwapDL(); - Run(DataReader(startAddress, startAddress + size), &cycles, true); + Run(DataReader(start_address, start_address + size), &cycles, true); INCSTAT(g_stats.this_frame.num_dlists_called); // un-swap @@ -65,43 +65,43 @@ static u32 InterpretDisplayList(u32 address, u32 size) static void InterpretDisplayListPreprocess(u32 address, u32 size) { - u8* startAddress = Memory::GetPointer(address); + u8* const start_address = Memory::GetPointer(address); - Fifo::PushFifoAuxBuffer(startAddress, size); + Fifo::PushFifoAuxBuffer(start_address, size); - if (startAddress != nullptr) - { - Run(DataReader(startAddress, startAddress + size), nullptr, true); - } + if (start_address == nullptr) + return; + + Run(DataReader(start_address, start_address + size), nullptr, true); } void Init() { - s_bFifoErrorSeen = false; + s_is_fifo_error_seen = false; } template u8* Run(DataReader src, u32* cycles, bool in_display_list) { - u32 totalCycles = 0; - u8* opcodeStart; + u32 total_cycles = 0; + u8* opcode_start; while (true) { - opcodeStart = src.GetPointer(); + opcode_start = src.GetPointer(); if (!src.size()) goto end; - u8 cmd_byte = src.Read(); + const u8 cmd_byte = src.Read(); int refarray; switch (cmd_byte) { case GX_NOP: - totalCycles += 6; // Hm, this means that we scan over nop streams pretty slowly... + total_cycles += 6; // Hm, this means that we scan over nop streams pretty slowly... break; case GX_UNKNOWN_RESET: - totalCycles += 6; // Datel software uses this command + total_cycles += 6; // Datel software uses this command DEBUG_LOG(VIDEO, "GX Reset?: %08x", cmd_byte); break; @@ -109,9 +109,11 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) { if (src.size() < 1 + 4) goto end; - totalCycles += 12; - u8 sub_cmd = src.Read(); - u32 value = src.Read(); + + total_cycles += 12; + + const u8 sub_cmd = src.Read(); + const u32 value = src.Read(); LoadCPReg(sub_cmd, value, is_preprocess); if (!is_preprocess) INCSTAT(g_stats.this_frame.num_cp_loads); @@ -122,14 +124,17 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) { if (src.size() < 4) goto end; - u32 Cmd2 = src.Read(); - int transfer_size = ((Cmd2 >> 16) & 15) + 1; + + const u32 cmd2 = src.Read(); + const int transfer_size = ((cmd2 >> 16) & 15) + 1; if (src.size() < transfer_size * sizeof(u32)) goto end; - totalCycles += 18 + 6 * transfer_size; + + total_cycles += 18 + 6 * transfer_size; + if (!is_preprocess) { - u32 xf_address = Cmd2 & 0xFFFF; + const u32 xf_address = cmd2 & 0xFFFF; LoadXFReg(transfer_size, xf_address, src); INCSTAT(g_stats.this_frame.num_xf_loads); @@ -153,7 +158,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) load_indx: if (src.size() < 4) goto end; - totalCycles += 6; + total_cycles += 6; if (is_preprocess) PreprocessIndexedXF(src.Read(), refarray); else @@ -164,12 +169,13 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) { if (src.size() < 8) goto end; - u32 address = src.Read(); - u32 count = src.Read(); + + const u32 address = src.Read(); + const u32 count = src.Read(); if (in_display_list) { - totalCycles += 6; + total_cycles += 6; INFO_LOG(VIDEO, "recursive display list detected"); } else @@ -177,19 +183,19 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) if (is_preprocess) InterpretDisplayListPreprocess(address, count); else - totalCycles += 6 + InterpretDisplayList(address, count); + total_cycles += 6 + InterpretDisplayList(address, count); } } break; case GX_CMD_UNKNOWN_METRICS: // zelda 4 swords calls it and checks the metrics registers after // that - totalCycles += 6; + total_cycles += 6; DEBUG_LOG(VIDEO, "GX 0x44: %08x", cmd_byte); break; case GX_CMD_INVL_VC: // Invalidate Vertex Cache - totalCycles += 6; + total_cycles += 6; DEBUG_LOG(VIDEO, "Invalidate (vertex cache?)"); break; @@ -199,8 +205,10 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) { if (src.size() < 4) goto end; - totalCycles += 12; - u32 bp_cmd = src.Read(); + + total_cycles += 12; + + const u32 bp_cmd = src.Read(); if (is_preprocess) { LoadBPRegPreprocess(bp_cmd); @@ -220,8 +228,9 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) // load vertices if (src.size() < 2) goto end; - u16 num_vertices = src.Read(); - int bytes = VertexLoaderManager::RunVertices( + + const u16 num_vertices = src.Read(); + const int bytes = VertexLoaderManager::RunVertices( cmd_byte & GX_VAT_MASK, // Vertex loader index (0 - 7) (cmd_byte & GX_PRIMITIVE_MASK) >> GX_PRIMITIVE_SHIFT, num_vertices, src, is_preprocess); @@ -231,16 +240,16 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) src.Skip(bytes); // 4 GPU ticks per vertex, 3 CPU ticks per GPU tick - totalCycles += num_vertices * 4 * 3 + 6; + total_cycles += num_vertices * 4 * 3 + 6; } else { - if (!s_bFifoErrorSeen) - CommandProcessor::HandleUnknownOpcode(cmd_byte, opcodeStart, is_preprocess); + if (!s_is_fifo_error_seen) + CommandProcessor::HandleUnknownOpcode(cmd_byte, opcode_start, is_preprocess); ERROR_LOG(VIDEO, "FIFO: Unknown Opcode(0x%02x @ %p, preprocessing = %s)", cmd_byte, - opcodeStart, is_preprocess ? "yes" : "no"); - s_bFifoErrorSeen = true; - totalCycles += 1; + opcode_start, is_preprocess ? "yes" : "no"); + s_is_fifo_error_seen = true; + total_cycles += 1; } break; } @@ -248,18 +257,17 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) // Display lists get added directly into the FIFO stream if (!is_preprocess && g_bRecordFifoData && cmd_byte != GX_CMD_CALL_DL) { - u8* opcodeEnd; - opcodeEnd = src.GetPointer(); - FifoRecorder::GetInstance().WriteGPCommand(opcodeStart, u32(opcodeEnd - opcodeStart)); + const u8* const opcode_end = src.GetPointer(); + FifoRecorder::GetInstance().WriteGPCommand(opcode_start, u32(opcode_end - opcode_start)); } } end: if (cycles) { - *cycles = totalCycles; + *cycles = total_cycles; } - return opcodeStart; + return opcode_start; } template u8* Run(DataReader src, u32* cycles, bool in_display_list); From b2a9c365011a501ead2b5505d219f9c817b6b7e9 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:11:52 -0500 Subject: [PATCH 2/8] VideoCommon/OpcodeDecoding: Move g_bRecordFifoData into namespace Keeps the global localized with the code that it's primarily related to. Now it's obvious from a glance what the global variable is affecting. --- Source/Core/VideoCommon/BPStructs.cpp | 5 +++-- Source/Core/VideoCommon/OpcodeDecoding.cpp | 6 +++--- Source/Core/VideoCommon/OpcodeDecoding.h | 3 +++ Source/Core/VideoCommon/RenderBase.cpp | 20 ++++++++++---------- Source/Core/VideoCommon/TextureCacheBase.cpp | 7 +++++-- Source/Core/VideoCommon/VideoCommon.h | 3 --- 6 files changed, 24 insertions(+), 20 deletions(-) diff --git a/Source/Core/VideoCommon/BPStructs.cpp b/Source/Core/VideoCommon/BPStructs.cpp index f9be647114..4a45ba7c52 100644 --- a/Source/Core/VideoCommon/BPStructs.cpp +++ b/Source/Core/VideoCommon/BPStructs.cpp @@ -25,6 +25,7 @@ #include "VideoCommon/Fifo.h" #include "VideoCommon/FramebufferManager.h" #include "VideoCommon/GeometryShaderManager.h" +#include "VideoCommon/OpcodeDecoding.h" #include "VideoCommon/PerfQueryBase.h" #include "VideoCommon/PixelEngine.h" #include "VideoCommon/PixelShaderManager.h" @@ -343,7 +344,7 @@ static void BPWritten(const BPCmd& bp) Memory::CopyFromEmu(texMem + tlutTMemAddr, addr, tlutXferCount); - if (g_bRecordFifoData) + if (OpcodeDecoder::g_record_fifo_data) FifoRecorder::GetInstance().UseMemory(addr, tlutXferCount, MemoryUpdate::TMEM); TextureCacheBase::InvalidateAllBindPoints(); @@ -566,7 +567,7 @@ static void BPWritten(const BPCmd& bp) } } - if (g_bRecordFifoData) + if (OpcodeDecoder::g_record_fifo_data) FifoRecorder::GetInstance().UseMemory(src_addr, bytes_read, MemoryUpdate::TMEM); TextureCacheBase::InvalidateAllBindPoints(); diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index 39bb0a5765..e52bfe0137 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -30,8 +30,6 @@ #include "VideoCommon/VideoCommon.h" #include "VideoCommon/XFMemory.h" -bool g_bRecordFifoData = false; - namespace OpcodeDecoder { static bool s_is_fifo_error_seen = false; @@ -75,6 +73,8 @@ static void InterpretDisplayListPreprocess(u32 address, u32 size) Run(DataReader(start_address, start_address + size), nullptr, true); } +bool g_record_fifo_data = false; + void Init() { s_is_fifo_error_seen = false; @@ -255,7 +255,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) } // Display lists get added directly into the FIFO stream - if (!is_preprocess && g_bRecordFifoData && cmd_byte != GX_CMD_CALL_DL) + if (!is_preprocess && g_record_fifo_data && cmd_byte != GX_CMD_CALL_DL) { const u8* const opcode_end = src.GetPointer(); FifoRecorder::GetInstance().WriteGPCommand(opcode_start, u32(opcode_end - opcode_start)); diff --git a/Source/Core/VideoCommon/OpcodeDecoding.h b/Source/Core/VideoCommon/OpcodeDecoding.h index 3d2e5c8361..f3533c3735 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.h +++ b/Source/Core/VideoCommon/OpcodeDecoding.h @@ -10,6 +10,9 @@ class DataReader; namespace OpcodeDecoder { +// Global flag to signal if FifoRecorder is active. +extern bool g_record_fifo_data; + enum { GX_NOP = 0x00, diff --git a/Source/Core/VideoCommon/RenderBase.cpp b/Source/Core/VideoCommon/RenderBase.cpp index 1c193f9fdc..d94eac2c0d 100644 --- a/Source/Core/VideoCommon/RenderBase.cpp +++ b/Source/Core/VideoCommon/RenderBase.cpp @@ -65,6 +65,7 @@ #include "VideoCommon/NetPlayChatUI.h" #include "VideoCommon/NetPlayGolfUI.h" #include "VideoCommon/OnScreenDisplay.h" +#include "VideoCommon/OpcodeDecoding.h" #include "VideoCommon/PixelEngine.h" #include "VideoCommon/PixelShaderManager.h" #include "VideoCommon/PostProcessing.h" @@ -880,19 +881,18 @@ std::tuple Renderer::CalculateOutputDimensions(int width, int height) void Renderer::CheckFifoRecording() { - bool wasRecording = g_bRecordFifoData; - g_bRecordFifoData = FifoRecorder::GetInstance().IsRecording(); + const bool was_recording = OpcodeDecoder::g_record_fifo_data; + OpcodeDecoder::g_record_fifo_data = FifoRecorder::GetInstance().IsRecording(); - if (g_bRecordFifoData) + if (!OpcodeDecoder::g_record_fifo_data) + return; + + if (!was_recording) { - if (!wasRecording) - { - RecordVideoMemory(); - } - - FifoRecorder::GetInstance().EndFrame(CommandProcessor::fifo.CPBase, - CommandProcessor::fifo.CPEnd); + RecordVideoMemory(); } + + FifoRecorder::GetInstance().EndFrame(CommandProcessor::fifo.CPBase, CommandProcessor::fifo.CPEnd); } void Renderer::RecordVideoMemory() diff --git a/Source/Core/VideoCommon/TextureCacheBase.cpp b/Source/Core/VideoCommon/TextureCacheBase.cpp index f38745a318..c68ed147c2 100644 --- a/Source/Core/VideoCommon/TextureCacheBase.cpp +++ b/Source/Core/VideoCommon/TextureCacheBase.cpp @@ -38,6 +38,7 @@ #include "VideoCommon/BPMemory.h" #include "VideoCommon/FramebufferManager.h" #include "VideoCommon/HiresTextures.h" +#include "VideoCommon/OpcodeDecoding.h" #include "VideoCommon/PixelShaderManager.h" #include "VideoCommon/RenderBase.h" #include "VideoCommon/SamplerCommon.h" @@ -1260,9 +1261,11 @@ TextureCacheBase::GetTexture(u32 address, u32 width, u32 height, const TextureFo // If we are recording a FifoLog, keep track of what memory we read. FifoRecorder does // its own memory modification tracking independent of the texture hashing below. - if (g_bRecordFifoData && !from_tmem) + if (OpcodeDecoder::g_record_fifo_data && !from_tmem) + { FifoRecorder::GetInstance().UseMemory(address, texture_size + additional_mips_size, MemoryUpdate::TEXTURE_MAP); + } // TODO: This doesn't hash GB tiles for preloaded RGBA8 textures (instead, it's hashing more data // from the low tmem bank than it should) @@ -2294,7 +2297,7 @@ void TextureCacheBase::CopyRenderTargetToTexture( ++iter.first; } - if (g_bRecordFifoData) + if (OpcodeDecoder::g_record_fifo_data) { // Mark the memory behind this efb copy as dynamicly generated for the Fifo log u32 address = dstAddr; diff --git a/Source/Core/VideoCommon/VideoCommon.h b/Source/Core/VideoCommon/VideoCommon.h index 5c4b1b3615..f3d4251690 100644 --- a/Source/Core/VideoCommon/VideoCommon.h +++ b/Source/Core/VideoCommon/VideoCommon.h @@ -6,9 +6,6 @@ #include "Common/CommonTypes.h" -// Global flag to signal if FifoRecorder is active. -extern bool g_bRecordFifoData; - // These are accurate (disregarding AA modes). constexpr u32 EFB_WIDTH = 640; constexpr u32 EFB_HEIGHT = 528; From 6b4e3409959ae478ee720ebd5357e4d02ab9f29b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:18:12 -0500 Subject: [PATCH 3/8] VideoCommon/OpcodeDecoding: Amend comment formatting Amends a documentation comment that acquired some wonky formatting during the introduction of clang-format a few years ago. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index e52bfe0137..8e5d2fa663 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -9,10 +9,9 @@ // Super Mario Galaxy has nearly all geometry and more than half of the state in DLs (great!) // Note that it IS NOT GENERALLY POSSIBLE to precompile display lists! You can compile them as they -// are -// while interpreting them, and hope that the vertex format doesn't change, though, if you do it -// right -// when they are called. The reason is that the vertex format affects the sizes of the vertices. +// are while interpreting them, and hope that the vertex format doesn't change, though, if you do +// it right when they are called. The reason is that the vertex format affects the sizes of the +// vertices. #include "VideoCommon/OpcodeDecoding.h" #include "Common/CommonTypes.h" From f74503cce01c3c10e0c85b6b1bd588a3a7059ca7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:20:34 -0500 Subject: [PATCH 4/8] VideoCommon/OpcodeDecoding: Make use of anonymous namespace Provides a region for all internal utilities. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index 8e5d2fa663..fe4bba007d 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -31,9 +31,11 @@ namespace OpcodeDecoder { -static bool s_is_fifo_error_seen = false; +namespace +{ +bool s_is_fifo_error_seen = false; -static u32 InterpretDisplayList(u32 address, u32 size) +u32 InterpretDisplayList(u32 address, u32 size) { u8* start_address; @@ -60,7 +62,7 @@ static u32 InterpretDisplayList(u32 address, u32 size) return cycles; } -static void InterpretDisplayListPreprocess(u32 address, u32 size) +void InterpretDisplayListPreprocess(u32 address, u32 size) { u8* const start_address = Memory::GetPointer(address); @@ -71,6 +73,7 @@ static void InterpretDisplayListPreprocess(u32 address, u32 size) Run(DataReader(start_address, start_address + size), nullptr, true); } +} // Anonymous namespace bool g_record_fifo_data = false; From 4710b82f43eb9b58db0b57566bcf88e4d1773396 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:24:47 -0500 Subject: [PATCH 5/8] VideoCommon/OpcodeDecoding: Remove use of goto in Run() With the use of a lambda and a change in switch fallthrough, we can completely eliminate the use of goto within Run(). --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 71 +++++++++++----------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index fe4bba007d..8aeb385977 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -86,16 +86,24 @@ template u8* Run(DataReader src, u32* cycles, bool in_display_list) { u32 total_cycles = 0; - u8* opcode_start; + u8* opcode_start = nullptr; + + const auto finish_up = [cycles, &opcode_start, &total_cycles] { + if (cycles != nullptr) + { + *cycles = total_cycles; + } + return opcode_start; + }; + while (true) { opcode_start = src.GetPointer(); if (!src.size()) - goto end; + return finish_up(); const u8 cmd_byte = src.Read(); - int refarray; switch (cmd_byte) { case GX_NOP: @@ -110,7 +118,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) case GX_LOAD_CP_REG: { if (src.size() < 1 + 4) - goto end; + return finish_up(); total_cycles += 12; @@ -125,12 +133,12 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) case GX_LOAD_XF_REG: { if (src.size() < 4) - goto end; + return finish_up(); const u32 cmd2 = src.Read(); const int transfer_size = ((cmd2 >> 16) & 15) + 1; if (src.size() < transfer_size * sizeof(u32)) - goto end; + return finish_up(); total_cycles += 18 + 6 * transfer_size; @@ -145,32 +153,34 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) } break; - case GX_LOAD_INDX_A: // used for position matrices - refarray = 0xC; - goto load_indx; - case GX_LOAD_INDX_B: // used for normal matrices - refarray = 0xD; - goto load_indx; - case GX_LOAD_INDX_C: // used for postmatrices - refarray = 0xE; - goto load_indx; - case GX_LOAD_INDX_D: // used for lights - refarray = 0xF; - goto load_indx; - load_indx: + case GX_LOAD_INDX_A: // Used for position matrices + case GX_LOAD_INDX_B: // Used for normal matrices + case GX_LOAD_INDX_C: // Used for postmatrices + case GX_LOAD_INDX_D: // Used for lights + { if (src.size() < 4) - goto end; + return finish_up(); + total_cycles += 6; + + // Map the command byte to its ref array. + // GX_LOAD_INDX_A (32) -> 0xC + // GX_LOAD_INDX_B (40) -> 0xD + // GX_LOAD_INDX_C (48) -> 0xE + // GX_LOAD_INDX_D (56) -> 0xF + const int ref_array = (cmd_byte / 8) + 8; + if (is_preprocess) - PreprocessIndexedXF(src.Read(), refarray); + PreprocessIndexedXF(src.Read(), ref_array); else - LoadIndexedXF(src.Read(), refarray); - break; + LoadIndexedXF(src.Read(), ref_array); + } + break; case GX_CMD_CALL_DL: { if (src.size() < 8) - goto end; + return finish_up(); const u32 address = src.Read(); const u32 count = src.Read(); @@ -206,7 +216,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) // tokens and stuff. TODO: Call a much simplified LoadBPReg instead. { if (src.size() < 4) - goto end; + return finish_up(); total_cycles += 12; @@ -229,7 +239,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) { // load vertices if (src.size() < 2) - goto end; + return finish_up(); const u16 num_vertices = src.Read(); const int bytes = VertexLoaderManager::RunVertices( @@ -237,7 +247,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) (cmd_byte & GX_PRIMITIVE_MASK) >> GX_PRIMITIVE_SHIFT, num_vertices, src, is_preprocess); if (bytes < 0) - goto end; + return finish_up(); src.Skip(bytes); @@ -263,13 +273,6 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) FifoRecorder::GetInstance().WriteGPCommand(opcode_start, u32(opcode_end - opcode_start)); } } - -end: - if (cycles) - { - *cycles = total_cycles; - } - return opcode_start; } template u8* Run(DataReader src, u32* cycles, bool in_display_list); From 6339a5ea8ea8920c1c7f1076b687512013da1457 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:43:34 -0500 Subject: [PATCH 6/8] VideoCommon/OpcodeDecoding: Resolve implicit signedness conversion cmd2 is a u32, so any bitwise arithmetic on it with a type of the same size or smaller will result in a u32 value. This is also implicitly converted to an unsigned type in the if statement as well, given that size_t * int -> size_t. This is just more explicit about the operations occurring and also likely silences a sign conversion warning. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index 8aeb385977..ab904e6a8c 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -136,7 +136,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) return finish_up(); const u32 cmd2 = src.Read(); - const int transfer_size = ((cmd2 >> 16) & 15) + 1; + const u32 transfer_size = ((cmd2 >> 16) & 15) + 1; if (src.size() < transfer_size * sizeof(u32)) return finish_up(); From 99353c3baa665cd7fcc2465a0c70a4e03bb3389b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:47:22 -0500 Subject: [PATCH 7/8] VideoCommon/OpcodeDecoding: Remove unused headers Nothing provided by these headers are used, so we can remove them. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index ab904e6a8c..1b6e8068d5 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -14,9 +14,9 @@ // vertices. #include "VideoCommon/OpcodeDecoding.h" + #include "Common/CommonTypes.h" #include "Common/Logging/Log.h" -#include "Common/MsgHandler.h" #include "Core/FifoPlayer/FifoRecorder.h" #include "Core/HW/Memmap.h" #include "VideoCommon/BPMemory.h" @@ -26,7 +26,6 @@ #include "VideoCommon/Fifo.h" #include "VideoCommon/Statistics.h" #include "VideoCommon/VertexLoaderManager.h" -#include "VideoCommon/VideoCommon.h" #include "VideoCommon/XFMemory.h" namespace OpcodeDecoder From 1f46a6a64bfaade4550276879b475511fd11f136 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 5 Dec 2019 08:49:05 -0500 Subject: [PATCH 8/8] VideoCommon/OpcodeDecoding: Make use of if constexpr We can make use of if constexpr in several scenarios here to allow compilers to exise the relevant code paths out. Technically a decent compiler would do this already, but now we can give compilers a little more nudging here in the event that isn't the case. --- Source/Core/VideoCommon/OpcodeDecoding.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Source/Core/VideoCommon/OpcodeDecoding.cpp b/Source/Core/VideoCommon/OpcodeDecoding.cpp index 1b6e8068d5..69e5f1ea01 100644 --- a/Source/Core/VideoCommon/OpcodeDecoding.cpp +++ b/Source/Core/VideoCommon/OpcodeDecoding.cpp @@ -124,7 +124,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) const u8 sub_cmd = src.Read(); const u32 value = src.Read(); LoadCPReg(sub_cmd, value, is_preprocess); - if (!is_preprocess) + if constexpr (!is_preprocess) INCSTAT(g_stats.this_frame.num_cp_loads); } break; @@ -141,7 +141,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) total_cycles += 18 + 6 * transfer_size; - if (!is_preprocess) + if constexpr (!is_preprocess) { const u32 xf_address = cmd2 & 0xFFFF; LoadXFReg(transfer_size, xf_address, src); @@ -169,7 +169,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) // GX_LOAD_INDX_D (56) -> 0xF const int ref_array = (cmd_byte / 8) + 8; - if (is_preprocess) + if constexpr (is_preprocess) PreprocessIndexedXF(src.Read(), ref_array); else LoadIndexedXF(src.Read(), ref_array); @@ -191,7 +191,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) } else { - if (is_preprocess) + if constexpr (is_preprocess) InterpretDisplayListPreprocess(address, count); else total_cycles += 6 + InterpretDisplayList(address, count); @@ -220,7 +220,7 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) total_cycles += 12; const u32 bp_cmd = src.Read(); - if (is_preprocess) + if constexpr (is_preprocess) { LoadBPRegPreprocess(bp_cmd); } @@ -266,10 +266,13 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list) } // Display lists get added directly into the FIFO stream - if (!is_preprocess && g_record_fifo_data && cmd_byte != GX_CMD_CALL_DL) + if constexpr (!is_preprocess) { - const u8* const opcode_end = src.GetPointer(); - FifoRecorder::GetInstance().WriteGPCommand(opcode_start, u32(opcode_end - opcode_start)); + if (g_record_fifo_data && cmd_byte != GX_CMD_CALL_DL) + { + const u8* const opcode_end = src.GetPointer(); + FifoRecorder::GetInstance().WriteGPCommand(opcode_start, u32(opcode_end - opcode_start)); + } } } }