Fix capture wraparound, JIT settings, and savestate section validation - #1
Merged
Conversation
… validation Limit wrapped capture readback to the remaining blocks. Validate section headers and lengths before both returning a matching section and skipping one. Reuse the one-line ARMJIT fix authored by Ogato in melonDS-emu#2654; its base ARMJIT.cpp blob matches this fork. The savestate work is informed by melonDS-emu#2712 but also validates matching sections before returning. FIFO validation from that PR is not included. Add focused standalone C++ regression tests and a GCC/Clang sanitizer workflow. Document measurement-first performance direction without introducing a language rewrite or unmeasured SIMD requirements. Locally verified: 7/7 CTest entries with GCC 14.2 and Clang 17 ASan/UBSan. These are dependency-isolated tests, not a complete emulator build, real OpenGL execution, or game/performance validation.
Owner
Author
GitHub verification updateThe PR-triggered Core regressions workflow completed successfully for commit Both matrix jobs passed configuration, build, and tests: GCC and Clang ASan UBSan. The Clang job log confirms 7/7 CTest entries passed using Clang 14.0.0 on Ubuntu 22.04.5, from GitHub's full repository checkout rather than the local source fixture. This verifies the focused test project against the real repository headers. It is still not a full application/game/real-OpenGL or performance test. At this check, the inherited Ubuntu, Windows, macOS, and BSD application workflows were still in progress. The PR remains draft; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Initial, bounded correctness pass for this fork; no language rewrite or unmeasured AVX-512/assembly changes.
GLRenderer::SyncVRAMCaptureto use the remaining block count after VRAM wraparound. The previous loop wrote/marked extra guest VRAM blocks in three wrap cases.BranchOptimizationswhen changing JIT maximum block size, using the one-line fix authored by Ogato in Use BranchOptimizations in ARMJIT::SetMaxBlockSize melonDS-emu/melonDS#2654. The PR base and this fork have the same originalARMJIT.cppblob; the corrected full-file blob is reused unchanged.docs/Performance.md.Production changes are confined to three files, with no save-format version, renderer interface, native JIT emitter, ISA baseline, or dependency changes.
Executed local verification
Combined CTest entries: 7/7 passed with GCC 14.2.0, and 7/7 passed with Clang 17.0.0 plus AddressSanitizer and UndefinedBehaviorSanitizer.
Sanitizer commands and coverage details are in
tests/README.md.Verification boundaries / why draft
The local environment could not clone GitHub or install the complete frontend dependencies. Verification used a dependency-isolated source fixture: complete
Savestate.cpp,Savestate.h,types.h, andGPU_OpenGL.cppwere checked against their Git blob hashes; the JIT method and argument layout were retrieved as source excerpts. A local-only declaration shim suppliedPlatform::Logfor the parser fixture; it is not part of this PR. The committed tests use the repository's realPlatform.hwhen run from a full checkout.The capture and JIT tests compile definitions extracted from the current production source at build time, with recording OpenGL / setter boundaries. They test byte ranges, dirty flags and argument forwarding, not real GL drivers, shader results, native JIT execution, or full core behavior. The source extractor is intentionally formatting-dependent and fails rather than falling back to a stale implementation.
Not yet verified here: complete application builds on supported platforms, actual game/DSi/savestate compatibility, real OpenGL readback, and end-to-end performance. New GitHub workflow results must be evaluated separately from the local results above. No claim that arbitrary malformed savestates are now safe; FIFO metadata, complete restore transactionality, and other parser boundaries remain outside this patch.
Performance direction
Retain C++ until an isolated comparison demonstrates a reason to change a component. Profile common 2D/capture/readback paths, software rasterizer work, and synchronization before selecting instruction-set optimizations. Any future AVX2/AVX-512 kernels should preserve exact guest-visible results, check the required CPU/OS features, keep a portable fallback, and beat the scalar/compiler-generated baseline on representative workloads.
masteris intentionally unchanged. This PR does not merge or publish anything upstream.