kervinck / gigatron-rom

System, apps and tooling for the Gigatron TTL microcomputer
BSD 2-Clause "Simplified" License
229 stars 79 forks source link

fix a few bugs in the gtemu I hit, the optimizer is the most interesting one #258

Closed fetchingcat closed 2 months ago

fetchingcat commented 2 months ago

Hi at67.

If you don't want these changes that is fine, or just want to use them to make better ones someday that works as well. I wasn't sure if you are open to updates / fixes still on this project. If so, here are a few fixes to consider.

optimiser.cpp this one was fun. :-) I isolated some bad code gen (really due to optimization pass) with something like the following dim mis_startx(4) n=1 mis_startx(n) = 256 I actually found it with -1 as the operand. But this works too. The reason is the 2 byte operand rather than 1. that ends up with LDWI but the optimizer address calculations treated is as 2 byte instruction rather than 3. the code before ends with address reuse and some very interesting but incorrect code generation. it would seem with my fix its working as expected in this repro and in the cases in the code I encountered it with. This change is the most complex. In order to validate it I compiled all at67's games and several other gbas programs

assembler.cpp
    I found that when I started the emu with _singleStepEnabled to step the OS startup sequence to verify my own projects
    when switching from memory views and disassembled it would crash.  The reason seems to be the address usage with the unsigned value.
        for(uint16_t addr=address-1; addr>=address-3; --addr)
    I added a check to just continue if the value is invalid.

cpu.h
    I bumped the version up one, not sure if wanted

expression.cpp
    I found that when I named a variable starting with rem that the line went away, this took a bit to understand as a user.
    example
    removeMissle = 1
    what I did is change the check to require a space after "rem" which I think is maybe OK?
at67 commented 2 months ago

Thanks for finding these, I will merge the pull request even thought I have fixed all of these already and many many more in my current local repo.

Indeed the optimiser is quite interesting, while looking like a mess, it is actually extremely straightforward to maintain and to add new instruction sequences, (it does unfortunately slow down host compile/link time though, specifically optimisation, the irony!).

fetchingcat commented 2 months ago

Thanks, and good to know! Looking forward to the updates. I agree, the design of the project is great, and I can see how it helps with new additions and maintaining. I have been enjoying working with it. Thanks again!