Merge pull request #4168 from EmptyChaos/coretiming-cleanup

Core: CoreTiming Cleanup (Add UnitTests)

Source/Core/Common/ChunkFile.h

@@ -51,12 +51,6 @@
 #error No version of is_trivially_copyable
 #endif
 
-template <class T>
-struct LinkedListItem : public T
-{
-  LinkedListItem<T>* next;
-};
-
 // Wrapper class
 class PointerWrap
 {
@@ -244,67 +238,6 @@ public:
     }
   }
 
-  // Let's pretend std::list doesn't exist!
-  template <class T, LinkedListItem<T>* (*TNew)(), void (*TFree)(LinkedListItem<T>*),
-            void (*TDo)(PointerWrap&, T*)>
-  void DoLinkedList(LinkedListItem<T>*& list_start, LinkedListItem<T>** list_end = 0)
-  {
-    LinkedListItem<T>* list_cur = list_start;
-    LinkedListItem<T>* prev = nullptr;
-
-    while (true)
-    {
-      u8 shouldExist = !!list_cur;
-      Do(shouldExist);
-      if (shouldExist == 1)
-      {
-        LinkedListItem<T>* cur = list_cur ? list_cur : TNew();
-        TDo(*this, (T*)cur);
-        if (!list_cur)
-        {
-          if (mode == MODE_READ)
-          {
-            cur->next = nullptr;
-            list_cur = cur;
-            if (prev)
-              prev->next = cur;
-            else
-              list_start = cur;
-          }
-          else
-          {
-            TFree(cur);
-            continue;
-          }
-        }
-      }
-      else
-      {
-        if (mode == MODE_READ)
-        {
-          if (prev)
-            prev->next = nullptr;
-          if (list_end)
-            *list_end = prev;
-          if (list_cur)
-          {
-            if (list_start == list_cur)
-              list_start = nullptr;
-            do
-            {
-              LinkedListItem<T>* next = list_cur->next;
-              TFree(list_cur);
-              list_cur = next;
-            } while (list_cur);
-          }
-        }
-        break;
-      }
-      prev = list_cur;
-      list_cur = list_cur->next;
-    }
-  }
-
   void DoMarker(const std::string& prevName, u32 arbitraryNumber = 0x42)
   {
     u32 cookie = arbitraryNumber;
@@ -319,16 +252,22 @@ public:
     }
   }
 
+  template <typename T, typename Functor>
+  void DoEachElement(T& container, Functor member)
+  {
+    u32 size = static_cast<u32>(container.size());
+    Do(size);
+    container.resize(size);
+
+    for (auto& elem : container)
+      member(*this, elem);
+  }
+
 private:
   template <typename T>
   void DoContainer(T& x)
   {
-    u32 size = (u32)x.size();
-    Do(size);
-    x.resize(size);
-
-    for (auto& elem : x)
-      Do(elem);
+    DoEachElement(x, [](PointerWrap& p, typename T::value_type& elem) { p.Do(elem); });
   }
 
   __forceinline void DoVoid(void* data, u32 size)

Source/Core/Core/CoreTiming.h

@@ -25,12 +25,14 @@ class PointerWrap;
 namespace CoreTiming
 {
 // These really shouldn't be global, but jit64 accesses them directly
-extern s64 g_globalTimer;
-extern u64 g_fakeTBStartValue;
-extern u64 g_fakeTBStartTicks;
-extern int g_slicelength;
-extern float g_lastOCFactor_inverted;
+extern s64 g_global_timer;
+extern u64 g_fake_TB_start_value;
+extern u64 g_fake_TB_start_ticks;
+extern int g_slice_length;
+extern float g_last_OC_factor_inverted;
 
+// CoreTiming begins at the boundary of timing slice -1. An initial call to Advance() is
+// required to end slice -1 and start slice 0 before the first cycle of code is executed.
 void Init();
 void Shutdown();
 
@@ -43,9 +45,11 @@ u64 GetIdleTicks();
 
 void DoState(PointerWrap& p);
 
+struct EventType;
+
 // Returns the event_type identifier. if name is not unique, an existing event_type will be
 // discarded.
-int RegisterEvent(const std::string& name, TimedCallback callback);
+EventType* RegisterEvent(const std::string& name, TimedCallback callback);
 void UnregisterAllEvents();
 
 enum class FromThread
@@ -58,12 +62,22 @@ enum class FromThread
 };
 
 // userdata MAY NOT CONTAIN POINTERS. userdata might get written and reloaded from savestates.
-void ScheduleEvent(s64 cycles_into_future, int event_type, u64 userdata = 0,
+// After the first Advance, the slice lengths and the downcount will be reduced whenever an event
+// is scheduled earlier than the current values (when scheduled from the CPU Thread only).
+// Scheduling from a callback will not update the downcount until the Advance() completes.
+void ScheduleEvent(s64 cycles_into_future, EventType* event_type, u64 userdata = 0,
                    FromThread from = FromThread::CPU);
 
 // We only permit one event of each type in the queue at a time.
-void RemoveEvent(int event_type);
-void RemoveAllEvents(int event_type);
+void RemoveEvent(EventType* event_type);
+void RemoveAllEvents(EventType* event_type);
+
+// Advance must be called at the beginning of dispatcher loops, not the end. Advance() ends
+// the previous timing slice and begins the next one, you must Advance from the previous
+// slice to the current one before executing any cycles. CoreTiming starts in slice -1 so an
+// Advance() is required to initialize the slice length before the first cycle of emulated
+// instructions is executed.
+// NOTE: Advance updates the PowerPC downcount and performs a PPC external exception check.
 void Advance();
 void MoveEvents();
 

Source/Core/Core/HW/AudioInterface.cpp

@@ -130,7 +130,7 @@ static void GenerateAudioInterrupt();
 static void UpdateInterrupts();
 static void IncreaseSampleCount(const u32 _uAmount);
 static int GetAIPeriod();
-static int et_AI;
+static CoreTiming::EventType* et_AI;
 static void Update(u64 userdata, s64 cyclesLate);
 
 void Init()

Source/Core/Core/HW/DSP.cpp

@@ -188,8 +188,8 @@ static void UpdateInterrupts();
 static void Do_ARAM_DMA();
 static void GenerateDSPInterrupt(u64 DSPIntType, s64 cyclesLate = 0);
 
-static int et_GenerateDSPInterrupt;
-static int et_CompleteARAM;
+static CoreTiming::EventType* et_GenerateDSPInterrupt;
+static CoreTiming::EventType* et_CompleteARAM;
 
 static void CompleteARAM(u64 userdata, s64 cyclesLate)
 {

Source/Core/Core/HW/DVDInterface.cpp

@@ -238,14 +238,14 @@ static u32 s_error_code = 0;
 static bool s_disc_inside = false;
 static bool s_stream = false;
 static bool s_stop_at_track_end = false;
-static int s_finish_executing_command = 0;
-static int s_dtk = 0;
+static CoreTiming::EventType* s_finish_executing_command;
+static CoreTiming::EventType* s_dtk;
 
 static u64 s_last_read_offset;
 static u64 s_last_read_time;
 
-static int s_eject_disc;
-static int s_insert_disc;
+static CoreTiming::EventType* s_eject_disc;
+static CoreTiming::EventType* s_insert_disc;
 
 static void EjectDiscCallback(u64 userdata, s64 cyclesLate);
 static void InsertDiscCallback(u64 userdata, s64 cyclesLate);

Source/Core/Core/HW/DVDThread.cpp

@@ -30,7 +30,7 @@ namespace DVDThread
 static void DVDThread();
 
 static void FinishRead(u64 userdata, s64 cycles_late);
-static int s_finish_read;
+static CoreTiming::EventType* s_finish_read;
 
 static std::thread s_dvd_thread;
 static Common::Event s_dvd_thread_start_working;

Source/Core/Core/HW/EXI.cpp

@@ -12,6 +12,7 @@
 #include "Core/CoreTiming.h"
 #include "Core/HW/EXI.h"
 #include "Core/HW/EXI_Channel.h"
+#include "Core/HW/EXI_DeviceMemoryCard.h"
 #include "Core/HW/MMIO.h"
 #include "Core/HW/ProcessorInterface.h"
 #include "Core/HW/Sram.h"
@@ -23,8 +24,8 @@ bool g_SRAM_netplay_initialized = false;
 
 namespace ExpansionInterface
 {
-static int changeDevice;
-static int updateInterrupts;
+static CoreTiming::EventType* changeDevice;
+static CoreTiming::EventType* updateInterrupts;
 
 static std::array<std::unique_ptr<CEXIChannel>, MAX_EXI_CHANNELS> g_Channels;
 
@@ -38,6 +39,7 @@ void Init()
     InitSRAM();
   }
 
+  CEXIMemoryCard::Init();
   for (u32 i = 0; i < MAX_EXI_CHANNELS; i++)
     g_Channels[i] = std::make_unique<CEXIChannel>(i);
 
@@ -65,6 +67,8 @@ void Shutdown()
 {
   for (auto& channel : g_Channels)
     channel.reset();
+
+  CEXIMemoryCard::Shutdown();
 }
 
 void DoState(PointerWrap& p)

Source/Core/Core/HW/EXI_DeviceMemoryCard.cpp

@@ -2,6 +2,7 @@
 // Licensed under GPLv2+
 // Refer to the license.txt file included.
 
+#include <array>
 #include <cstring>
 #include <memory>
 #include <string>
@@ -41,6 +42,9 @@
 static const u32 MC_TRANSFER_RATE_READ = 512 * 1024;
 static const u32 MC_TRANSFER_RATE_WRITE = (u32)(96.125f * 1024.0f);
 
+static std::array<CoreTiming::EventType*, 2> s_et_cmd_done;
+static std::array<CoreTiming::EventType*, 2> s_et_transfer_complete;
+
 // Takes care of the nasty recovery of the 'this' pointer from card_index,
 // stored in the userdata parameter of the CoreTiming event.
 void CEXIMemoryCard::EventCompleteFindInstance(u64 userdata,
@@ -70,25 +74,37 @@ void CEXIMemoryCard::TransferCompleteCallback(u64 userdata, s64 cyclesLate)
                             [](CEXIMemoryCard* instance) { instance->TransferComplete(); });
 }
 
+void CEXIMemoryCard::Init()
+{
+  static constexpr char DONE_PREFIX[] = "memcardDone";
+  static constexpr char TRANSFER_COMPLETE_PREFIX[] = "memcardTransferComplete";
+
+  static_assert(s_et_cmd_done.size() == s_et_transfer_complete.size(), "Event array size differs");
+  for (unsigned int i = 0; i < s_et_cmd_done.size(); ++i)
+  {
+    std::string name = DONE_PREFIX;
+    name += static_cast<char>('A' + i);
+    s_et_cmd_done[i] = CoreTiming::RegisterEvent(name, CmdDoneCallback);
+
+    name = TRANSFER_COMPLETE_PREFIX;
+    name += static_cast<char>('A' + i);
+    s_et_transfer_complete[i] = CoreTiming::RegisterEvent(name, TransferCompleteCallback);
+  }
+}
+
+void CEXIMemoryCard::Shutdown()
+{
+  s_et_cmd_done.fill(nullptr);
+  s_et_transfer_complete.fill(nullptr);
+}
+
 CEXIMemoryCard::CEXIMemoryCard(const int index, bool gciFolder) : card_index(index)
 {
-  struct
-  {
-    const char* done;
-    const char* transfer_complete;
-  } const event_names[] = {
-      {"memcardDoneA", "memcardTransferCompleteA"}, {"memcardDoneB", "memcardTransferCompleteB"},
-  };
+  _assert_msg_(EXPANSIONINTERFACE, static_cast<std::size_t>(index) < s_et_cmd_done.size(),
+               "Trying to create invalid memory card index %d.", index);
 
-  if ((size_t)index >= ArraySize(event_names))
-  {
-    PanicAlertT("Trying to create invalid memory card index.");
-  }
-  // we're potentially leaking events here, since there's no RemoveEvent
-  // until emu shutdown, but I guess it's inconsequential
-  et_cmd_done = CoreTiming::RegisterEvent(event_names[index].done, CmdDoneCallback);
-  et_transfer_complete =
-      CoreTiming::RegisterEvent(event_names[index].transfer_complete, TransferCompleteCallback);
+  // NOTE: When loading a save state, DMA completion callbacks (s_et_transfer_complete) and such
+  //   may have been restored, we need to anticipate those arriving.
 
   interruptSwitch = 0;
   m_bInterruptSet = 0;
@@ -248,8 +264,8 @@ void CEXIMemoryCard::SetupRawMemcard(u16 sizeMb)
 
 CEXIMemoryCard::~CEXIMemoryCard()
 {
-  CoreTiming::RemoveEvent(et_cmd_done);
-  CoreTiming::RemoveEvent(et_transfer_complete);
+  CoreTiming::RemoveEvent(s_et_cmd_done[card_index]);
+  CoreTiming::RemoveEvent(s_et_transfer_complete[card_index]);
 }
 
 bool CEXIMemoryCard::UseDelayedTransferCompletion() const
@@ -279,8 +295,8 @@ void CEXIMemoryCard::TransferComplete()
 
 void CEXIMemoryCard::CmdDoneLater(u64 cycles)
 {
-  CoreTiming::RemoveEvent(et_cmd_done);
-  CoreTiming::ScheduleEvent((int)cycles, et_cmd_done, (u64)card_index);
+  CoreTiming::RemoveEvent(s_et_cmd_done[card_index]);
+  CoreTiming::ScheduleEvent((int)cycles, s_et_cmd_done[card_index], (u64)card_index);
 }
 
 void CEXIMemoryCard::SetCS(int cs)
@@ -547,7 +563,7 @@ void CEXIMemoryCard::DMARead(u32 _uAddr, u32 _uSize)
 
   // Schedule transfer complete later based on read speed
   CoreTiming::ScheduleEvent(_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_READ),
-                            et_transfer_complete, (u64)card_index);
+                            s_et_transfer_complete[card_index], (u64)card_index);
 }
 
 // DMA write are preceded by all of the necessary setup via IMMWrite
@@ -563,5 +579,5 @@ void CEXIMemoryCard::DMAWrite(u32 _uAddr, u32 _uSize)
 
   // Schedule transfer complete later based on write speed
   CoreTiming::ScheduleEvent(_uSize * (SystemTimers::GetTicksPerSecond() / MC_TRANSFER_RATE_WRITE),
-                            et_transfer_complete, (u64)card_index);
+                            s_et_transfer_complete[card_index], (u64)card_index);
 }

Source/Core/Core/HW/EXI_DeviceMemoryCard.h

@@ -26,6 +26,12 @@ public:
   void DMARead(u32 _uAddr, u32 _uSize) override;
   void DMAWrite(u32 _uAddr, u32 _uSize) override;
 
+  // CoreTiming events need to be registered during boot since CoreTiming is DoState()-ed
+  // before ExpansionInterface so we'll lose the save stated events if the callbacks are
+  // not already registered first.
+  static void Init();
+  static void Shutdown();
+
 private:
   void SetupGciFolder(u16 sizeMb);
   void SetupRawMemcard(u16 sizeMb);
@@ -67,7 +73,6 @@ private:
   };
 
   int card_index;
-  int et_cmd_done, et_transfer_complete;
   //! memory card state
 
   // STATE_TO_SAVE

Source/Core/Core/HW/ProcessorInterface.cpp

@@ -31,10 +31,10 @@ static u32 m_FlipperRev;
 static u32 m_Unknown;
 
 // ID and callback for scheduling reset button presses/releases
-static int toggleResetButton;
+static CoreTiming::EventType* toggleResetButton;
 static void ToggleResetButtonCallback(u64 userdata, s64 cyclesLate);
 
-static int iosNotifyResetButton;
+static CoreTiming::EventType* iosNotifyResetButton;
 static void IOSNotifyResetButtonCallback(u64 userdata, s64 cyclesLate);
 
 // Let the PPC know that an external exception is set/cleared

Source/Core/Core/HW/SI.cpp

@@ -23,8 +23,8 @@
 
 namespace SerialInterface
 {
-static int changeDevice;
-static int et_transfer_pending;
+static CoreTiming::EventType* changeDevice;
+static CoreTiming::EventType* et_transfer_pending;
 
 static void RunSIBuffer(u64 userdata, s64 cyclesLate);
 static void UpdateInterrupts();

Source/Core/Core/HW/SystemTimers.cpp

@@ -64,13 +64,14 @@ IPC_HLE_PERIOD: For the Wiimote this is the call schedule:
 
 namespace SystemTimers
 {
-static int et_Dec;
-static int et_VI;
-static int et_AudioDMA;
-static int et_DSP;
-static int et_IPC_HLE;
-static int et_PatchEngine;  // PatchEngine updates every 1/60th of a second by default
-static int et_Throttle;
+static CoreTiming::EventType* et_Dec;
+static CoreTiming::EventType* et_VI;
+static CoreTiming::EventType* et_AudioDMA;
+static CoreTiming::EventType* et_DSP;
+static CoreTiming::EventType* et_IPC_HLE;
+// PatchEngine updates every 1/60th of a second by default
+static CoreTiming::EventType* et_PatchEngine;
+static CoreTiming::EventType* et_Throttle;
 
 static u32 s_cpu_core_clock = 486000000u;  // 486 mhz (its not 485, stop bugging me!)
 

Source/Core/Core/HW/WII_IPC.cpp

@@ -98,7 +98,7 @@ static u32 arm_irq_masks;
 
 static u32 sensorbar_power;  // do we need to care about this?
 
-static int updateInterrupts;
+static CoreTiming::EventType* updateInterrupts;
 static void UpdateInterrupts(u64 = 0, s64 cyclesLate = 0);
 
 void DoState(PointerWrap& p)
@@ -113,7 +113,7 @@ void DoState(PointerWrap& p)
   p.Do(sensorbar_power);
 }
 
-void Init()
+static void InitState()
 {
   ctrl = CtrlRegister();
   ppc_msg = 0;
@@ -127,14 +127,18 @@ void Init()
   sensorbar_power = 0;
 
   ppc_irq_masks |= INT_CAUSE_IPC_BROADWAY;
+}
 
+void Init()
+{
+  InitState();
   updateInterrupts = CoreTiming::RegisterEvent("IPCInterrupt", UpdateInterrupts);
 }
 
 void Reset()
 {
   INFO_LOG(WII_IPC, "Resetting ...");
-  Init();
+  InitState();
   WII_IPC_HLE_Interface::Reset();
 }
 

Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp

@@ -76,8 +76,8 @@ static ipc_msg_queue request_queue;  // ppc -> arm
 static ipc_msg_queue reply_queue;    // arm -> ppc
 static ipc_msg_queue ack_queue;      // arm -> ppc
 
-static int event_enqueue;
-static int event_sdio_notify;
+static CoreTiming::EventType* event_enqueue;
+static CoreTiming::EventType* event_sdio_notify;
 
 static u64 last_reply_time;
 

Source/Core/Core/MemoryWatcher.cpp

@@ -15,7 +15,7 @@
 #include "Core/MemoryWatcher.h"
 
 static std::unique_ptr<MemoryWatcher> s_memory_watcher;
-static int s_event;
+static CoreTiming::EventType* s_event;
 static const int MW_RATE = 600;  // Steps per second
 
 static void MWCallback(u64 userdata, s64 cyclesLate)

Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp

@@ -198,7 +198,7 @@ void Interpreter::SingleStep()
 {
   SingleStepInner();
 
-  CoreTiming::g_slicelength = 1;
+  CoreTiming::g_slice_length = 1;
   PowerPC::ppcState.downcount = 0;
   CoreTiming::Advance();
 

Source/Core/Core/PowerPC/Jit64/Jit_SystemRegisters.cpp

@@ -286,13 +286,13 @@ void Jit64::mfspr(UGeckoInstruction inst)
     // cost of calling out to C for this is actually significant.
     // Scale downcount by the CPU overclocking factor.
     CVTSI2SS(XMM0, PPCSTATE(downcount));
-    MULSS(XMM0, M(&CoreTiming::g_lastOCFactor_inverted));
+    MULSS(XMM0, M(&CoreTiming::g_last_OC_factor_inverted));
     CVTSS2SI(RDX, R(XMM0));  // RDX is downcount scaled by the overclocking factor
-    MOV(32, R(RAX), M(&CoreTiming::g_slicelength));
+    MOV(32, R(RAX), M(&CoreTiming::g_slice_length));
     SUB(64, R(RAX), R(RDX));  // cycles since the last CoreTiming::Advance() event is (slicelength -
                               // Scaled_downcount)
-    ADD(64, R(RAX), M(&CoreTiming::g_globalTimer));
-    SUB(64, R(RAX), M(&CoreTiming::g_fakeTBStartTicks));
+    ADD(64, R(RAX), M(&CoreTiming::g_global_timer));
+    SUB(64, R(RAX), M(&CoreTiming::g_fake_TB_start_ticks));
     // It might seem convenient to correct the timer for the block position here for even more
     // accurate
     // timing, but as of currently, this can break games. If we end up reading a time *after* the
@@ -308,7 +308,7 @@ void Jit64::mfspr(UGeckoInstruction inst)
     // a / 12 = (a * 0xAAAAAAAAAAAAAAAB) >> 67
     MOV(64, R(RDX), Imm64(0xAAAAAAAAAAAAAAABULL));
     MUL(64, R(RDX));
-    MOV(64, R(RAX), M(&CoreTiming::g_fakeTBStartValue));
+    MOV(64, R(RAX), M(&CoreTiming::g_fake_TB_start_value));
     SHR(64, R(RDX), Imm8(3));
     ADD(64, R(RAX), R(RDX));
     MOV(64, PPCSTATE(spr[SPR_TL]), R(RAX));

Source/Core/Core/PowerPC/JitArm64/JitArm64_SystemRegisters.cpp

@@ -234,9 +234,9 @@ void JitArm64::mfspr(UGeckoInstruction inst)
 
     // An inline implementation of CoreTiming::GetFakeTimeBase, since in timer-heavy games the
     // cost of calling out to C for this is actually significant.
-    MOVI2R(XA, (u64)&CoreTiming::g_globalTimer);
+    MOVI2R(XA, (u64)&CoreTiming::g_global_timer);
     LDR(INDEX_UNSIGNED, XA, XA, 0);
-    MOVI2R(XB, (u64)&CoreTiming::g_fakeTBStartTicks);
+    MOVI2R(XB, (u64)&CoreTiming::g_fake_TB_start_ticks);
     LDR(INDEX_UNSIGNED, XB, XB, 0);
     SUB(XA, XA, XB);
 
@@ -254,7 +254,7 @@ void JitArm64::mfspr(UGeckoInstruction inst)
     ADD(XB, XB, 1);
     UMULH(XA, XA, XB);
 
-    MOVI2R(XB, (u64)&CoreTiming::g_fakeTBStartValue);
+    MOVI2R(XB, (u64)&CoreTiming::g_fake_TB_start_value);
     LDR(INDEX_UNSIGNED, XB, XB, 0);
     ADD(XA, XB, XA, ArithOption(XA, ST_LSR, 3));
     STR(INDEX_UNSIGNED, XA, PPC_REG, PPCSTATE_OFF(spr[SPR_TL]));

Source/Core/Core/PowerPC/PowerPC.cpp

@@ -36,7 +36,7 @@ BreakPoints breakpoints;
 MemChecks memchecks;
 PPCDebugInterface debug_interface;
 
-static int s_invalidate_cache_thread_safe;
+static CoreTiming::EventType* s_invalidate_cache_thread_safe;
 static void InvalidateCacheThreadSafe(u64 userdata, s64 cyclesLate)
 {
   ppcState.iCache.Invalidate(static_cast<u32>(userdata));