Project

General

Profile

Actions

Emulator Issues #5993

closed

Wrong PC address insert to fifoWriteAddress in CheckGatherPipe.

Added by hk.konpie about 11 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
% Done:

0%

Operating system:
N/A
Issue type:
Bug
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
No
Relates to maintainability:
No
Regression start:
Fixed in:

Description

I try this check code insert at line 103 in GPFifo.cpp.

NOTICE_LOG(GPFIFO, "PC=%08X, FIFO write inst=%s", PC, GetOpInfo(Memory::ReadUnchecked_U32(PC))->opname);

but log display wrong PPC instraction.(Must store instraction!!!!)

It means a fatal bug.....

Please revise it as soon as possible.

Sorry, my bad english from japan.

Actions #1

Updated by delroth about 11 years ago

  • Status changed from New to Invalid

This is normal behavior: the PC value (alias for PowerPC::ppcState.pc) is updated at runtime and its value while compiling code with the JIT is not what you would expect (most of the time it will be the address of the block entry). You probably want to use js.compilerPC instead.

Actions #2

Updated by delroth about 11 years ago

  • Status changed from Invalid to New

I actually misunderstood what you said: in that context PC should be valid. Not sure why PC does not point to the store instruction, ccing skid_au (I think he wrote the code).

Actions #3

Updated by delroth about 11 years ago

  • Status changed from New to Accepted

After looking at the code a bit more, there definitely seems to be something wrong. Random notes:

  • CheckGatherPipe is inserted after FIFO writes. Because FIFO writes could cause exceptions that need to be handled as soon as possible before putting more data in the FIFO (overflows for example), CheckGatherPipe will patch the instruction at PC to add an exception check.

  • PC is not updated at every instruction, so in most cases CheckGatherPipe will actually invalidate from the start of the block and insert the exception check there (which might be completely useless, depending on the specific case).

  • There seems to be no way for CheckGatherPipe to get the real PC value without updating PC before every write instruction in the JIT.

  • We could remove a lot of CheckGatherPipe patching by adding js.compilerPC to fifoWriteAddresses when we detect a write to the FIFO in the JIT store handlers.

Actions #4

Updated by skidau about 11 years ago

  • Status changed from Accepted to Invalid

CheckGatherPipe will only be called on HW writes at 0xcc008xxx. Jit_Util.cpp updates the PC before it goes into the memory handler. Look for the comment: "Helps external systems know which instruction triggered the write". The PC logged should be the PC after the store instruction, not the store instruction itself. If it were captured at the store instruction, the exception would be checked before it writes to the GP FIFO.

If you put the log into Jit.cpp at the fifoWriteAddress, you'll see they are mostly on store instructions.

// Add an external exception check if the instruction writes to the FIFO.
if (jit->js.fifoWriteAddresses.find(ops[i].address) != jit->js.fifoWriteAddresses.end())
{
NOTICE_LOG(GPFIFO, "PC=%08X, FIFO write inst=%s", ops[i].address, GetOpInfo(Memory::ReadUnchecked_U32(ops[i].address))->opname);

Actions #5

Updated by hk.konpie about 11 years ago

CheckGatherPipe will only be called on HW writes at 0xcc008xxx.
And, CheckGatherPipe is called for fifoBytesThisBlock at Jit64::Cleanup() and Line564 in Jit.cpp.

Actions #6

Updated by hk.konpie about 11 years ago

"Helps external systems know which instruction triggered the write".
psq_st is no problem? (WriteDual32 address=0xCC008000)

Actions #7

Updated by skidau about 11 years ago

  • Status changed from Invalid to Fixed

This issue was closed by revision 056930cac809.

Actions

Also available in: Atom PDF