Merge pull request #4201 from EmptyChaos/interpreter-coretiming

Interpreter/CachedInterpreter/JitArm64: Fix CoreTiming::Advance usage

4 files changed, 67 insertions, 42 deletions

diff --git a/Source/Core/Core/PowerPC/CachedInterpreter.cpp b/Source/Core/Core/PowerPC/CachedInterpreter.cpp
index 5a560d9d24..de7aa218ae 100644
--- a/Source/Core/Core/PowerPC/CachedInterpreter.cpp
+++ b/Source/Core/Core/PowerPC/CachedInterpreter.cpp
@@ -32,51 +32,59 @@ void CachedInterpreter::Shutdown()
   JitBaseBlockCache::Shutdown();
 }
 
-void CachedInterpreter::Run()
-{
-  while (!CPU::GetState())
-  {
-    SingleStep();
-  }
-}
-
-void CachedInterpreter::SingleStep()
+void CachedInterpreter::ExecuteOneBlock()
 {
-  const u8* normalEntry = jit->GetBlockCache()->Dispatch();
-  const Instruction* code = reinterpret_cast<const Instruction*>(normalEntry);
+  const u8* normal_entry = JitBaseBlockCache::Dispatch();
+  const Instruction* code = reinterpret_cast<const Instruction*>(normal_entry);
 
-  while (true)
+  for (; code->type != Instruction::INSTRUCTION_ABORT; ++code)
   {
     switch (code->type)
     {
-    case Instruction::INSTRUCTION_ABORT:
-      return;
-
     case Instruction::INSTRUCTION_TYPE_COMMON:
       code->common_callback(UGeckoInstruction(code->data));
-      code++;
       break;
 
     case Instruction::INSTRUCTION_TYPE_CONDITIONAL:
-      bool ret = code->conditional_callback(code->data);
-      code++;
-      if (ret)
+      if (code->conditional_callback(code->data))
         return;
       break;
+
+    default:
+      ERROR_LOG(POWERPC, "Unknown CachedInterpreter Instruction: %d", code->type);
+      break;
     }
   }
 }
 
-static void EndBlock(UGeckoInstruction data)
+void CachedInterpreter::Run()
 {
-  PC = NPC;
-  PowerPC::ppcState.downcount -= data.hex;
-  if (PowerPC::ppcState.downcount <= 0)
+  while (CPU::GetState() == CPU::CPU_RUNNING)
   {
+    // Start new timing slice
+    // NOTE: Exceptions may change PC
     CoreTiming::Advance();
+
+    do
+    {
+      ExecuteOneBlock();
+    } while (PowerPC::ppcState.downcount > 0);
   }
 }
 
+void CachedInterpreter::SingleStep()
+{
+  // Enter new timing slice
+  CoreTiming::Advance();
+  ExecuteOneBlock();
+}
+
+static void EndBlock(UGeckoInstruction data)
+{
+  PC = NPC;
+  PowerPC::ppcState.downcount -= data.hex;
+}
+
 static void WritePC(UGeckoInstruction data)
 {
   PC = data.hex;
diff --git a/Source/Core/Core/PowerPC/CachedInterpreter.h b/Source/Core/Core/PowerPC/CachedInterpreter.h
index f91322eb4d..3b2423e9ee 100644
--- a/Source/Core/Core/PowerPC/CachedInterpreter.h
+++ b/Source/Core/Core/PowerPC/CachedInterpreter.h
@@ -56,7 +56,8 @@ private:
   };
 
   const u8* GetCodePtr() { return (u8*)(m_code.data() + m_code.size()); }
-  std::vector<Instruction> m_code;
+  void ExecuteOneBlock();
 
+  std::vector<Instruction> m_code;
   PPCAnalyst::CodeBuffer code_buffer;
 };
diff --git a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp
index 336371914e..ec147c41bd 100644
--- a/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp
+++ b/Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp
@@ -196,11 +196,14 @@ int Interpreter::SingleStepInner()
 
 void Interpreter::SingleStep()
 {
+  // Declare start of new slice
+  CoreTiming::Advance();
+
   SingleStepInner();
 
+  // The interpreter ignores instruction timing information outside the 'fast runloop'.
   CoreTiming::g_slice_length = 1;
   PowerPC::ppcState.downcount = 0;
-  CoreTiming::Advance();
 
   if (PowerPC::ppcState.Exceptions)
   {
@@ -222,6 +225,11 @@ void Interpreter::Run()
 {
   while (!CPU::GetState())
   {
+    // CoreTiming Advance() ends the previous slice and declares the start of the next
+    // one so it must always be called at the start. At boot, we are in slice -1 and must
+    // advance into slice 0 to get a correct slice length before executing any cycles.
+    CoreTiming::Advance();
+
     // we have to check exceptions at branches apparently (or maybe just rfi?)
     if (SConfig::GetInstance().bEnableDebugging)
     {
@@ -295,8 +303,6 @@ void Interpreter::Run()
         PowerPC::ppcState.downcount -= cycles;
       }
     }
-
-    CoreTiming::Advance();
   }
 }
 
diff --git a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp
index bf6f395586..7c47e7a2b6 100644
--- a/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp
+++ b/Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp
@@ -28,18 +28,28 @@ void JitArm64::GenerateAsm()
 
   MOVP2R(PPC_REG, &PowerPC::ppcState);
 
-  // Load the current PC into DISPATCHER_PC
-  LDR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
-
-  FixupBranch to_dispatcher = B();
+  // The PC will be loaded into DISPATCHER_PC after the call to CoreTiming::Advance().
+  // Advance() does an exception check so we don't know what PC to use until afterwards.
+  FixupBranch to_start_of_timing_slice = B();
 
   // If we align the dispatcher to a page then we can load its location with one ADRP instruction
+  // do
+  // {
+  //   CoreTiming::Advance();  // <-- Checks for exceptions (changes PC)
+  //   DISPATCHER_PC = PC;
+  //   do
+  //   {
+  // dispatcherNoCheck:
+  //     ExecuteBlock(JitBase::Dispatch());
+  // dispatcher:
+  //   } while (PowerPC::ppcState.downcount > 0);
+  // doTiming:
+  //   NPC = PC = DISPATCHER_PC;
+  // } while (CPU::GetState() == CPU::CPU_RUNNING);
   AlignCodePage();
   dispatcher = GetCodePtr();
   WARN_LOG(DYNA_REC, "Dispatcher is %p\n", dispatcher);
 
-  SetJumpTarget(to_dispatcher);
-
   // Downcount Check
   // The result of slice decrementation should be in flags if somebody jumped here
   // IMPORTANT - We jump on negative, not carry!!!
@@ -119,12 +129,6 @@ void JitArm64::GenerateAsm()
   STR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
   STR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(npc));
 
-  MOVP2R(X30, &CoreTiming::Advance);
-  BLR(X30);
-
-  // Load the PC back into DISPATCHER_PC (the exception handler might have changed it)
-  LDR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
-
   // Check the state pointer to see if we are exiting
   // Gets checked on at the end of every slice
   MOVP2R(X0, CPU::GetStatePtr());
@@ -133,11 +137,17 @@ void JitArm64::GenerateAsm()
   CMP(W0, 0);
   FixupBranch Exit = B(CC_NEQ);
 
-  B(dispatcher);
+  SetJumpTarget(to_start_of_timing_slice);
+  MOVP2R(X30, &CoreTiming::Advance);
+  BLR(X30);
 
-  SetJumpTarget(Exit);
-  STR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
+  // Load the PC back into DISPATCHER_PC (the exception handler might have changed it)
+  LDR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
 
+  // We can safely assume that downcount >= 1
+  B(dispatcherNoCheck);
+
+  SetJumpTarget(Exit);
   ABI_PopRegisters(regs_to_save);
   RET(X30);