Repository navigation
Clear the index high bytes when the X flag is set - #214
Conversation
On the 65816, setting X makes X and Y 8-bit with a zero high byte. The interpreter does this in interp816_setFlags; generated code only zero-extended 8-bit X/Y writes, so a register kept across SEP #$10 still held its old high byte. An interrupt handler that switched back to 16-bit and pushed X then stored that stale byte. Found on a HiROM title. cpu_p_to_mirrors now clears XH/YH whenever X ends up set, which covers SEP, PLP and RTI. The XCE emitter clears them when entering emulation mode.
mstan
left a comment
mstan
left a comment
There was a problem hiding this comment.
No blocking code findings. Clearing the high bytes in cpu_p_to_mirrors covers the generated SEP/PLP/RTI paths, and the XCE change covers entering emulation without disturbing a native-to-native exchange. This correctness fix applies by default.
Reviewed with #199 and #215 on current main 246c0d4. The combined tree passes the codegen width lint and 860 Python tests (2 platform skips), all six production PPU tests and the production runtime dispatch fixture. Extra compiled checks exercise emitted SEP then REP and both XCE directions. All three workflows on this reviewed PR head are green.
The shared regression branch is review/open-prs-final-20261026 (b19edfb). This is code and automated-fixture approval; the new #214/#215 pair still needs the normal combined title gameplay/current-build save-load/runtime gate and owner handoff before merge. Prior #199 title approval does not qualify this new pair. No games were relaunched in this review.
mstan
left a comment
mstan
left a comment
There was a problem hiding this comment.
Combined regression follow-up: the current-main #199/#214/#215 branch review/open-prs-final-20261026, frozen at b19edfb, passes all 860 Python tests (2 platform skips), six production PPU harnesses, runtime dispatch and additional emitted SEP/REP/XCE boundary checks. All eight matched Release baseline/candidate builds have recorded engine/title/UI/ROM/executable hashes.
Muted gameplay, movement, current-build save/load and nonzero emulated audio checks pass in Mega Man X, ALttP, SMW and Super Metroid. Brief built-runtime checks show no consistent introduced regression; an initial SM timing outlier did not reproduce with opposite-order matched-scene repeats. SMW remains host-paced near 60fps in Yoshi's House, so this does not qualify wider-stage margins or uncapped performance. SM uses the established original-loader Zebes setup and bypasses Ceres.
The automated combined gate is now complete for this reviewed head. Approval stands; the owner's quick visual/play check of the new #214/#215 batch remains pending before merge. No author changes requested.
…racts-193 Share HiROM contracts and evidence while preserving execution banks. Qualified in the combined #199/#214/#215 regression batch at b19edfb: 860 Python tests, native checks, and four representative matched gameplay/save-load/runtime checks. Owner explicitly approved merge after regression confirmation.
Draw BG modes 4-6 with the compatibility renderer under NewRenderer. Qualified in the combined #199/#214/#215 regression batch at b19edfb: 860 Python tests, native checks, and four representative matched gameplay/save-load/runtime checks. Owner explicitly approved merge after regression confirmation.
On the 65816, setting the X flag makes X and Y 8-bit, and their high bytes become zero. The interpreter does this in
interp816_setFlags. Generated code only zero-extends 8-bit X/Y writes, so a register that keeps its 16-bit value acrossSEP #$10still holds the old high byte. The comment in_emit_incregalready assumes the hardware behaviour ("SEP #$10 zeros it at the flag transition"), but nothing on the compiled side actually does it.I ran into this on a HiROM title: an interrupt handler switched back to 16-bit index registers and pushed X, and the compiled tier stored the stale high byte where the interpreter stored zero.
Change
cpu_p_to_mirrorsclears XH/YH whenever X ends up set. SEP, PLP and RTI all go through it, so one place covers them._emit_xceclears them when entering emulation mode, next to the existingm_flag/x_flagreset.Test
tests/v2/test_index_high_byte_on_x_set.pycompiles a small harness againstcpu_state.hand the emitted XCE body. It checks that X set clears the high bytes, that X clear keeps them, and that XCE into emulation clears them. Before the fix it fails (x set: X=F000 Y=1234 exp 0000/0034,xce: X=ABCD Y=5678 exp 00CD/0078), after the fix it passes.Full Python suite on the branch: 847 passed, 2 skipped (Windows, MinGW gcc 16.2).