Skip to content

Commit

Permalink
Fix a race condition when pausing the CPU core.
Browse files Browse the repository at this point in the history
This affects enabling and disabling blocking profiling on the fly.
The block profiling pauses the CPU cores and then flushes the JIT's block cache and enables block profile.
The issue with this is that when we pause the CPU core, we don't have a way to tell if the JIT recompiler has actually left.
So if the secondary thread that is clearing the JIT block cache is too quick, it will clear the cache as a recompiler is still running that block that
has been cleared.
  • Loading branch information
Sonicadvance1 committed May 11, 2015
1 parent 805eaa9 commit 89e0a99
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 0 deletions.
3 changes: 3 additions & 0 deletions Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,9 @@ void Interpreter::Run()
PC = NPC;
}
}

// Let the waiting thread know we are done leaving
PowerPC::FinishStateMove();
}

void Interpreter::unknown_instruction(UGeckoInstruction _inst)
Expand Down
6 changes: 6 additions & 0 deletions Source/Core/Core/PowerPC/Jit64/JitAsm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ void Jit64AsmRoutineManager::Generate()
ADD(64, R(RSP), Imm8(0x18));
POP(RSP);
}

// Let the waiting thread know we are done leaving
ABI_PushRegistersAndAdjustStack({}, 0);
ABI_CallFunction(reinterpret_cast<void *>(&PowerPC::FinishStateMove));
ABI_PopRegistersAndAdjustStack({}, 0);

ABI_PopRegistersAndAdjustStack(ABI_ALL_CALLEE_SAVED, 8, 16);
RET();

Expand Down
4 changes: 4 additions & 0 deletions Source/Core/Core/PowerPC/JitArm32/JitAsm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ void JitArmAsmRoutineManager::Generate()
if (SConfig::GetInstance().m_LocalCoreStartupParameter.bEnableDebugging)
SetJumpTarget(dbg_exit);

// Let the waiting thread know we are done leaving
MOVI2R(R0, (u32)&PowerPC::FinishStateMove);
BL(R0);

ADD(_SP, _SP, 4);

POP(9, R4, R5, R6, R7, R8, R9, R10, R11, _PC); // Returns
Expand Down
4 changes: 4 additions & 0 deletions Source/Core/Core/PowerPC/JitArm64/JitAsm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ void JitArm64AsmRoutineManager::Generate()

SetJumpTarget(Exit);

// Let the waiting thread know we are done leaving
MOVI2R(X0, (u64)&PowerPC::FinishStateMove);
BLR(X0);

ABI_PopRegisters(regs_to_save);
RET(X30);

Expand Down
17 changes: 17 additions & 0 deletions Source/Core/Core/PowerPC/PowerPC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "Common/ChunkFile.h"
#include "Common/CommonTypes.h"
#include "Common/Event.h"
#include "Common/FPURoundMode.h"
#include "Common/MathUtil.h"

Expand Down Expand Up @@ -33,6 +34,7 @@ static volatile CPUState state = CPU_POWERDOWN;

Interpreter * const interpreter = Interpreter::getInstance();
static CoreMode mode;
static Common::Event s_state_change;

Watches watches;
BreakPoints breakpoints;
Expand Down Expand Up @@ -238,16 +240,31 @@ void Start()

void Pause()
{
volatile CPUState old_state = state;
state = CPU_STEPPING;

// Wait for the CPU core to leave
if (old_state == CPU_RUNNING)
s_state_change.WaitFor(std::chrono::seconds(1));
Host_UpdateDisasmDialog();
}

void Stop()
{
volatile CPUState old_state = state;
state = CPU_POWERDOWN;

// Wait for the CPU core to leave
if (old_state == CPU_RUNNING)
s_state_change.WaitFor(std::chrono::seconds(1));
Host_UpdateDisasmDialog();
}

void FinishStateMove()
{
s_state_change.Set();
}

void UpdatePerformanceMonitor(u32 cycles, u32 num_load_stores, u32 num_fp_inst)
{
switch (MMCR0.PMC1SELECT)
Expand Down
1 change: 1 addition & 0 deletions Source/Core/Core/PowerPC/PowerPC.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ void RunLoop();
void Start();
void Pause();
void Stop();
void FinishStateMove();
CPUState GetState();
volatile CPUState *GetStatePtr(); // this oddity is here instead of an extern declaration to easily be able to find all direct accesses throughout the code.

Expand Down

0 comments on commit 89e0a99

Please sign in to comment.