cpu/mips3: Fix DRC debugger interaction for register and memory modifications#14944
cpu/mips3: Fix DRC debugger interaction for register and memory modifications#14944nekuz0r wants to merge 1 commit intomamedev:masterfrom
Conversation
|
Anybody to review ? |
| const unsigned regnum = entry.index() - MIPS3_R0; | ||
| if (m_regmap[regnum].is_int_register()) | ||
| logerror("debugger R%u = %08X, must update UML I%u\n", regnum, m_core->r[regnum], m_regmap[regnum].ireg() - uml::REG_I0); | ||
| m_drc_iregs_dirty = true; |
There was a problem hiding this comment.
The rest of this file uses Allman brace style, so this should too for consistency.
There was a problem hiding this comment.
There's quite a few one line if/else statements in the file without braces, i kept it as it, should i update it and update the other occurrences or leave it ?
|
|
||
| void mips3_device::device_debug_setup() | ||
| { | ||
| if (!m_isdrc) return; |
There was a problem hiding this comment.
This also should be Allman style.
| UML_DEBUG(block, desc->pc); // debug desc->pc | ||
| UML_DEBUG(block, desc->pc); // debug desc->pc | ||
| UML_TEST(block, mem(&m_drc_iregs_dirty), 1); // test [debugger_iregs_dirty],1 | ||
| UML_JMPc(block, COND_Z, skip_reload); // jmp skip_reload,Z | ||
| UML_MOV(block, mem(&m_drc_iregs_dirty), 0); // mov [debugger_iregs_dirty],0 | ||
| load_fast_iregs(block); // <load fast integer registers> | ||
| UML_LABEL(block, skip_reload); // skip_reload: |
There was a problem hiding this comment.
Considering how frequently this is going to occur, would it be better to turn it into a helper function to keep generated code size down?
There was a problem hiding this comment.
Not sure to understand what's expected to be done, any guidance ?
| debug()->symtable().set_memory_modified_func( | ||
| [this]() { m_drc_cache_dirty = true; } | ||
| ); |
There was a problem hiding this comment.
You don't want to be doing this for every memory modification from the debugger - it's too expensive when most of the time, people will just be modifying data memory.
There was a problem hiding this comment.
The cache invalidation is triggered when editing the ':maincpu' program space, not through any other spaces/regions.
What is your suggestion ?
3 participants
Summary
When debugging MIPS3 code running under the DRC, two things were broken: changing a register value in the debugger had no effect (the DRC kept using its own cached copy), and patching instructions in memory didn't do anything either since the DRC just kept running its already-translated code.
Details
The DRC maps hot registers to internal UML integer registers for performance. The problem is that the debugger writes to the memory-backed register array, not those internal registers — so any change made during a breakpoint was immediately overwritten when execution resumed. The old code knew about this and even logged an error message.
Similarly, there was no mechanism to tell the DRC that memory contents had changed. If you patched an opcode through the debugger, the DRC would happily keep running the stale translated block.
Register reload: A
m_drc_iregs_dirtyflag is set instate_importwhen the debugger writes to a register that lives in a fast UML ireg. After eachUML_DEBUGhook, the generated code tests this flag and reloads the internal registers from the memory-backed array when set. The flag is declared asuint32_trather thanuint8_tbecauseUML_MOV/UML_TESToperate on 32 bits wide data, using a smaller type would corrupt the adjacentm_fpmodetable.Cache invalidation:
device_debug_setuphooks the symbol table'sset_memory_modified_funccallback to setm_drc_cache_dirty, which is checked at the top ofexecute_runand triggers a full cache flush before the next execution slice.Related issue
#4904