Lift srt 3 / rsrt 2 / rsrt 3 closure-bind to native VM dispatch (PR B of ILO-45)#699
Merged
Conversation
PR B of ILO-45. Adds the descending-sort finalizer opcode and a ctx-threading variant of the keyed-finalize helper, lifting three more HOFs off the tree-bridge: - rsrt fn xs (2-arg, descending sort by key) - srt fn ctx xs (3-arg, closure-bind ascending) - rsrt fn ctx xs (3-arg, closure-bind descending) New opcode OP_RSRT_BY_KEY=191, dispatch arm in the VM, jit_rsrt_by_key extern helper, and Cranelift FuncId + dispatch arm in both compile and JIT paths so AOT and JIT both inherit the lift. The two finalizer shapes (asc/desc) share srt_by_key_finalize_inner via a descending bool, so the comparator-reversal logic lives in one place. emit_hof_keyed_finalize gains an emit_hof_keyed_finalize_ctx sibling that threads an extra ctx register through OP_CALL_DYN with argc=2, mirroring PR A's map/flt/fld ctx arms. The non-ctx path is unchanged. Three more entries removed from is_tree_bridge_eligible. What remains on the bridge for FnRef-taking builtins: ct 2 / ct 3 (PR C). After that, non-HOF bridge entries get audited for transitive eval_body reachability (PR D) before the eval loop itself can be deleted (PR E). AOT object baselines regenerated for all 136 entries since the new helper FuncId shifts symbol layout in every compiled program. Test plan: cargo test --release --features cranelift green. New opcode exercised via examples/rsrt-by-key.@ (worst-by-abs / longest- words / top-scaled — already in repo, now go through native dispatch rather than the bridge). Smoke test across default / --vm / --jit gives identical output for ascending and descending closure-bind sorts.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
After PR B lifted rsrt 3-arg from the tree-bridge to native VM + Cranelift dispatch, the mget misuse now raises directly with its native code (ILO-R004 "key must be text or finite number") rather than being remapped through the bridge to the generic ILO-R009. The test's intent was "callback errors must surface on every engine" - still true. Same shape as the existing flatmap test, which already asserts the shared substring rather than a specific code.
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
1 participant
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
PR B of ILO-45 — second slice of the tree-walker eval-loop deletion. Adds the descending-sort finalizer opcode and a ctx-threading variant of the keyed-finalize helper, lifting three more HOFs to native VM + Cranelift dispatch.
Builds on PR A (#619) which lifted
map 3/flt 3/fld 4. After this PR, the only FnRef-taking entries left on the tree-bridge arect 2/ct 3(PR C). Then the non-HOF bridge entries get audited for transitiveeval_bodyreachability (PR D), and only then can the eval loop actually be deleted (PR E).What's in the diff
Single commit, four files:
src/vm/mod.rsOP_RSRT_BY_KEY = 191opcode with full ABC contract docstringjit_rsrt_by_keyextern helper, thin wrapper overrsrt_by_key_finalizesrt_by_key_finalizeand newrsrt_by_key_finalizesharesrt_by_key_finalize_inner(_, _, descending: bool). The descending path usesOrdering::reverse—Equal.reverse() == Equalkeeps stable-sort identity on mixed-type keys, matching the tree-walkeremit_hof_keyed_finalizenow delegates to_innerwithctx: None; newemit_hof_keyed_finalize_ctxpassesSome(ctx_arg). Threads an extra register through OP_CALL_DYN with argc=2 for the closure-bind path. Non-ctx callers (srt 2 / grp 2 / uniqby 2) emit byte-identical bytecode(Builtin::Rsrt, 2),(Builtin::Srt, 3),(Builtin::Rsrt, 3)is_tree_bridge_eligiblesrc/vm/compile_cranelift.rsandsrc/vm/jit_cranelift.rsrsrt_by_key: FuncIdfield in the helpers structdeclare_helper(module, "jit_rsrt_by_key", 3, 1)registration("jit_rsrt_by_key", jit_rsrt_by_key as *const u8))tests/aot-baselines/obj-baselines.tsvrsrt_by_keyto the helpers struct shifts symbol layout for every compiled program. Partial drift would have been the red flagTest plan
cargo test --release --features cranelift— full suite green, no regressions--vm/--jitexamples/rsrt-by-key.@already exercisesrsrt fn xsandrsrt fn ctx xs— they now go through native dispatch instead of the bridge; output unchangedsuperpowers:code-reviewersubagent run on the diff — no blockersFollow-ups
ct 2/ct 3— needs anOP_COUNT_BY_PREDfinalizer or inline-counter VM loopeval_bodyreachability; lift any that need iteval_body/eval_stmt/eval_expr/Env/ trampoline /ACTIVE_AST_PROGRAM(headline LoC win)