emu-rs / rustual-boy

Rustual Boy - A Virtual Boy emulator.
https://rustualboy.com/
Apache License 2.0
232 stars 30 forks source link

Add cpu tracing thats enabled via features cpu-trace #25

Closed jwestfall69 closed 7 years ago

jwestfall69 commented 7 years ago

cpu tracing allows tracking the before and after states of the registers/memory/etc that an instruction will use. This before and after data can then be displayed when using the step function in the debugger.

(vb-rs 0xfff90fea) > cputrace cpu trace enabled: true

Then when you run the step command it will look like this

(vb-rs 0x07000c00) > s 0x07000c00 0fce0400 ld.w 4[r15], r16 before PC: 0x07000c00 PSW: 0x00010000 r15: 0x0500085c r16: 0x00000000 disp: 4 mem.w: [addr: 0x05000860, value: 0x0500089c] cpu cycles: 4 after PC: 0x07000c04 PSW: 0x00010000 r15: 0x0500085c r16: 0x0500089c disp: 4 mem.w: [addr: 0x05000860, value: 0x0500089c]

This required tweaking the debugger step function which used to cpu.step(), then disassemble() to disassemble(), then cpu.step(). This was needed to make the pc reg and the disassembled instruction match up.

jwestfall69 commented 7 years ago

2 things to be aware of.

  1. Tracing system registers doesnt give any useful info. The cpu doesnt keep state on the system registers and just makes up/parses their values on the fly. This makes it impossible to see what those registers are before/after the instruction executes.

  2. Long lines on console for the trace data. This is mainly a problem with the bit string ops which may touch numerous memory addresses, all of which get added to tracing and displayed.

(vb-rs 0x07028ed2) > 
  0x07028ed2  087c        orbsu
    before PC: 0x07028ed2 PSW: 0x00000000 imm5: 8 r0: 0x00000000 r26: 0x00000000 r27: 0x00000000 r28: 0x00000002 r29: 0x000039c0 r30: 0x07028cf4 mem.w: [addr: 0x07028cf4, data: 0xaaaaaaaa] mem.w: [addr: 0x000039c0, data: 0x00000000] mem.w: [addr: 0x07028cf4, data: 0xaaaaaaaa] mem.w: [addr: 0x000039c0, data: 0x00000000] cpu_cycles: 1
     after PC: 0x07028ed4 PSW: 0x00000000 imm5: 8 r0: 0x00000000 r26: 0x00000002 r27: 0x00000002 r28: 0x00000000 r29: 0x000039c0 r30: 0x07028cf4 mem.w: [addr: 0x07028cf4, data: 0xaaaaaaaa] mem.w: [addr: 0x000039c0, data: 0x00000002] mem.w: [addr: 0x07028cf4, data: 0xaaaaaaaa] mem.w: [addr: 0x000039c0, data: 0x00000002]

Hrm, that orbsu trace output is indicating you maybe needless re-reading memory addresses in that opcode.

yupferris commented 7 years ago
  1. Tracing system registers doesnt give any useful info. The cpu doesnt keep state on the system registers and just makes up/parses their values on the fly. This makes it impossible to see what those registers are before/after the instruction executes.

Perhaps I misunderstood the issue here; check my review comments for some feedback.

  1. Long lines on console for the trace data. This is mainly a problem with the bit string ops which may touch numerous memory addresses, all of which get added to tracing and displayed.

This is probably ok, but if it gets really ugly might want to consider a bit smarter formatting (eg. multiple lines), esp. since the number of entries is known before printing.

Hrm, that orbsu trace output is indicating you maybe needless re-reading memory addresses in that opcode.

Yes, the current implementation is very naive in that sense; meant to be as dumb as possible for a simple implementation. A more proper implementation would probably cache the src/dst values until it had to read/write new ones. Feel free to give this a shot if it gives you trouble. :)

jwestfall69 commented 7 years ago

Can we hold off for a bit or close this pr. I ran into tracing a few bsu instructions last night and I would like to get the output issue fixed. I believe it would be best to adjust the enum to also include the post exec data. So Gpr(reg, data) would become Gpr(reg, pre_data, post_data). This should make things easier to deal with in the debugger and allow for output like

println!("r{}: 0x{:08x} => 0x{:08x}", te.reg, te.pre_data, te.post_data)
yupferris commented 7 years ago

Aha, yeah, that sounds sensible. I'll go ahead and close this and you can open a new PR when it's ready :)