Repository navigation
Conversation
meta-cla
Bot
added
the
CLA Signed
|
@tmikov has exported this pull request. If you are a Meta employee, you can view the originating Diff in D117588421. |
Summary:
Two heap-value-mode build trees for the x86-64 JIT:
cmake-build-x86jit-hv32 -DHERMESVM_HEAP_HV_MODE=HEAP_HV_PREFER32
cmake-build-x86jit-boxed -DHERMESVM_HEAP_HV_MODE=HEAP_HV_BOXED
On x86-64, PREFER32 turns on `HERMESVM_COMPRESSED_POINTERS`,
`HERMESVM_BOXED_DOUBLES` and `HERMESVM_CONTIGUOUS_HEAP` together, while
HEAP_HV_BOXED turns on only `HERMESVM_BOXED_DOUBLES` and so isolates
the boxed-double decode from pointer compression. Between them they
give the first compile, and the first execution, to every mode-guarded
branch this backend transcribed blind across milestones 3 and 4.
No heap-mode emitter fix was required: every mode-guarded branch
compiled and ran correctly on the first try. The commit also carries
three small x86-64 emitter changes that apply in all modes:
`loadConstStringInGpX` now declines instead of asserting when the
lookup-entry offset exceeds a disp32; a debug-only `rspDelta_` counter
enforces the stack-argument rsp contract from `callImpl()` (a multiple
of 16 at `callRuntime()`, zero at instruction boundaries and in
`leave()`); and `fastArrayLoad`'s `isInTry()` sync gets a comment.
`objects.js` is reflowed to 80 columns.
Two earlier notes turn out to be wrong, and coverage rather than code
corrects them. `loadParentNoTraps` was recorded as unreachable; it
compiles now that calls and property access have landed, so `super`
reaches the nullable compressed-pointer decode. And
`newObjectWithBufferSlow` is reached in both boxed builds, not only
HV32: its trigger is `SmallHermesValue::canInlineDouble()` rejecting a
literal's double, which is a question about value bits, not about
pointer compression.
Differential Revision: D117588404
Summary: Port the 14 remaining scalar emitters from arm64, leaving 17 stubs: exceptions, switches, iterators, arguments and for-in. `typeOfIs` and `jmpTypeOfIs` are structurally verbatim ports, with arm64's `cset` becoming `setcc` plus a zero-extension and the condition codes mapped kEQ->kE, kLO->kB, kHI->kA, kLS->kBE. The result-initializing `mov`s inside the object and function cases sit between a comparison and the branch that reads it, so they have to stay flag-clean `mov`s and not become `xor`s. The code says so. `addS` has no inlined type check on either backend, since it is emitted only where both operands are already proved to be strings. The emitter says so, because the plan's text implies otherwise. Also adds the milestone-5 implementation plan, `doc/superpowers/plans/2026-08-24-x86-64-jit-exceptions.md`, which is the plan referred to above. Differential Revision: D117588405
Summary: A function with an exception table finally compiles, now that `leave()` no longer declines one. The SHJmpBuf/setjmp prologue has been in place since milestone 1 and was unreachable until now. Three x86-64 deltas, each commented at its site. arm64's `adr` and `br` become a RIP-relative `lea` and `jmp rax`. The label-delta table is explicitly 4-byte aligned, since it follows a variable-length `jmp` and `_jit_find_catch_target` reads it through an `int32_t *`. And `emit_sh_ljs_is_empty` was missing, so it joins its neighbours in `JitEmitter-internal.h`. The inherited `rspDelta_` scope obligation is resolved by folding `bumpAllocAndUnpoison`'s ASan pushes into the counter rather than documenting a split. They are balanced within the helper, and the 8 pushes total 64 bytes, a multiple of 16, so `callRuntime()`'s alignment assert now actually checks that call site instead of passing vacuously. The counter accounts for all transient `rsp` movement in the backend. `fastArrayLoad`'s `isInTry()` branch is live for the first time. Its comment said "unreachable today" and now says what drives it. Differential Revision: D117588406
Summary: Port the last twelve emitter stubs, so `JitEmitter-stubs.cpp` is empty and goes away. The only permanent decline left is `AsyncBreakCheck`, and it lives in the shared driver exactly as it does for arm64. `uintSwitchImm`'s jump table follows arm64's structure, 32-bit deltas relative to the table's own address emitted right after the indirect jump, with `lea`/`movsxd`/`add`/`jmp` in place of `adr`/`ldr`/`add`/`br`. It branches on both `jne` and `jp` after `emit_double_is_uint32()`, since `vucomisd` reports an unordered compare as equal. Without the `jp`, a NaN switch operand would be treated as the value 0 and, in a table whose `minVal` is 0, would run the first case. `reifyArgumentsImpl` widens the lazy register's recorded type after the merge point, which arm64 does not. The register is an in/out operand holding an object afterwards, while ISel's `LoadConstUndefined` left it recorded as a non-pointer. The type asserts caught it through `strictEqualImpl`'s raw-bit tier. Differential Revision: D117588407
Summary: The x86-64 backend has reached arm64's full opcode surface, and this adds the standing test that pins it. Grepping the x86-64 tree for `unsupported(` now finds two call sites, and neither declines an opcode. One is a shift-count register check in the two-register-temp path: the count must land in `rcx` for the x86 shift encoding, and a compare catches an allocator surprise instead of silently reading a stale `cl`. The other is a disp32-range check on the identifier lookup table offset, since `SymbolID`'s index is 28 bits and the theoretical offset can exceed INT32_MAX even though no real program gets close. Both decline an out-of-range encoding the same way arm64's own defensive checks would. The only real opcode decline anywhere in the tree is `AsyncBreakCheck`, in the architecture-independent driver, shared verbatim with arm64. There is no x86-64-specific unsupported list left at all. It also adds `-Xjit-crash-on-error` to the differential RUN lines of the existing x86-64 tests, since none of them declines any more. `exceptions.js` now prints `e.message`, so the two derived-constructor errors can be told apart, and pins the full threshold-mode compile chain. `reifyArgumentsImpl` asserts that its dead global-register reload never runs. The jmp_buf alignment comments are corrected, and the x86-64 section of `doc/JIT.md` is updated for milestone 5. Differential Revision: D117588408
Summary: test/jit was restricted to `jit-arch-arm64` because it was written against that backend and had never been audited for arm64-only assumptions. None of its 48 files checks backend-specific assembly; they are behavioral or compile-status tests, and the one that matches dump text (`getbyid-spec-exhausted.js`, the `// Read property cache` marker) matches a comment both backends emit. So the audit is the whole job, and it comes out clean. No file needed gating or adapting. The directory is now skipped only when no `jit-arch-*` feature exists at all, which keeps milestone 1's warning for an unrecognized architecture. Two files depend on a frame register getting a global, callee-saved register, and x86-64 has three of those against arm64's sixteen, so their allocations were checked against `-Xdump-jitcode=3` rather than assumed. `reset-fr-global.js` still puts its loop counter in one and is fully exercised. `new-object-parent-global-reg.js` does not, and is reduced to a plain behavioral test there. Both now say so. Differential Revision: D117588409
Summary:
Emitters that can throw call `syncAllFRTempExcept()` before the call
and exclude their own destination FR, assuming it is dead across the
call. Inside a try region that is wrong. Register allocation coalesces
a variable's phi with the destination, so the excluded FR is also the
one holding the value live into the catch handler. The prior value is
dropped rather than stored, and after the longjmp the handler reads the
frame slot, which still holds the zero fill `_sh_enter` wrote. So
var y = "prior"; try { y = o.throwingGetter } catch { use(y) }
yields 0, not "prior".
The JIT asserts cannot catch this. The FR holds a well-formed but stale
`HermesValue`, so it is a lost store, not a type error.
The fix is deferred to the maintainer. It spans 54 exclusion sites
per backend, 108 across both, or else requires liveness to treat
exceptional edges as uses, and this branch owes arm64 byte-identical
emission. The repro goes in the shared test/jit directory marked
`XFAIL: *`, so it runs on both architectures and a fix announces itself
as an XPASS. `doc/JIT.md`'s open question is replaced by the answer.
It also brings `doc/JIT.md`'s milestone-6 paragraph and the
`test/jit/lit.local.cfg` header comment up to date with the earlier
un-gating of `test/jit`, which neither had reflected.
Differential Revision: D117588410
Summary:
x86-64 needs less canonicalization than arm64. Its `loadBits64InGp` is
an unconditional `mov reg, imm64`, with no `isCheapConst` split and no
RO-data fallback for GP constants, so the mechanism that makes two
arm64 runs of the same binary diverge is absent. Confirmed against real
dumps: every differing line is a `mov` of an 8-or-more-digit hex
immediate, already collapsed by the existing trailing
`s/0x[0-9A-Fa-f]{8,}/ADDR/g` rule.
The only addition deletes `.dq` lines, dropping x86-64's RO_DATA
payload the way the existing rule already drops arm64's `.xword` lines.
Differential Revision: D117588411
Summary: The fifth build configuration, `cmake-build-x86jit-rel` (Release, clang, `HERMESVM_ALLOW_JIT=2`, no ASan), is the first non-ASan non-Debug x86-64 run in this port, so the first time NDEBUG strips every assert, including any with load-bearing side effects. Nothing diverged. Perf sanity on that build, interpreter against `-Xjit=force`, 3 runs, median: a hot numeric loop 2.25x, a scaled-up stress.js-shaped workload 1.61x, a property and IC heavy loop 1.45x. That clears the spec's "must not lose to the interpreter" bar with no tuning. The matrix goes in a new `doc/JITTesting.md` rather than into `doc/JIT.md`, which is already 1180+ lines mixing architecture with a findings appendix. Differential Revision: D117588413
Summary: The spec's Testing bullet was wrong: test/jit held no assembly goldens. The real golden artifact is the per-arch jit-dump baseline workflow, and un-gating was a gate-predicate change plus an arch-shape audit. Corrected, and a new Delivered section records the milestones, the deviations and the three findings left to the maintainer. `doc/JITTesting.md` now notes that Release reaches 68/1/1 over a different set of tests, that the arm64 leg is HV64 only, and when a dump baseline may roll forward. `utils/jit/README.md` and `jit-dump.sh` document that the trailing `ADDR` rule also hides genuine 64-bit immediates. `try-catch-dest-reg.js` now XFAILs per architecture, so a fix on one backend can un-XFAIL just that backend. Differential Revision: D117588414
Summary: The try-region destination-sync bug (both backends), the arm64 `reifyArguments` stale-type hole, and the `static_assert` message-form sweep, all found during the x86-64 JIT port. Differential Revision: D117588416
Summary: The one-argument form `static_assert(cond && "msg")` ANDs the message into the condition instead of using it as the diagnostic. All tag-helper asserts in both backends move to `static_assert(cond, "msg")`. The arm64 message for `HVTag_Str` was copy-pasted and claimed "HV_TagObject must be -1". The x86-64 comment that tracked that divergence goes away with it. Closes dz 01a03bbd-4888. Differential Revision: D117588417
Summary: `reifyArgumentsImpl` replaces the lazy-arguments register's undefined with a reified Arguments object on both the fast path, where it is already an object, and the slow path, where it is freshly allocated. arm64 never updated the register's recorded FR type to match. It stayed `FRType::OtherNonPtr`, the type recorded when ISel initializes the register with `LoadConstUndefined`, and nothing else in the emitted code touched that bookkeeping. The stale non-pointer claim propagates through `Mov`. Today its only consumer is `strictEqualImpl`'s raw-bit tier, which compares an object's bits correctly, since bit equality is object identity. So it produces no wrong results, but the recorded type is false: `-Xjit-emit-type-asserts` traps on it, and any future fast path that relies on `OtherNonPtr` more heavily would inherit the hole. x86-64 hit this and fixed it earlier under `-Xjit-emit-type-asserts`. arm64 gets the same `frUpdateType` call at the structurally equivalent point, right after the fast and slow paths join at `contLab` and before the slow-path lambda is registered. Closes dz 01a03bbd-4870. Differential Revision: D117588418
Summary:
Every emitter that can throw calls `syncAllFRTempExcept()` before the
call and passes its own destination FR as the exception, on the
assumption that the destination is dead across the call. Inside a try
region that assumption is wrong. Register allocation may coalesce a
live variable's phi with the destination, so the excluded FR is also
the FR holding a value live into the catch handler. When the
instruction throws instead of writing its destination, the handler
reads that frame slot and finds the zero fill `_sh_enter` wrote on
entry.
var y = "prior"; try { y = o.throwingGetter; } catch { use(y); }
printed 0 under `-Xjit=force` at -O, on arm64 and x86-64 alike, in all
three heap-value modes. The value is a well-formed `HermesValue`,
merely stale, so neither `-Xjit-emit-asserts` nor
`-Xjit-emit-type-asserts` could catch it: a lost store, not a bad tag.
Fixing it at the call sites would mean 54 exclusions per backend, 108
across both, each needing the same reasoning applied by hand. Instead
`syncAllFRTempExcept()` drops the exclusion itself when `isInTry()`.
That is sound because the exclusion was only ever an optimization.
Syncing an extra FR stores that FR's current value and nothing else,
and every call site issues the sync before allocating its destination
register, so the destination still holds its pre-instruction value at
sync time. Plain calls were never affected: `callImpl` already passes
`FR()` and syncs the whole frame.
The cost is one extra store per throwing instruction, and only in
functions containing a try. Those have no global registers, since
`RegisterAllocator::getRegClass` forces `RegClass::Other`, so the store
goes straight to the memory frame.
This deliberately changes emitted code on both backends, arm64
included. That backend's byte-identical-to-the-export-base constraint
was scoped to the port itself, not to a later cross-backend bug fix.
Both baselines were recaptured.
Closes dz 01a03bbd-02cf.
Differential Revision: D117588419
Summary: X-link: meta#2165 The spec's Delivered section said, as of milestone 6, that none of the three maintainer findings were fixed and that the arm64 reify-arguments hole was left unfixed deliberately. All three were fixed since, and `doc/JIT.md`'s findings list already documents that in detail. The historical record stays intact: each now-false present-tense claim is date-scoped to milestone 6 with a pointer to `doc/JIT.md`, rather than rewritten. `syncAllFRTempExcept`'s guard comment in both backends explained why excluding the destination FR is unsound in a try, but not the precondition its soundness rests on: callers must invoke the sync before allocating or writing the instruction's destination register, so the destination still holds its pre-instruction value when stored. That precondition is now written down, character-for-character identical between the arm64 and x86-64 files. Differential Revision: D117588421
tmikov
force-pushed
the
export-D117588421
branch
from
October 30, 2026 02:22
eb0bdf3 to
b9c251c
Compare
This branch was previously deployed
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:
X-link: #2165
The spec's Delivered section said, as of milestone 6, that none of the
three maintainer findings were fixed and that the arm64
reify-arguments hole was left unfixed deliberately. All three were
fixed since, and
doc/JIT.md's findings list already documents that indetail. The historical record stays intact: each now-false present-tense
claim is date-scoped to milestone 6 with a pointer to
doc/JIT.md,rather than rewritten.
syncAllFRTempExcept's guard comment in both backends explained whyexcluding the destination FR is unsound in a try, but not the
precondition its soundness rests on: callers must invoke the sync
before allocating or writing the instruction's destination register, so
the destination still holds its pre-instruction value when stored. That
precondition is now written down, character-for-character identical
between the arm64 and x86-64 files.
Differential Revision: D117588421