MEGA65 / mega65-core

MEGA65 FPGA core
Other
241 stars 88 forks source link

RTS immediate mode instruction completely broken #790

Closed ki-bo closed 3 months ago

ki-bo commented 7 months ago

Test Environment (required) You can use MEGA65INFO to retrieve this.

Describe the bug The RTS instruction in the CSG4510 has an immediate mode version that should jump back from a subroutine to the address on the stack, and in addition also add a number of bytes to the SP provided as immediate parameter. This instruction is listed in the compendium, but does not work at all. It even behaves differently if stepping in the serial monitor compared to running without trace mode enabled.

To Reproduce Steps to reproduce the behavior:

  1. Use a small ASM test program like this (Kickass syntax):
    sei
    jsr label
    !:  inc $d020
    jmp !-
    label:
    //rts #$00
    .byte $62, $00
    !:  inc $d021
    jmp !-
  2. Execute this with trace mode enabled in serial monitor (t1)
  3. The instruction (shown as RTN #$00) will just be executed as if it was a NOP, the background color will start flashing (incorrect - the border should flash instead).
  4. Execute again, but this time without trace mode
  5. The ROM monitor will be shown, indicating we were crashing, eventually executing a BRK command. I wasn't able to exactly find out where the PC was going in this case. But it behaves differently compared to trace mode and it is incorrect behaviour.

Expected behavior The rts #$00 command should just jump back from label without further altering the stack. The border should start flashing as a result.

Additional context I am pretty sure the description in the compendium is also incorrect. It says:

This instruction adds an optional argument to the Stack Pointer (SP) Register, and then pops the Program Counter (PC) register from the stack, allowing a routine to return to its caller.

This would not make sense, as the typical operation of calling a subroutine is to first push parameters on the stack and then correct the SP to remove the pushed parameter again. This workflow is for when the called routine will not pull the parameters but read them directly from the stack's memory (eg. via TSXand then LDA $0103,x). I assume the correct behaviour would be to first pull the return address from the stack.

lydon42 commented 3 months ago

Extra issue for unittest added.