SingleStepTests / ProcessorTests

A language-agnostic JSON-encoded instruction-by-instruction test suite for the 8088, 68000, 65816, 65[c]02 and SPC700 that includes bus activity.
187 stars 13 forks source link

65816: Incorrect IO cycles for branch instructions in emulation mode #24

Closed qookei closed 1 year ago

qookei commented 1 year ago

For example, in test "80 e 10000", 2 stall cycles are expected at the end despite the target not crossing a page:

"cycles": [
    [10062147, 128, "dp-remx-"],
    [10062148, 114, "-p-remx-"],
    [10062148, null, "---remx-"],  (5) Add 1 cycle if branch is taken.
    [10062148, null, "---remx-"]  (6?) Add 1 cycle if branch is taken across page boundaries in 6502 emulation mode (E=1).
]

In that test, the initial PC is $8943, branch offset is $72, and final PC is $89b7, which AFAICU does not constitute a crossing of the page boundary (($8943 & $ff00) == ($89b7 & $ff00)), which is also the case according to the 65816 emulator cores I've checked (the ones in mesen2, bsnes, and snes9x).

Perhaps I am fundamentally misunderstanding how page crossing is determined, but this appears to affect ~75% of tests in 80.e.json: image

EDIT: For reference, here's my code that handles branches:

exec_op2:
    if (opcode_ == 0x80 /* BRA */ || (opcode_ & 0b11111) == 0b10000 /* Bcc */) {
        uint8_t flag_bits[] = { flags::N, flags::V, flags::C, flags::Z };
        uint8_t flag = flag_bits[(opcode_ >> 6) & 0b11];
        bool taken = opcode_ == 0x80 || (!!(r_.flags & flag) == !!(opcode_ & (1 << 5)));

        if (taken) {
            int8_t off8 = uint8_t(target_addr_ - r_.pc);

            if (r_.e && (((r_.pc & 0xFF) + off8) & 0xFF00))
                enqueue(&&stall_last);

            r_.pc = target_addr_;

            enqueue(&&complete_op);
            goto stall_last;
        } else {
            goto complete_op;
        }
    }
TomHarte commented 1 year ago

Having had a peek, the problem was even greater than this — per the relevant typo I'm pretty sure the extra cycle was being skipped where the target page is equal to the relative jump, e.g. BRA $03 would skip a cycle if the target was in page $03. Very dumb.

The linked pull request is intended to fix the problem, though I've yet to do any manual verification.

qookei commented 1 year ago

PR #30 makes all the branch instruction tests pass in my emulator now