Refactor EVM memory planning - #298
Merged
Merged
Conversation
Move the promote-vs-shadow local search (problem/state types, move search, packing-entry builders, cost model) and its tests out of memory_plan.rs, which now holds the plan data model and program-level assembly only. No behavior change.
The inline test module was more than half of ptr_escape.rs. Also point the PtrEscapeSummary doc at the production call-site derivation path (escape_scan) instead of a test-only helper.
- scratch_plan.rs -> fixed_slots.rs: the module is about the fixed spill slots at 0x00-0x3f, not the arena scratch region. Rename the scratch_effects/touches_scratch family to fixed_slot_effects/ touches_fixed_slots to match. - isa/evm/provenance.rs -> ptr_provenance.rs: avoids the collision with the unrelated transform/aggregate/provenance.rs. - static_chain_peak_words -> stable_chain_peak_words and StableMode::StaticAbs -> StableAbs: the call-surviving tier is consistently 'stable' now. - EvmCall/EvmCallCode ret_offset -> ret_len: the operand is the return buffer length; delete the comment apologizing for the misnomer. Snapshot diffs are the renamed dump labels only.
Add CallGraphSchedule (call graph + SCCs + callers-before-callees topo order + condensation edges) to module_analysis and thread it through the EVM backend, replacing eight independent CallGraph+SCC builds and three duplicated topo_sort_sccs/build_scc_edges copies. Rewrite the naive while-changed interprocedural fixpoints as single passes over the condensation: - compute_abs_clobber_words_with_extra: reverse-topo join over callees - compute_return_escape_caller_clamp_words: forward-topo join over callers (cyclic SCCs fold their own members' clobber bounds) - compute_entry_may_have_live_frame: forward-topo reachability from dynamic-frame functions - mem_effects: shared join_over_callees The duplicated stable-chain prefix layout in memory_plan.rs is also unified into compute_stable_chain_layout.
Extend FuncPreAnalysis with the CFG, pointer provenance (real escape summaries), and the deliberately conservative value-provenance variant that heap bounds rely on. The bundle is built once per section, right after critical-edge splitting; the source module is never mutated inside the spill-reserve convergence loop, so every consumer now reads the bundle instead of recomputing provenance and CFGs per call: - static arena object collection, malloc escape kinds, transient mallocs, heap future bounds, arena-base reservation scan, free-ptr slot facts, malloc placement, private-static malloc planning, and the free-pointer floor dataflow (which previously rebuilt CFG+provenance inside three nested interprocedural fixpoints). A debug assertion on instruction counts guards against future IR mutations invalidating the bundle.
Add machine/heap_state.rs: a shared forward-dataflow engine over the state of memory[0x40], instantiated by two passes — the exactness pass (pre-placement; drives Fixed-vs-Heap and private-static candidacy) and the numeric floor pass in free_ptr_floor (post-placement; its transfer reads MallocPlacement, which is why one fixpoint cannot serve both). - Delete ExactHeapBaseAnalysis and the duplicated inst_writes_free_ptr_slot helper. - Hoist per-function heap facts (exactness + terminal-private mallocs + dyn-sp clamp need) into FuncHeapFacts, computed once and shared by malloc placement and private-static planning, which previously each recomputed both analyses per function. - Rewrite compute_free_ptr_write_summaries as a condensation join (all callees are section-internal at this stage; asserted by the memory planner). - Document (PARITY notes) two preserved conservatisms: calls kill exactness unconditionally, and the floor merge loses information at any block with predecessors. Both are precision, not soundness; relaxing them is a separate snapshot-visible change.
FuncMemPlan served four phases: built by the semantic planner, retrofitted by reserve application, half-cleared by machine_mem_plan_from_semantic, remapped by hand, then grown by final spills. Split it: - SemanticFuncPlan: the planner output (object/alloca locations, frame words, call-preserve keyed by source insts, malloc annotations). The malloc annotation fields no longer need clearing — they simply do not exist on the machine type. - MachineFuncPlan: what lowering, final spills, and emission consume. Its only constructor, from_semantic(plan, func_map), forces the call_preserve source->machine key remap that was previously enforced by convention only (a missed remap silently dropped shadow saves). Also delete confirmed-dead surface: ObjLoc::StackPinned (every consumer panicked on it), the PreserveMode enum (collapsed into CallPreservePlan - ShadowRuns was the only live variant), ProgramMemoryPlan::sccs, CallSiteObjects::arg_count, and the semantic plan's always-empty spill_obj together with heap_plan's dead value_spill_bound path (spills exist only on the machine side since the stackify cutover). abs_words_end_with_extra_reserve becomes SemanticFuncPlan::abs_words_end_with_reserve with a doc note on how it deliberately differs from abs_words_end for StableMode::None.
- add_words: one overflow policy for word-count arithmetic in the
memory planner, replacing per-site checked_add(...).expect ceremony
where the operands are word counts.
- align_up_to_word: one copy replacing the identical helpers in
prepare.rs and final_spills.rs and backing lower.rs's malloc size
alignment.
- expect_func_entry: total-map lookup replacing the repeated
unwrap_or_else(|| panic!("missing ... for func")) incantations.
- prepare.rs's byte_ranges_overlap renamed to addr_len_overlaps_range:
it is not a duplicate of final_spills' end-based helper — it takes a
length and treats an overflowing end as overlapping (conservative);
the review's duplication claim was wrong in detail.
choose_optional_placements evaluated each scratch-vs-stable choice for optional final spills by re-running the entire program memory planner (every function's placement search, malloc planning, and free-pointer dataflows) - 2+N full replans per convergence iteration. Corpus measurement (SONATINA_MEM_LOOP_STATS): 313 sections, 91% converge in one iteration, 9% in two, none more; 21 sections carry optional spills and paid the replan amplification. The per-function placements are reserve-independent (reserves are applied after the placement search), so a trial reserve only shifts each function's frame words by its delta over the committed reserve. The new scorer recomputes the section aggregates directly: scratch peak (one max), stable chain peak (one pass over the condensation), arena base (the loop-invariant fixed-reservation scan is now computed once per section and combined with the per-trial facts). Two second-order feedback paths are held at their base values (private-static malloc extras; transient-malloc flips from new fixed-slot effects). The old replanning scorer is kept in debug builds as a cross-check oracle: SONATINA_SPILL_SCORE_XCHECK=1 compares every evaluation - 150 evaluations across the corpus, zero mismatches. This implements the sanctioned interim from the memory plans' R5; the full late-layout rework (R1) is deliberately left for its own effort: the measurement shows the convergence loop's cost is architectural rather than compile-time, and R1's symbolic-immediate stages need their own snapshot-reviewed landing sequence.
Corpus measurement (SONATINA_PLACEMENT_STATS, kept in-tree as a debug-only oracle): of 1220 placement problems, 93% have zero promotion candidates and every problem small enough to enumerate (N <= 14, 76 cases) is solved exactly optimally by the local search; 4 problems reach N = 31. The search's results are optimal-in-practice, so the machinery worth removing is the incremental move-evaluation layer, whose only purpose was speed the workload does not need. Score each candidate move by cloning the state, applying the move, and evaluating directly - deleting the trial iterators, per-move item-list builders, changed-shadow-call tracking, staged lower-bound short-circuit, and their dedicated tests (~400 lines). The debug cross-check that already asserted incremental == direct evaluation is what guarantees this refactor is behavior-identical; the snapshot corpus confirms it.
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.
Makes phase boundaries explicit and eliminates duplicated interprocedural analysis.