Merge pull request #4201 from EmptyChaos/interpreter-coretiming

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

Source/Core/Core/PowerPC/CachedInterpreter.cpp

@@ -32,49 +32,57 @@ void CachedInterpreter::Shutdown()
   JitBaseBlockCache::Shutdown();
 }
 
+void CachedInterpreter::ExecuteOneBlock()
+{
+  const u8* normal_entry = JitBaseBlockCache::Dispatch();
+  const Instruction* code = reinterpret_cast<const Instruction*>(normal_entry);
+
+  for (; code->type != Instruction::INSTRUCTION_ABORT; ++code)
+  {
+    switch (code->type)
+    {
+    case Instruction::INSTRUCTION_TYPE_COMMON:
+      code->common_callback(UGeckoInstruction(code->data));
+      break;
+
+    case Instruction::INSTRUCTION_TYPE_CONDITIONAL:
+      if (code->conditional_callback(code->data))
+        return;
+      break;
+
+    default:
+      ERROR_LOG(POWERPC, "Unknown CachedInterpreter Instruction: %d", code->type);
+      break;
+    }
+  }
+}
+
 void CachedInterpreter::Run()
 {
-  while (!CPU::GetState())
+  while (CPU::GetState() == CPU::CPU_RUNNING)
   {
-    SingleStep();
+    // Start new timing slice
+    // NOTE: Exceptions may change PC
+    CoreTiming::Advance();
+
+    do
+    {
+      ExecuteOneBlock();
+    } while (PowerPC::ppcState.downcount > 0);
   }
 }
 
 void CachedInterpreter::SingleStep()
 {
-  const u8* normalEntry = jit->GetBlockCache()->Dispatch();
-  const Instruction* code = reinterpret_cast<const Instruction*>(normalEntry);
-
-  while (true)
-  {
-    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)
-        return;
-      break;
-    }
-  }
+  // Enter new timing slice
+  CoreTiming::Advance();
+  ExecuteOneBlock();
 }
 
 static void EndBlock(UGeckoInstruction data)
 {
   PC = NPC;
   PowerPC::ppcState.downcount -= data.hex;
-  if (PowerPC::ppcState.downcount <= 0)
-  {
-    CoreTiming::Advance();
-  }
 }
 
 static void WritePC(UGeckoInstruction data)

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;
 };

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();
   }
 }
 

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);
+
+  // 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);
-  STR(INDEX_UNSIGNED, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
-
   ABI_PopRegisters(regs_to_save);
   RET(X30);