search

MEGA65 / mega65-core

MEGA65 FPGA core
Other
245 stars 88 forks source link

BRK instruction in MEGA65 Memory/Machine Inspection interface disassembles to 2 bytes #843

Open dansanderson opened 3 weeks ago

dansanderson commented 3 weeks ago

Originally submitted by @pchnl as https://github.com/MEGA65/mega65-rom-public/issues/179

When assembling the BRK instruction, it is assembled to 1 byte ($00) and I believe that to be correct (despite the stack manipulation done by the interrupt handling).

When attempting to step through some code with the MEGA65 Memory/Machine Inspection interface (Mega + Tab) I got confused because it shows the original code (00 EE 20 D0 00) which is: BRK INC $D020 BRK

as: BRK $EE <== looks wrong to me JSR $00D0

By the way, the MEGA65 monitor (MONITOR) displays: 00 BRK EE 20 D0 INC $D020 00 BRK

Which I think is correct.

pchnl commented 3 weeks ago

As to svOlli's comment: Thanks svOlli. That is valuable insight.

It is totally OK with me that BRK is a 1 byte instruction + 1 byte operand. In my mind, it would make sense if the second byte is an operand as in BRK #12 to indicate why we do an interrupt.

Anyway, I am still learning 6502 and it is confusing to have MONITOR disassemble it one way and the MEGA65 Memory/Machine inspection interface disassemble it in another way. Unless I miss something...

To solve this for now, I think I should always code a NOP after each BRK?

SvOlli commented 3 weeks ago

You're welcome. As far as I can tell, both variants are (with and without operand) are somewhat correct.

The MONITOR works like it was done for >35 years now. Everyone thought of BRK taking two bytes as a caveat to work around. For example using Action Replay on a C64 (when selecting the option "Install Fastload"), when triggering a BRK, it even falls back to the address BRK when returning. My takeaway from this is that the processor specification was more taken like a recommendation and not like the only way to do it.

This is why I've seen no disassembler (or assembler for that matter) implementing BRK with a parameter. To switch there to a two byte variant, even though technically correct, would be incorrect from the evolutionary point of view. Remember that the MONITOR of a C128 and a Plus/4 (dis-)assemble BRK to an opcode without parameters. And this is the software provided by the manufacturer of the CPU.

However, the MEGA65 Memory/Machine inspection was implemented way closer to the CPU design than the "historic" disassembler evolution. (I did the same with mine.)

So, both approaches make sense and are the best solution from their "point of view". The best solution would be to have a pop-up disclaimer explaining that to the developer. ;-) Okay, it doesn't make sense.

Still changing the behaviour of the MONITOR commend will result in another ticket, that BRK is not working like in any other monitor.

So, I can't suggest a solution for this. My first idea was to have the MEGA65 variant configurable in some way, but this only moves the problem to "how to set the default".

The more I think about it: if it was my project, I'd put this in the documentation and leave things as they are.

P.S.: My guess is that this is how the BRK got an operand: the interrupt logic of the chip would have gotten complexer, if a BRK was implemented like a single byte opcode. So they decided to keep things as they are, document it in a way like it could be used as a feature.