ppc64-QVM: zero-extend cached OP_LOAD1/LOAD2 instead of sign-extend - #412
Merged
Conversation
The register-caching fast path reduced an already-cached wider value to a byte/halfword with EXTSB/EXTSH (sign extend). OP_LOAD1/OP_LOAD2 are unsigned loads, so any cached byte >= 0x80 (or halfword >= 0x8000) came back negative, diverging from the interpreter and the fresh-load path (LBZ/LHZ). This corrupted byte-sized cgame values and made HUD/item icons disappear under the ppc64le JIT (CPMA, some OpenArena mods). Use CLRLDI to zero-extend, matching reduce_map_size() unsigned constant folding and the aarch64 JIT UXTB/UXTH. The genuine OP_SEX8/OP_SEX16 sign-extends are unaffected. Fixes ec-/Quake3e#411
Contributor
Author
|
@anengineer1 please try my fix |
Sorry, no luck so far it seems |
Contributor
Author
|
@anengineer1 did you build with release type? |
|
It works, sorry, I confused PRs and I ended up testing the NaN one |
runlevel5
marked this pull request as ready for review
June 29, 2026 22:47
…load The cached OP_LOAD1/LOAD2 fast path zero-extended the cached register in place. If that register was still a live value elsewhere on the opStack (e.g. a wider constant reused across stores of different sizes, or a prior load result still pending), the in-place reduction corrupted that value - the JIT returned a result differing from the interpreter by a power of two. Mirror the clone-on-alias guard already used by load_rx_opstack/finish_rx: when search_opstack() shows the register is live elsewhere, zero-extend a copy and leave the cached register untouched; otherwise reduce in place as before. OP_LOAD4 is unchanged. Found and verified with a JIT-vs-interpreter differential fuzzer: reproduced at iteration 17966 before, 0 divergences over 5,000,000 iterations after.
OP_LEF/OP_GEF mapped to a single CR0 test (branch if !GT / !LT). PPC FCMPU sets only CR0[SO] when an operand is NaN, leaving LT/GT/EQ clear, so those conditions wrongly took the branch on unordered operands - diverging from the interpreter and aarch64 (where NaN <= / >= is false). Fold the unordered bit into the tested bit via cror before the branch (GT |= SO for <=, LT |= SO for >=) so a NaN operand does not branch. Ordered comparisons and the other four float compares are unaffected; the integer LE/GE paths are untouched. Verified with a differential harness (JIT vs interpreter): NaN <= / >= 1.0 returned 1 on the JIT before and 0 after, matching the interpreter.
ec-
approved these changes
Jun 29, 2026
ec-
reviewed
Jul 3, 2026
ec-
left a comment
ec-
left a comment
Owner
There was a problem hiding this comment.
Good catch, thank you!
I guess that other JITs needs similar fixes, on x86/armv7l there is not much registers so search_opstack() never triggered for me
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
3 participants
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.
Fixes #411