Repository navigation
perf(ir_passes): share repeated buffer property reads, descriptors and record addresses - #2044
Merged
Merged
Conversation
…into perf/property-buffer-reuse
nahime0
marked this pull request as ready for review
October 24, 2026 08:08
Contributor
[Medium impact] The PR appears safe to merge; no actionable new issues remain. SummaryShares repeated buffer-property reads, packed record addresses, and buffer descriptors within clean regions of a method.
Review checks were based on source inspection; tests were not run.
|
…f buffer properties
github-merge-queue
Bot
removed this pull request from the merge queue due to invalid changes in the merge commit
Oct 24, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to invalid changes in the merge commit
Oct 24, 2026
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.
Was stacked on #2036, which has merged; main is merged into this branch, so the diff is this PR's own change.
Problem
Inside a method, every
$this->slots[$index]->fieldaccess reloads the property, calls__rt_buffer_resolveand recomputes the record address. The #1959 hoisting only covers loops whose buffer handle is loop-invariant (locals, parameters); a property read inside the body is neither, and straight-line code is never looked at. Ivory'sEntities::matches()(the shapebench/ecscalls once per entity) makes two such reads per call.Base (main f708b38, debug build) EIR of the task's
Table::matches:On linux-x86_64 each
prop_getis a null-receiver test, a typed-property initialization test and a load, and eachbuffer_getspills the handle and index aroundcall __rt_buffer_resolve.Change
A new straight-line phase
src/ir_passes/buffer_property_reuse.rsruns insidebuffer_accessafter loop hoisting (same generator, exception-handler,evalandpcntl_async_signals()guards). Three rewrites, each only across a clean region (every instruction on any path from the earliest instruction either access depends on to the later access):prop_getof the same property on a matching receiver is replaced by the dominating read.buffer_getof the same property-read handle and a matching index reuses the dominating record address.buffer_resolveinserted right before the leader and becomebuffer_get_resolved/buffer_set_resolved(so a stale handle still dies with the use-after-free fatal at the leader).Only property reads whose receiver is the method's own
$thisare trusted, as candidates and inside a region: effect refinement drops the null-receiver warning of any other receiver (a nullable one, or a non-nullable parameter a dynamic call passednull; filed as task b153a540), and that warning can run a user error handler that replaces or frees the buffer. Modules where a class declares__get, or where the eval bridge can declare one, skip the phase, so no trusted read dispatches to user code.The region classifier is default-deny: the loop pass's buffer-preserving allowlist minus every opcode that can run user code (even a call with a read-only effect summary, since summaries drop the callee's local effects and matched slots are not proven free of address escapes), plus the no-op
acquire/releaseof a buffer handle and named property reads refined toreads_heap, may_throw. A property store orunseton any object (another reference may alias the receiver),buffer_free(), a call, a release, an allocation, a warning or output ends the region. Receivers and indexes match when they are one SSA value (seen through trivial join parameters), equal integer constants, or loads of one stable slot (a by-value PHP local that onlyload_local/store_localname) that the region does not write. A bufferacquireleft with nothing but itsrelease(both emit no code for buffers) is dropped with it. Local buffer handles in straight-line code are left alone.Expected EIR of
Table::matchesafter the change (asserted by the tests): oneprop_getofslots, onebuffer_get, nobuffer_resolve;logical.rhskeeps only the field readprop_get v4 data[2]on the first record address. A method touching two indexes of one property (swapMasks) gets onebuffer_resolveand twobuffer_get_resolved.Small shared-code changes in
src/ir_passes/buffer_access.rs(from #2036):optimize_functionnow calls the loop hoisting (hoist_loops) and then the new phase, andinstruction_preserves_buffers,slots_named_as_storage,insert_instruction,incoming_argumentsandcanonical_valuebecomepub(super).Tests
src/ir_passes/tests/buffer_property_reuse_test.rs(hand-built EIR): shared property read and record address, one descriptor for two indexes, and the reads stay apart across a property store through another reference,unset,buffer_free(), a read-only user call, a property read on another receiver, an index write (descriptor still shared, records not), another receiver, functions without$this, and local handles.tests/codegen/optimizer/buffer_property_reuse.rs(EIR shape plus optimizer on/off runtime parity):Table::matches,Table::swapMasks, the same reads inside a loop body (countMatching), an aliasing property store ($alias->cells = ...with$alias === $this), an index write,buffer_free()or a freeing call between the reads, which must keep the use-after-free fatal in both modes, a null-receiver warning whose error handler replaces or frees the buffer between the reads, and a program declaring__get, which turns the phase off. The fixture outputs were checked against the base compiler in both modes.examples/buffer-property-table/main.php(an ECS-style slot table), compiled and run bytest_buffer_property_table_example_shares_property_reads.Docs:
docs/internals/the-optimizer.md,docs/compiling/optimization.md,docs/compiling/source-maps.md.Limitations
$thisare left alone. Once a method such asEntities::matches()is inlined into a caller's loop, its receiver becomes the caller's$entitiesparameter, which a dynamic call can set tonull, so those inlined reads are not shared until effect refinement proves receivers non-null (task b153a540).$this->slots[$i]still resolves per access across iterations (this PR shares the work within one iteration); hoisting the property read out of such loops is task ad0374ba.No benchmark was run locally (shared machine); the EIR shape above is the evidence, and a benchmark run (Ivory
bench/ecs) is left to CI or the maintainer.