6502 engine fixes - #87
Conversation
Added RAMTOP optional define to aambox, default is now $c000 to stress test page eviction.
The page cache sits directly below the heap, but during init its upper
bound is a fixed guess. initsegment sets firstpg = SAFEPG and endpg =
SAFEPG + 48 so the init code can page in story data before the heap
layout is known; endpg is only tightened to the real heap bottom much
later, in initengine5.
Everything the init code allocates in between grows the heap downwards:
the virtual-address table (freeptr -= vtsize), every allocwords call,
and every loadchunk. If freeptr+1 drops below SAFEPG+48 the two
regions overlap and corrupt one another.
To prevent that, we call 'shrinkcache' when changing heap allocation.
It lowers endpg to freeptr+1 and evicts every physical page that
falls outside the new window, so nothing stale stays mapped over heap.
Two details:
- The window is clamped to at least one page (firstpg+1).
- If the round-robin cursor itself ended up outside the window, it is
pulled back to firstpg.
evict doubles as a flag: initsegment zeroes it and
skips the eviction pass while it is zero, since at that point the page
table does not exist yet and only endpg needs lowering.
savegame builds the savefile image in RAM at SAVEADDR, which lies inside the page cache. It reserves that region by raising firstpg above it, so the round robin is temporarily prevented from handing out the reserved pages. Fixes: * savegame now compares the top of the reserved region against endpg before touching anything and takes the normal failure path; a machine too small to hold both simply cannot save. * We now set evict = firstpg so that the evict cursor does not fall out of the reserved region. * putsavebyte had no upper bound. It walks phytmp forward a page at a time, evicting as it goes, so an image larger than the reserved region ran off the end of the cache and into the heap. It now tests phytmp against endpg, and if out of room it unwinds everything and fails.
The undo and save/restore features cost a lot of bytes of engine code, so add conditional defines UNDO and SAVERESTORE to remove them from the engine if needed. The frontend assembler file should define these as 0 (excluded) or 1 (included). This might come in handy if someone wants to try to squeeze an interpreter into 48 kB.
Convenience routine to print a string with address in X/Y.
…ytes) The Apple II port needs a fixed-size save game file, so this is no longer an implementation detail.
The phypc instruction pointer points to a physical page, and is only protected by the round robin function. If it happened to sit inside of the save game buffer being built, it was left untouched, and so the next instruction read would likely be garbage data. Fixed by moving the PC's page up into the reserved window -- evictx then swapin -- before any of the image is written.
|
This looks fantastic! I'm thrilled that someone who properly understands 6502 assembly is interested in this, because I'm struggling with adding any new features to it. While you're here, I did want to ask about one thing I added: Opcode $67 changes its behavior based on a byte in the header. Is it safe to Aside from that question, though, this looks great and passes all the tests, so I have no reservations about merging it in. An Apple II port would be a wonderful addition to the suite of interpreters, and I'm sure the retro community would love to see that too! |
|
I think the header is allocated in initengine1 via allocwords and never paged out, so this ought to be safe. To save 3 bytes you could comment out |
|
Thanks! |
|
My only other request before merging this: add a note to the README so people know what changed in the new version. |
|
Will do -- I'll assume these engine fixes will be part of 1.0.3, and hopefully also the imminent Apple II stuff. |
|
Yep! Just put them under the top heading in the readme, which should currently be 1.0.3. |
2 participants
These are a set of 6502 engine fixes resulting from a concurrent Apple II port.
There were a few memory corruption issues identified during testing, which were more frequent because of the limited RAM of the port. These especially surfaced during SAVE/RESTORE/UNDO operations. Some of them can be replicated by progressively squeezing aambox's heap and calling these commands at various times, though most of my testing was on the Apple II port via emulator.
There are also a couple of new #ifdefs in the engine for completely removing save/undo features to conserve memory (this was initially for the Apple port, though I have opened up enough RAM so these features probably won't have to be turned off)
If these look good, I can then create a PR for the Apple II port on top, probably squashed unless you want to see the whole messy history.