Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merging the AR panels, etc. #23

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

john-peterson
Copy link
Member

No description provided.

@john-peterson
Copy link
Member Author

Reducing duplication in the code and memory view

Problem

There's duplication in

  • CodeView
  • MemoryView

Solution

Moving duplicate code to a shared class because

  • that makes it faster to change the shared code becuse it can be changed in one rather than two locations

Text location

CodeView

The left horisontal text location is

  • margin: 16 (unchanged)
  • address: margin + 1 (unchanged)
  • instruction: address + 1 or 9 (unchanged)
  • disassembly: instruction + 10 (+ 2)
  • symbol: disassembly + 20 (+ 2)
  • branch: symbol + 15 (+ 1)

The changed locations

  • reduce the text overlap

There's still overlap because

  • it's important that branches are visible in a 1920 / 2 wide Dolphin window
  • long disassembly and symbols are unusual

MemoryView

The changed locations

  • remove text overlap

Adding names for numbers

Names are added for the text location numbers because

  • that makes it easier to understand what the number mean

Test

Tested in

  • CodeWindow
  • MemoryWindow
  • DSPDebuggerLLE

with

  • selecting code row
  • go to address
  • search for value

Code and memory view synchronisation

Go to address goes to address in both code and memory view because

that's better than a go to function that's unique to the code and memory window because

  • it require sending events between classes

Pause/play doesn't go to the current execution in the memory view because

  • it's not always meaningful because the user sometimes intend to continue to observe the current memory view

@john-peterson
Copy link
Member Author

Adding option to change compilation setting during emulation

Changeable settings

The changeable settings are

  • debug (step and breakpoint)
  • interpreter
  • skip idle
  • compiler block link
  • compiled code cache size
  • optional compilation instructions

Compiler

Changing between the default and intermediate language compiler isn't enabled because

it crashes with an empty call stack on the first step (F11) after the change

because

  • a state outside the compiler instance (PowerPC::ppcState) is different despite execution being at the same instruction

Optional step and breakpoint support in debug mode

Breakpoints are made optional (rather than non-optional) when debug mode is enabled because

  • non-debug mode is faster (around 3×) than debug mode
  • debug mode can be enabled for another reason than breakpoints; code and memory view
  • it's useful to test enable/disable the functions it dsiables (skip idle, compiler block link) in debug mode

Compiler instance variable

The compiler setting for

  • debug
  • skip idle

are changed from Core::g_CoreStartupParameter to JitBase::jo because

  • it's meaingful to distinguish between variables that exist for the duration of the JIT instance and emulation instance because the JIT instance can be reloaded during emulation
  • allow searching for the compiler instance (rather than emulation instance) variable

HLE

If emulation is started with bEnableDebugging = true

it's not possible to undo HLE::PatchFunctions when debugging is disabled during emulation because

  • HLE doesn't have a function to undo patch by saving the non-patched code

bEnableDebugging = false emulation without HLE::PatchFunctions can display the message

---------------------------
Warning
---------------------------
Unknown EXT interrupt: Exceptions == 00000000
---------------------------
Yes   No   
---------------------------

Disableable instructions

Compilation for these instructions can't be disabled

Default

Branch

Unhandled exception at 0x000000001245AF93 in DolphinDF.exe: 0xC0000005: Access violation writing location 0x000000004E800020.

Intermediate language

Integer > cmpXX

---------------------------
Warning
---------------------------
Retrieving unknown spill slot?!
---------------------------
OK   
---------------------------

---------------------------
Warning
---------------------------
WriteRest: op out of range (0x1236cdb3 uses 0x40219e13c)
---------------------------
Yes   No   
---------------------------

Menu organisation

The disable options are placed in a submenu because

  • that's better organisation

Missing setting

Adding bJITBranch to Codewindow because

  • it's missing

Clarifying event ranges

Adding enumerator range name

IDM_DEBUG_BEGIN
IDM_DEBUG_END

because that

  • clarify that the interviewing IDs are used in a range
  • return the range code on search for the enumerator name

Moving interpreter setting to separate variable

Moving interpreter from being in the same variable as recompilation to being in a separate variable because

  • that allow enable/disable interpreter option without changing the recompilation option
  • run a movie without the movie enable/disable interpreter based on what was used when its state was saved

Known word

The compiler variable name is iCompiler instead of iJIT because

  • abbreviations should be avoided when there's a more widely known word with equivalent meaning

Using variable instead of argument

Arguments for which there's a global variable are removed, f.e.

-InitTables(int cpu_core)
+InitTables()

because it's better to read the variable than an argument because

  • the function identity is shorter
  • the code that call the function is shorter

Code view instruction position

The code view position isn't changed when a compile setting is change because

  • it doesn't necessarily have a meaning
  • it's easy to select show PC if this action is desired

Test

This is tested in

  • SSBM intro movie

Message

Changing the message

Clearing code cache

to

Reloading code

because

  • that communicate the action most clearly

Refactor

The variable names are refactored to

remove a "Off" suffix because

  • it's better to use "var = false" than "varOff = true" because the name is shorter

add word separator "_" because

  • that makes it easier to read

shorter names

bJITBlockLinking changed to bJITBlockLink because

  • it's consistent with the other bJIT* variables and enableBlocklink because they're nouns rather than verbs
  • the noun is shorter than the verb

@john-peterson
Copy link
Member Author

Adding missing memory breakpoint flags

To

void CBreakPointView::Update()

from

void MemChecks::AddFromStrings(const TMemChecksStr& mcs)

@john-peterson
Copy link
Member Author

Changing memory check to consider the data size

Problem

To check the byte 0x03 the user must enter more than one memory check

at 0x00 to detect 32 bit access
at 0x02 to detect 16 bit access
at 0x03 to detect 8 bit access

Solution

Considering the read/write size because

that require only one memory check

at 0x03 to detect 8, 16, 32 bit access

because a build test reduce commits with build errors
@john-peterson
Copy link
Member Author

Merging the AR panels

Problem

Duplication

There's two AR panels that have the same purpose

Solution

Removing duplicate panel code

Merge the AR panels because of these benefits

  • the Cheats manager window AR selection is saved
  • the Cheats manager window AR tab receive the AR edit/add/remove options
  • the ISO properties AR tab receive the code info box

Removing duplicate arCodes

Two arCodes are reduced to one

Removing GetSelection check for enable/disable

Changing

int index = m_CheckListBox_CheatsList->GetSelection();

to

int index = event.GetInt();

because

  • check/uncheck shouldn't consider GetSelection because it's not necessary to select a wxCheckListBox item to check/uncheck

Test

These AR code actions are tested during emulation

  • enable/disable/edit/add/remove/create

Merge status

Approval status

  • not approved

@john-peterson
Copy link
Member Author

Adding memory log

because it's useful for interpreting machine code addresses

@john-peterson
Copy link
Member Author

Removing an unhandled exception for wx AUI toolbar items that are clickable and have a menu

Problem

Selecting a clickable toolbar menu item result in an unhandled exception in wxWidgets

Reproduce

samples/aui/auidemo.cpp

void MyFrame::OnDropDownToolbarItem(wxAuiToolBarEvent& evt)
{
    evt.Skip();

return

Unhandled exception at 0x774a15de in auidemo.exe: 0xC0000005: Access violation reading location 0x000000a4.

>   msvcp90d.dll!std::basic_string<unsigned short,std::char_traits<unsigned short>,std::allocator<unsigned short> >::basic_string<unsigned short,std::char_traits<unsigned short>,std::allocator<unsigned short> >(std::basic_string<unsigned short,std::char_traits<unsigned short>,std::allocator<unsigned short> >::_Has_debug_it _Hdi={...})  Line 591  C++
    0018f128()  
    …
    auidemo.exe!wxWindow::HandleMouseMove(int x=79, int y=77, unsigned int flags=0)  Line 5576  C++

because that information sometimes not meaningful
because it's otherwise incompatible with Common
because it's useful to save the state
because they show the breakpoint conditions
because it's useful for changing a game
because that reduce duplication

Fixing AR toggle (it used selection rather than event index)
because it makes it faster to reload
because it's useful to copy it
because the other disassembly is hexadecimal
because that allow analysing a changing value
because that apply code cheats
@john-peterson
Copy link
Member Author

Making memory check optional

Commit

The commit is in this branch and has the same title as this post

Problem

Memory breakpoints can't be used in the Release configuration

This is a problem because

  • the Release configuration emulation of F-Zero GX and SSBM is around 3 × the speed of the DebugFast configuration when bEnableDebugging = false
  • the same speed when bEnableDebugging = true but when debugging it's useful to fast forward to another location by disabling debugging

Enabled options

The execution and memory broakpoint toolbar buttons aren't disabled when debugging is disabled because

  • it can still be useful to enable/disable breakpoints because they can be used whe n debugging is enabled

because that's better organisation
because it's useful to show code at another address
because that reduce the number of checks
because that allow changing the setting faster
because it affect performance
because it's useful for interpreting machine code addresses
because it's useful to test code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant