Merge pull request #8511 from lioncash/if-constexpr

VideoCommon/OpcodeDecoding: Make use of if constexpr

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"
@@ -341,7 +342,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();
@@ -563,7 +564,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();

Source/Core/VideoCommon/OpcodeDecoding.cpp

@@ -9,15 +9,14 @@
 //  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
+// are while interpreting them, and hope that the vertex format doesn't change, though, if you do
-// while interpreting them, and hope that the vertex format doesn't change, though, if you do it
+// it right when they are called. The reason is that the vertex format affects the sizes of the
-// right
+// vertices.
-// 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"
 #include "Common/Logging/Log.h"
-#include "Common/MsgHandler.h"
 #include "Core/FifoPlayer/FifoRecorder.h"
 #include "Core/HW/Memmap.h"
 #include "VideoCommon/BPMemory.h"
@@ -27,33 +26,32 @@
 #include "VideoCommon/Fifo.h"
 #include "VideoCommon/Statistics.h"
 #include "VideoCommon/VertexLoaderManager.h"
-#include "VideoCommon/VideoCommon.h"
 #include "VideoCommon/XFMemory.h"
 
-bool g_bRecordFifoData = false;
-
 namespace OpcodeDecoder
 {
-static bool s_bFifoErrorSeen = false;
+namespace
-
-static u32 InterpretDisplayList(u32 address, u32 size)
 {
-  u8* startAddress;
+bool s_is_fifo_error_seen = false;
+
+u32 InterpretDisplayList(u32 address, u32 size)
+{
+  u8* start_address;
 
   if (Fifo::UseDeterministicGPUThread())
-    startAddress = (u8*)Fifo::PopFifoAuxBuffer(size);
+    start_address = static_cast<u8*>(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
@@ -63,57 +61,70 @@ static u32 InterpretDisplayList(u32 address, u32 size)
   return cycles;
 }
 
-static void InterpretDisplayListPreprocess(u32 address, u32 size)
+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)
+  if (start_address == nullptr)
-  {
+    return;
-    Run<true>(DataReader(startAddress, startAddress + size), nullptr, true);
+
-  }
+  Run<true>(DataReader(start_address, start_address + size), nullptr, true);
 }
+}  // Anonymous namespace
+
+bool g_record_fifo_data = false;
 
 void Init()
 {
-  s_bFifoErrorSeen = false;
+  s_is_fifo_error_seen = false;
 }
 
 template <bool is_preprocess>
 u8* Run(DataReader src, u32* cycles, bool in_display_list)
 {
-  u32 totalCycles = 0;
+  u32 total_cycles = 0;
-  u8* opcodeStart;
+  u8* opcode_start = nullptr;
+
+  const auto finish_up = [cycles, &opcode_start, &total_cycles] {
+    if (cycles != nullptr)
+    {
+      *cycles = total_cycles;
+    }
+    return opcode_start;
+  };
+
   while (true)
   {
-    opcodeStart = src.GetPointer();
+    opcode_start = src.GetPointer();
 
     if (!src.size())
-      goto end;
+      return finish_up();
 
-    u8 cmd_byte = src.Read<u8>();
+    const u8 cmd_byte = src.Read<u8>();
-    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;
 
     case GX_LOAD_CP_REG:
     {
       if (src.size() < 1 + 4)
-        goto end;
+        return finish_up();
-      totalCycles += 12;
+
-      u8 sub_cmd = src.Read<u8>();
+      total_cycles += 12;
-      u32 value = src.Read<u32>();
+
+      const u8 sub_cmd = src.Read<u8>();
+      const u32 value = src.Read<u32>();
       LoadCPReg(sub_cmd, value, is_preprocess);
-      if (!is_preprocess)
+      if constexpr (!is_preprocess)
         INCSTAT(g_stats.this_frame.num_cp_loads);
     }
     break;
@@ -121,15 +132,18 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list)
     case GX_LOAD_XF_REG:
     {
       if (src.size() < 4)
-        goto end;
+        return finish_up();
-      u32 Cmd2 = src.Read<u32>();
+
-      int transfer_size = ((Cmd2 >> 16) & 15) + 1;
+      const u32 cmd2 = src.Read<u32>();
+      const u32 transfer_size = ((cmd2 >> 16) & 15) + 1;
       if (src.size() < transfer_size * sizeof(u32))
-        goto end;
+        return finish_up();
-      totalCycles += 18 + 6 * transfer_size;
+
-      if (!is_preprocess)
+      total_cycles += 18 + 6 * transfer_size;
+
+      if constexpr (!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);
@@ -138,58 +152,61 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list)
     }
     break;
 
-    case GX_LOAD_INDX_A:  // used for position matrices
+    case GX_LOAD_INDX_A:  // Used for position matrices
-      refarray = 0xC;
+    case GX_LOAD_INDX_B:  // Used for normal matrices
-      goto load_indx;
+    case GX_LOAD_INDX_C:  // Used for postmatrices
-    case GX_LOAD_INDX_B:  // used for normal matrices
+    case GX_LOAD_INDX_D:  // Used for lights
-      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:
       if (src.size() < 4)
-        goto end;
+        return finish_up();
-      totalCycles += 6;
+
-      if (is_preprocess)
+      total_cycles += 6;
-        PreprocessIndexedXF(src.Read<u32>(), refarray);
+
+      // 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 constexpr (is_preprocess)
+        PreprocessIndexedXF(src.Read<u32>(), ref_array);
       else
-        LoadIndexedXF(src.Read<u32>(), refarray);
+        LoadIndexedXF(src.Read<u32>(), ref_array);
-      break;
+    }
+    break;
 
     case GX_CMD_CALL_DL:
     {
       if (src.size() < 8)
-        goto end;
+        return finish_up();
-      u32 address = src.Read<u32>();
+
-      u32 count = src.Read<u32>();
+      const u32 address = src.Read<u32>();
+      const u32 count = src.Read<u32>();
 
       if (in_display_list)
       {
-        totalCycles += 6;
+        total_cycles += 6;
         INFO_LOG(VIDEO, "recursive display list detected");
       }
       else
       {
-        if (is_preprocess)
+        if constexpr (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;
 
@@ -198,10 +215,12 @@ 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();
-        totalCycles += 12;
+
-        u32 bp_cmd = src.Read<u32>();
+        total_cycles += 12;
-        if (is_preprocess)
+
+        const u32 bp_cmd = src.Read<u32>();
+        if constexpr (is_preprocess)
         {
           LoadBPRegPreprocess(bp_cmd);
         }
@@ -219,47 +238,43 @@ u8* Run(DataReader src, u32* cycles, bool in_display_list)
       {
         // load vertices
         if (src.size() < 2)
-          goto end;
+          return finish_up();
-        u16 num_vertices = src.Read<u16>();
+
-        int bytes = VertexLoaderManager::RunVertices(
+        const u16 num_vertices = src.Read<u16>();
+        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);
 
         if (bytes < 0)
-          goto end;
+          return finish_up();
 
         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)
+        if (!s_is_fifo_error_seen)
-          CommandProcessor::HandleUnknownOpcode(cmd_byte, opcodeStart, is_preprocess);
+          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");
+                  opcode_start, is_preprocess ? "yes" : "no");
-        s_bFifoErrorSeen = true;
+        s_is_fifo_error_seen = true;
-        totalCycles += 1;
+        total_cycles += 1;
       }
       break;
     }
 
     // Display lists get added directly into the FIFO stream
-    if (!is_preprocess && g_bRecordFifoData && cmd_byte != GX_CMD_CALL_DL)
+    if constexpr (!is_preprocess)
     {
-      u8* opcodeEnd;
+      if (g_record_fifo_data && cmd_byte != GX_CMD_CALL_DL)
-      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;
-  }
-  return opcodeStart;
 }
 
 template u8* Run<true>(DataReader src, u32* cycles, bool in_display_list);

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,

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<int, int> Renderer::CalculateOutputDimensions(int width, int height)
 
 void Renderer::CheckFifoRecording()
 {
-  bool wasRecording = g_bRecordFifoData;
+  const bool was_recording = OpcodeDecoder::g_record_fifo_data;
-  g_bRecordFifoData = FifoRecorder::GetInstance().IsRecording();
+  OpcodeDecoder::g_record_fifo_data = FifoRecorder::GetInstance().IsRecording();
 
-  if (g_bRecordFifoData)
+  if (!OpcodeDecoder::g_record_fifo_data)
+    return;
+
+  if (!was_recording)
   {
-    if (!wasRecording)
+    RecordVideoMemory();
-    {
-      RecordVideoMemory();
-    }
-
-    FifoRecorder::GetInstance().EndFrame(CommandProcessor::fifo.CPBase,
-                                         CommandProcessor::fifo.CPEnd);
   }
+
+  FifoRecorder::GetInstance().EndFrame(CommandProcessor::fifo.CPBase, CommandProcessor::fifo.CPEnd);
 }
 
 void Renderer::RecordVideoMemory()

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;

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;