fix(jsc): correct ARM64 register-to-memory add64 operands - #12
Merged
Merged
Conversation
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.
ARM64 register-to-memory
add64currently overwrites its source register with twice the old memory value and stores the old value unchanged.ARM64Assembler::addtakes its destination first; put the loaded temporary first so the helper implements*dest += srcand preservessrc.The testmasm regression checks 169 operand pairs, 64-bit wraparound, source preservation, and adjacent memory. Native macOS ARM64 fails before the change with expected memory 1 versus actual 0, passes after, and passes all 466 assembler tests. Local qualification also passes 1,778 JSC stress configurations, 1,639 module configurations, testFFI, and five execution modes each for accounting and sampling. Native ARM64 CI adds the JSC regression selection, modules, and explicit interpreter/baseline/DFG/FTL/concurrent-GC accounting and sampling checks.
The ARM64 add/sub/and/or memory helpers and xor64 counterpart were audited with no sibling inversion found. The pinned upstream production register-to-memory add64 callers and Air memory forms are x86-64-only; the downstream DFG typed-array accounting path exposes the latent ARM64 bug. This corrects fast typed-array accounting; the separate Blob/Response lifetime issue is outside this change.
This is the same assembler correction and native ARM64 regression gate on
openclaw/main; the separately qualified release-line PR owns immutable artifact publication.The first native ARM64 run exposed four promise stack goldens on main that still expected call-parenthesis columns. This PR brings over the exact syntax-token expectations already qualified on the release line; all 68 focused configurations pass locally, with every assertion retained. The harness uses ENGINE_* variables to avoid the JSC engine-option environment namespace.
The qualification fixture isolates its hot busy loop in a small function so compiling the large surrounding fixture does not distort external-memory comparisons between samples. A controlled parent workload reproduced 30/30 failures before and 30/30 passes after the extraction; the 64 KiB bound, assertions, deadlines and JIT settings are preserved. Runtime code is unaffected by this test correction.