From 1bcd129683549db3581801a6220e531a558567c2 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Fri, 2 Sep 2016 07:10:51 +0000 Subject: [PATCH 1/3] Interpreter: Fix CoreTiming contract The interpreter does not use CoreTiming correctly. Calls to Advance must be made in advance of executing the associated slice, not afterwards. --- Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) 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(); } } From f5bfce657c90219cebe933978c91f2d1e3d0d1e6 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Thu, 8 Sep 2016 06:42:43 +0000 Subject: [PATCH 2/3] CachedInterpreter: Fix CoreTiming contract Call CoreTiming::Advance() before executing the slice, not after. --- .../Core/Core/PowerPC/CachedInterpreter.cpp | 66 +++++++++++-------- Source/Core/Core/PowerPC/CachedInterpreter.h | 3 +- 2 files changed, 39 insertions(+), 30 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,49 +32,57 @@ void CachedInterpreter::Shutdown() JitBaseBlockCache::Shutdown(); } +void CachedInterpreter::ExecuteOneBlock() +{ + const u8* normal_entry = JitBaseBlockCache::Dispatch(); + const Instruction* code = reinterpret_cast(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(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) 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 m_code; + void ExecuteOneBlock(); + std::vector m_code; PPCAnalyst::CodeBuffer code_buffer; }; From 55a7f576aadda53c835e2b29f82f4619078a4d45 Mon Sep 17 00:00:00 2001 From: EmptyChaos Date: Mon, 12 Sep 2016 09:08:56 +0000 Subject: [PATCH 3/3] JitArm64: Fix CoreTiming contract Call Advance at the start of each timing slice instead of the end. Possibly fixed a bug where a slice would randomly branch straight back to Advance() because the flags were not set to the right values before branching to the dispatcher entrypoint. --- Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp | 40 ++++++++++++-------- 1 file changed, 25 insertions(+), 15 deletions(-) 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); + + // 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);