melonDS-emu
Misc tweaks and fixes to the interpreter
Remake of my prior interpreter pr to separate out the very complex and performance intensive pipelining timing rework from the miscellaneous bugfixes to get something more easily mergeable.
Fixes and features include:
- Writeback should be done before jumping with ldm (fixes an edge case with spsr swapping ie. (sp!, {r15}^)
- Improvements to Data Abort behavior (timings are untested and very likely wrong though.....) (above fixes aid in getting gbarunner3 working!)
- Implement bit 15 of the CP15Control register aka. thumb interwork or w/e it's called
- Implement the strange and glitchy behavior of certain loads to r15 on the arm9.
- Fix the remaining pc stores to be addr+12 instead of addr+8, including "illegal" stores and mcr
- improve some instruction timings: msr, mrs, mcr, multiply instructions. (some are knowingly still wrong, at least on a technical level, but a proper implementation would require pipelining, so we'll pass on that for now)
- Fix being able to write to unwriteable bits of the PSR/MPU region sizing regs.
- fix clz r15 not being a proper jump (not a comprehensive fix as some other instructions still need pc writes to be tested and implemented)
- Implement the improved bus width of vram when the vram access scfg bit is set (bit 13)
- Caches should be disabled when the MPU is disabled
- Improve accuracy of Prefetch aborts: Timing cost for an aborted fetch is more accurate, and the cpu now behaves properly when moving into an un-executable region without a jump. Additionally melonDS no longer stops execution of the rom when jumping to an un-executable exception region. (that last part is extremely intensive though, ig it could be optimized with a simple hack? Though I strongly doubt it really matters?)
- Slightly tweak the way IRQs are processed, they are now processed before an instruction is executed, rather than after. Shouldn't make a huge difference but it should be more "technically correct". might also be a very slight speedup? somehow?
I have no idea what it does BUT people in this project really need to learn to squash their commits before submitting. Not only a string of haphazard commits is unreadable but, worse, it's a ticking time-bomb for merging. There is nothing worse for patch compatibility than making a change and partially reverting it in the next commit without merging the summary change instead.
Which is why it's been mere 2 weeks and this already does not apply to current master due to that. Add to this the fact that most pull requests are just ignored, even crash-fixing ones, then they bitrot at unprecedented speeds.
These changes do apply to master, at least if GitHub's indication is to be trusted.
You do have a point about the PRs situation, but I'm not sure what we can really do about it in practice. We don't really have the resources to review and merge the PRs that tend to show up, so they end up stale and not trivially mergeable.
These changes do apply to master, at least if GitHub's indication is to be trusted.
Apparently, it's not as downloaded patch did not apply to git's snapshot for OpenBuildService build. It's possible that Github creates summary patch before applying but spews out commit string if you try to download manually. Or, more likely, it's not even gets updated with master and only indicating status at the time of submission. These Github's "checks" are most often lies either way.
The patch failure was trivial but, apparently, I also should not try to apply patches with -F3 mode and instead use only -F1 because something got malformed and gave a bunch of compilation errors until I remade the squashed patch with one more manual change.
You do have a point about the PRs situation, but I'm not sure what we can really do about it in practice. We don't really have the resources to review and merge the PRs that tend to show up, so they end up stale and not trivially mergeable.
One problem would be bad enough but when 2 multiply on each other (letting it sit and being a complex string of commits), it's a recipe for disaster.
By the way, merge error here was trivial but your next one, https://github.com/melonDS-emu/melonDS/pull/2127, requires a lot more manual intervention and also is being broken by this one, as they touch the same file in bunch of places.
Well, our workflow uses GitHub, so if things apply properly in GitHub (or with git merge) then it's good enough. It sounds like what you're trying to do is a bit outside of the scope of what you should reasonably be expecting to work with WIP code in unmerged PRs.
Well, our workflow uses GitHub, so if things apply properly in GitHub (or with
git merge) then it's good enough. It sounds like what you're trying to do is a bit outside of the scope of what you should reasonably be expecting to work with WIP code in unmerged PRs.
Uh-huh, and the "what should reasonably be expecting to work with WIP code in unmerged PRs" is actually merging and compiling it >_< If you're not going to merge it here and I'm not going to merge it into an actual build then what are even doing here? Am I not supposed to actually testing code? Is anyone else? If no one is supposed to test it then what is it waiting here?
Also, such things happen only in badly maintained repos with ignored patches, in most you can grab whatever and mix and match them, unless whole subsystem got a recent rewrite but even in that case maintainers usually having a look and merge, fix / request changes or reject for good before proceeding. So, this is a you-problem (as in, this-whole-repo), not how git workflow supposed to operate.
By the way, OBS or any distro's build-service does not use git-merge. This is what maintainers have to resort to then you don't make releases or even merge changes by yourselves. Most just drop such packages and let users figure out it themselves.
You really should not be merging random PRs into a distro package build. Even the one distro I know of that makes the maybe-questionable-for-user-experience choice of tracking melonDS's master branch (NixOS) does not do that.
These PRs by Jakly in particular are all ongoing research about very specific hardware behavior, those that turn out to be correct will eventually get merged as discussed in our IRC/Discord chat. If you want to test any of this stuff there's the automatic builds (see the checks here), or you can simply clone the branch and build it yourself locally – code reviews and testing on mergeable PRs is welcome.
You really should not be merging random PRs into a distro package build. Even the one distro I know of that makes the maybe-questionable-for-user-experience choice of tracking melonDS's master branch (NixOS) does not do that.
If you don't want distros to do that then provide them with stable releases. I'm actually was checking out PR list to see if there was a dangling fix for a terrible crash (just like https://github.com/melonDS-emu/melonDS/pull/1982) on startup if ExternalBIOSEnable = false because of which I also can't load my old save for "Castlevania - Dawn of Sorrow" because it's tied to particular firmware at save's creation.
Since I'm here, I often decide to merge some other stuff since master might as well be broken anyway. That's for my personal build, the official distro maintainer gave up for good long ago.
These PRs by Jakly in particular are all ongoing research about very specific hardware behavior, those that turn out to be correct will eventually get merged as discussed in our IRC/Discord chat. If you want to test any of this stuff there's the automatic builds (see the checks here), or you can simply clone the branch and build it yourself locally – code reviews and testing on mergeable PRs is welcome.
Again, that's a you-problem. You're supposed to be discuss it here and test it for yourselves. That way you may actually lose bad habits and stop being sloppy. Instead of pushing users into adopting them too.
I also can't load my old save for "Castlevania - Dawn of Sorrow" because it's tied to particular firmware at save's creation.
The issue for that isn't so much it's "tied" to the particular firmware. It's rather it uses the BIOS' CRC function to checksum the save. This was not implemented correctly in FreeBIOS previously, resulting in a different CRC than the original BIOS. This was fixed a while ago, so dev builds should have the same behavior with FreeBIOS and the original BIOS wrt the CRC function. If you have a save created using the old FreeBIOS, it just won't be compatible with newer melonDS versions ever.
That's for my personal build
So it's your problem.
