From 5e28b1710012d9388e1ed8f8872f8bf8fd1ada56 Mon Sep 17 00:00:00 2001 From: Hironobu Sano Date: Wed, 26 Aug 2026 10:22:40 +0900 Subject: [PATCH 1/4] docs: plan shared recursive drop codegen --- docs/impl/21-build-perf-plan.md | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/docs/impl/21-build-perf-plan.md b/docs/impl/21-build-perf-plan.md index df60eabd..91d23955 100644 --- a/docs/impl/21-build-perf-plan.md +++ b/docs/impl/21-build-perf-plan.md @@ -21,6 +21,7 @@ Order is priority. | 2a | Required DB owner build-once/run-many | Shipped in #882 — exact-set concurrent execution across four isolated CI shards; required wall time fell from about 60 minutes to 15:25 while every shard kept the hard 30-minute budget | | 2b | DB CI changed-function scope | Implemented — direct DB/gate and dedicated DB-production paths remain unconditional, while mixed compiler sources provision PostgreSQL only when a changed zero-context hunk or its function header names the database boundary | | 3 | Pipelined compilation | Shipped as #884. A dependent unit's frontend starts as soon as each dependency interface summary exists while already-ready codegen runs within the same `-j` budget; validation, publication, and retry follow the ledger below | +| 3a | Shared recursive-Drop codegen | Implementing for align-llm Request 19 — emit one private pointer-based destructor per reachable Move struct instead of cloning its recursive cleanup CFG at every Drop site | | 4 | Prebuilt optimized cache distribution | Design settled below; implementation pending — ship warmed first-party `pkg` entries with each exact native compiler (compiler-provided `core`/`std` imports have no cacheable source unit) | | 5 | Daemon / watch mode | Keep the in-process memo alive across builds; the main lever for AI-agent edit-compile loops. `align-repl` (`docs/impl/22-repl-plan.md`) is the first consumer of this lever: it is already a long-lived process, so it realizes memo residency with no daemon machinery | | 6 | Function-level incremental compilation | Heaviest; requires its own design ledger before any implementation | @@ -31,6 +32,57 @@ Measured on `pkg.db`: cold per-unit codegen ~9s versus frontend ~3.4s. Caching (items 1–4) removes re-paying, parallelism (3) and residency (5) shorten the wall clock, and granularity (6) shrinks the unit of re-payment. +## Item 3a: shared recursive-Drop codegen + +### Measured problem and contract + +align-llm Request 19 supplies the first real-client owner: a 1,573-line verifier +fixture with 39 wide Move-record types and many early exits. At Align +`f57b986bc9326ba8d75dad5dbe4c6531c0f872b6` on Linux x86_64, +`alignc check` completes in 2.23 seconds and raw LLVM emission completes in 7.80 +seconds, but the fixture unit's `main` contains about 1.42 million raw IR lines. +The dominant repeated shape is not an aggregate load or store: every cleanup +site receives a fresh copy of the complete recursive Drop CFG, including loops +over owned dynamic arrays. A cold optimized build remains inside one LLVM job +after minutes while its resident set grows past 800 MiB. + +This item changes compiler-internal lowering only. Language ownership, +source-visible diagnostics and their order, MIR, interfaces, runtime ABI, +package ABI, allocation, generated-program effects, cleanup eligibility, field +order, active tagged-arm selection, element order, and exactly-once Drop remain +unchanged. Codegen emits one private `nounwind void(ptr)` helper for each Move +struct whose destructor is reached in a module. Each ordinary struct Drop, +replacement Drop, fixed-array element Drop, and dynamic-array element Drop +passes the exact existing storage pointer to that helper. The helper contains +the existing canonical pointer-based Drop plan once and returns only after all +children have been released in their current order. Its symbol is +module-private and derived only from the module-local struct id; it is neither +an interface symbol nor a cross-unit dependency. + +The running `alignc` byte hash already participates in every codegen, prelink, +and backend cache key, so changed helper emission invalidates old objects +without a cache-format or manifest-format change. Frontend cache entries remain +valid because HIR, MIR, interfaces, and their formats do not change. + +### Implementation closure matrix + +| Axis | Required closure | Owner | +| --- | --- | --- | +| Formation and construction | Copy structs emit no helper. Every reachable Move struct gets at most one private, defined `void(ptr)` helper, even when first reached from nested, tagged, fixed-array, or dynamic-array cleanup. A missing/out-of-range type record remains a diagnosed lowering error rather than a panic. | `align_codegen_llvm` helper inventory and malformed-id unit owners | +| Move-in, move-out, and source nulling | Moves keep the existing aggregate transfer and cleanup-bit behavior. The helper receives only the selected live storage pointer; moved or uninitialized storage remains zeroed before a possible call, so null-safe leaves stay null-safe. | existing Move struct transfer/nulling owners plus helper IR assertions | +| Normal Drop and replacement | Standalone struct Drop, reassignment, whole-field replacement, fixed Move-struct array element replacement, and dynamic Move-struct array element cleanup all call the same helper. Field and element order is byte-for-semantic identical to the former inline plan. | focused codegen IR owner; existing nested/owned-array runtime owners | +| Control exits | `if`, `match`, `else`, `?`, `map_err`, branch joins, loop back edges and breaks, return, and early error exits retain their existing cleanup guards and call the helper only on the same live paths. A terminating path manufactures no helper call. | existing ownership/control regression targets; Request 19 raw-IR call-count bound | +| Nested and tagged graphs | Nested Move structs, `Option`, `Result`, user sums, `array`, `array`, handles, resources, and recursively owned record arrays retain active-arm selection, loop bounds, native thunk choice, and exact child-before-parent restoration order. | parameterized Drop-plan/codegen owners and runtime exactly-once controls | +| Direct, imported, generic, and function-value paths | Whole-program and per-unit compilation emit equivalent private helpers in each owning module. Generic instances follow their concrete module-local struct ids. Calls, returns, imports, and function-value ABI are unchanged. | whole/per-unit IR and executable parity owners | +| Runtime and allocation parity | Helpers allocate nothing and perform no artifact/source I/O. They call the same runtime free/handle/resource thunks with the same pointers and counts. Helper calls are `nounwind`; no unwind cleanup path is introduced. | IR call inventory and existing allocation/failpoint owners | +| Cache and artifact identity | The running compiler-byte hash invalidates every affected object/prelink/backend key; no persisted field changes. Same compiler and inputs remain byte-deterministic, including parallel per-unit builds. | cache edit/revert and deterministic-object owners | +| Resource promise | The Request 19 fixture's raw IR no longer scales with cleanup sites times the recursive Drop graph. Its optimized build completes within the consumer's per-target budget with peak memory well below the recorded 1,525,732 KiB, and output remains byte-identical. | local `bench/large_drop_codegen` measurement plus align-llm `make prompt-verifier-smoke`; final consumer lane/fresh-worker proof belongs to align-llm | + +The implementation boundary is one codegen capability because helper creation +and every consuming Drop site must agree in the same module. Splitting a dormant +helper producer from call-site conversion would add unreachable code without a +stable consumer and would duplicate the correctness proof. + ## Item 3: pipelined frontend and codegen ### Boundary and public contract ledger From 2c15ce7f11751f50c6f319bd2e71f2a0473ecf3d Mon Sep 17 00:00:00 2001 From: Hironobu Sano Date: Wed, 26 Aug 2026 10:32:40 +0900 Subject: [PATCH 2/4] fix(plan): close drop codegen review findings --- docs/impl/21-build-perf-plan.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/impl/21-build-perf-plan.md b/docs/impl/21-build-perf-plan.md index 91d23955..f5f8984d 100644 --- a/docs/impl/21-build-perf-plan.md +++ b/docs/impl/21-build-perf-plan.md @@ -54,10 +54,14 @@ unchanged. Codegen emits one private `nounwind void(ptr)` helper for each Move struct whose destructor is reached in a module. Each ordinary struct Drop, replacement Drop, fixed-array element Drop, and dynamic-array element Drop passes the exact existing storage pointer to that helper. The helper contains -the existing canonical pointer-based Drop plan once and returns only after all -children have been released in their current order. Its symbol is -module-private and derived only from the module-local struct id; it is neither -an interface symbol nor a cross-unit dependency. +the existing canonical pointer-based **iterative** Drop plan once and returns +only after all children have been released in their current order. A generated +helper never calls another compiler-generated Drop helper: nested structs and +tagged/array children stay on that helper's compiler-emitted iterative CFG, so +generated-program call-stack depth is one regardless of nominal type depth. +Its symbol is module-private and its authoritative handle is indexed only by +the module-local struct id; it is neither an interface symbol nor a cross-unit +dependency. The running `alignc` byte hash already participates in every codegen, prelink, and backend cache key, so changed helper emission invalidates old objects @@ -72,11 +76,11 @@ valid because HIR, MIR, interfaces, and their formats do not change. | Move-in, move-out, and source nulling | Moves keep the existing aggregate transfer and cleanup-bit behavior. The helper receives only the selected live storage pointer; moved or uninitialized storage remains zeroed before a possible call, so null-safe leaves stay null-safe. | existing Move struct transfer/nulling owners plus helper IR assertions | | Normal Drop and replacement | Standalone struct Drop, reassignment, whole-field replacement, fixed Move-struct array element replacement, and dynamic Move-struct array element cleanup all call the same helper. Field and element order is byte-for-semantic identical to the former inline plan. | focused codegen IR owner; existing nested/owned-array runtime owners | | Control exits | `if`, `match`, `else`, `?`, `map_err`, branch joins, loop back edges and breaks, return, and early error exits retain their existing cleanup guards and call the helper only on the same live paths. A terminating path manufactures no helper call. | existing ownership/control regression targets; Request 19 raw-IR call-count bound | -| Nested and tagged graphs | Nested Move structs, `Option`, `Result`, user sums, `array`, `array`, handles, resources, and recursively owned record arrays retain active-arm selection, loop bounds, native thunk choice, and exact child-before-parent restoration order. | parameterized Drop-plan/codegen owners and runtime exactly-once controls | +| Nested and tagged graphs | Nested Move structs, `Option`, `Result`, user sums, `array`, `array`, handles, resources, and recursively owned record arrays retain active-arm selection, loop bounds, native thunk choice, and exact child-before-parent restoration order. Helper bodies use the existing compiler-owned iterative worklist/CFG and never call a generated Drop helper, so a 4,096-record valid acyclic graph executes with one helper stack frame rather than a type-depth call chain. | parameterized Drop-plan/codegen owners, an executable deep finite graph stack-bound owner, and runtime exactly-once controls | | Direct, imported, generic, and function-value paths | Whole-program and per-unit compilation emit equivalent private helpers in each owning module. Generic instances follow their concrete module-local struct ids. Calls, returns, imports, and function-value ABI are unchanged. | whole/per-unit IR and executable parity owners | | Runtime and allocation parity | Helpers allocate nothing and perform no artifact/source I/O. They call the same runtime free/handle/resource thunks with the same pointers and counts. Helper calls are `nounwind`; no unwind cleanup path is introduced. | IR call inventory and existing allocation/failpoint owners | | Cache and artifact identity | The running compiler-byte hash invalidates every affected object/prelink/backend key; no persisted field changes. Same compiler and inputs remain byte-deterministic, including parallel per-unit builds. | cache edit/revert and deterministic-object owners | -| Resource promise | The Request 19 fixture's raw IR no longer scales with cleanup sites times the recursive Drop graph. Its optimized build completes within the consumer's per-target budget with peak memory well below the recorded 1,525,732 KiB, and output remains byte-identical. | local `bench/large_drop_codegen` measurement plus align-llm `make prompt-verifier-smoke`; final consumer lane/fresh-worker proof belongs to align-llm | +| Resource promise | The Request 19 fixture's raw IR no longer scales with cleanup sites times the recursive Drop graph. Its optimized build completes within the consumer's per-target budget with peak memory well below the recorded 1,525,732 KiB, and output remains byte-identical. A representative small one-shot Move-record program is measured before and after for frontend/codegen work counts, wall time, peak memory, object size, and cleanup runtime; the optimization is not accepted if that unaffected path shows a material regression outside run-to-run spread. Counts come from actual compiler/cache outcomes and executed destructor counters, not an expected source/unit count. | local `bench/large_drop_codegen` pathological and unaffected controls plus align-llm `make prompt-verifier-smoke`; final consumer lane/fresh-worker proof belongs to align-llm | The implementation boundary is one codegen capability because helper creation and every consuming Drop site must agree in the same module. Splitting a dormant From 4f3372df60d4974c1aa080ab5423454aa769157d Mon Sep 17 00:00:00 2001 From: Hironobu Sano Date: Wed, 26 Aug 2026 11:03:03 +0900 Subject: [PATCH 3/4] perf(codegen): share recursive struct drop helpers --- bench/large_drop_codegen/README.md | 29 ++ bench/large_drop_codegen/control.align | 27 ++ bench/large_drop_codegen/measure.py | 39 ++ bench/large_drop_codegen/run.sh | 89 ++++ crates/align_codegen_llvm/src/drop_codegen.rs | 31 +- crates/align_codegen_llvm/src/lib.rs | 426 +++++++++++------- .../align_driver/tests/large_drop_codegen.rs | 168 +++++++ docs/impl/21-build-perf-plan.md | 24 +- 8 files changed, 649 insertions(+), 184 deletions(-) create mode 100644 bench/large_drop_codegen/README.md create mode 100644 bench/large_drop_codegen/control.align create mode 100755 bench/large_drop_codegen/measure.py create mode 100755 bench/large_drop_codegen/run.sh create mode 100644 crates/align_driver/tests/large_drop_codegen.rs diff --git a/bench/large_drop_codegen/README.md b/bench/large_drop_codegen/README.md new file mode 100644 index 00000000..d43b0f3f --- /dev/null +++ b/bench/large_drop_codegen/README.md @@ -0,0 +1,29 @@ +# Shared recursive-Drop codegen benchmark + +This local, non-gating benchmark owns the resource promise in +`docs/impl/21-build-perf-plan.md` item 3a. It compares optimized compiler +binaries from the pre-change and candidate revisions on two inputs: + +- `control.align`, a one-shot small Move record whose instrumented runtime must + print its value and report exactly one allocation and one destructor free; and +- align-llm Request 19's `prompt_verifier_smoke.align`, the pathological wide + Move-record and early-exit client fixture. + +Build each revision's `alignc` and then rebuild the adjacent runtime archive +with `align_runtime`'s `alloc-count` feature. Keep each binary and archive in a +separate directory, then run: + +```text +BASELINE_ALIGNC=/path/to/baseline/alignc \ +CANDIDATE_ALIGNC=/path/to/candidate/alignc \ +REQUEST19_SOURCE=/path/to/align-llm/src/prompt_verifier_smoke.align \ +bench/large_drop_codegen/run.sh +``` + +The harness uses fresh caches to report actual frontend/codegen miss counts, +then reports raw-IR lines, release object bytes, wall time, and peak compiler +RSS. It requires byte-identical program output, the exact `1 / 1` control +allocation/free result, and Request 19's PASS line. Both arms use `--no-rt-lto` +so the allocation counters come from the instrumented archive rather than the +ordinary runtime bitcode. Run the real consumer build separately with its +default runtime-LTO policy before accepting the item. diff --git a/bench/large_drop_codegen/control.align b/bench/large_drop_codegen/control.align new file mode 100644 index 00000000..ff676d02 --- /dev/null +++ b/bench/large_drop_codegen/control.align @@ -0,0 +1,27 @@ +extern "C" { + fn align_rt_alloc_count() -> i64 + fn align_rt_free_count() -> i64 +} + +SmallDrop { text: string, number: i64 } + +fn allocations() -> i64 { + unsafe { return align_rt_alloc_count() } +} + +fn frees() -> i64 { + unsafe { return align_rt_free_count() } +} + +fn one_drop() { + value := SmallDrop { text: "control".clone(), number: 7 } + print(value.number) +} + +fn main() { + allocations_before := allocations() + frees_before := frees() + one_drop() + print(allocations() - allocations_before) + print(frees() - frees_before) +} diff --git a/bench/large_drop_codegen/measure.py b/bench/large_drop_codegen/measure.py new file mode 100755 index 00000000..f9e3d54a --- /dev/null +++ b/bench/large_drop_codegen/measure.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +"""Run one benchmark command and report wall time plus child peak RSS as JSON.""" + +import json +import os +import resource +import subprocess +import sys +import time + + +def main() -> int: + if len(sys.argv) < 4 or sys.argv[2] != "--": + print("usage: measure.py WORKDIR -- COMMAND [ARG ...]", file=sys.stderr) + return 2 + workdir = sys.argv[1] + command = sys.argv[3:] + start = time.monotonic() + completed = subprocess.run( + command, + cwd=workdir, + env=os.environ.copy(), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + elapsed = time.monotonic() - start + rss = resource.getrusage(resource.RUSAGE_CHILDREN).ru_maxrss + if sys.platform == "darwin": + rss //= 1024 + if completed.returncode != 0: + sys.stdout.buffer.write(completed.stdout) + sys.stderr.buffer.write(completed.stderr) + return completed.returncode + print(json.dumps({"wall_seconds": round(elapsed, 3), "peak_rss_kib": rss}, sort_keys=True)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/bench/large_drop_codegen/run.sh b/bench/large_drop_codegen/run.sh new file mode 100755 index 00000000..b8b6efa2 --- /dev/null +++ b/bench/large_drop_codegen/run.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Local Request-19 compile-cost comparison. This is a measurement, not a CI gate. +set -euo pipefail + +here="$(cd "$(dirname "$0")" && pwd)" +baseline="${BASELINE_ALIGNC:-}" +candidate="${CANDIDATE_ALIGNC:-}" +request_source="${REQUEST19_SOURCE:-$here/../../../align-llm/src/prompt_verifier_smoke.align}" + +if [ -z "$baseline" ] || [ -z "$candidate" ]; then + echo "set BASELINE_ALIGNC and CANDIDATE_ALIGNC to optimized compiler binaries" >&2 + exit 2 +fi +for compiler in "$baseline" "$candidate"; do + if [ ! -x "$compiler" ]; then + echo "compiler is not executable: $compiler" >&2 + exit 2 + fi + if [ ! -f "$(dirname "$compiler")/libalign_runtime.a" ]; then + echo "instrumented libalign_runtime.a is missing beside $compiler" >&2 + exit 2 + fi +done +if [ ! -f "$request_source" ]; then + echo "Request 19 source is missing; set REQUEST19_SOURCE explicitly" >&2 + exit 2 +fi + +work="$(mktemp -d)" +cleanup() { + rm -rf "$work" +} +trap cleanup EXIT + +measure_case() { + revision="$1" + compiler="$2" + case_name="$3" + source="$4" + dir="$work/$revision-$case_name" + mkdir -p "$dir" + # Preserve sibling modules imported by the entry fixture. The Request 19 owner imports + # prompt_artifacts.align and prompt_score.align from its source directory. + cp "$(dirname "$source")"/*.align "$dir/" + cp "$source" "$dir/main.align" + + ( + cd "$dir" + ALIGNC_CACHE="$dir/cache" "$compiler" build main.align --profile release --no-rt-lto \ + --cache-stats >build.stdout 2>build.stderr + ) + frontend_misses="$(awk '/^alignc: cache: .* frontend miss \(/ { count++ } END { print count + 0 }' "$dir/build.stderr")" + frontend_hits="$(awk '/^alignc: cache: .* frontend hit$/ { count++ } END { print count + 0 }' "$dir/build.stderr")" + codegen_misses="$(awk '/^alignc: cache: .* miss \(/ && $0 !~ / frontend miss \(/ { count++ } END { print count + 0 }' "$dir/build.stderr")" + codegen_hits="$(awk '/^alignc: cache: .* hit$/ && $0 !~ / frontend hit$/ { count++ } END { print count + 0 }' "$dir/build.stderr")" + if [ "$frontend_hits" -ne 0 ] || [ "$codegen_hits" -ne 0 ]; then + echo "$revision/$case_name unexpectedly hit a fresh cache" >&2 + sed -n '/^alignc: cache:/p' "$dir/build.stderr" >&2 + exit 1 + fi + + ( + cd "$dir" + "$compiler" emit-llvm main.align --stage raw --no-rt-lto >raw.ll + "$compiler" emit-obj main.align main.o --profile release --no-rt-lto + ) + timing="$(ALIGNC_CACHE=off python3 "$here/measure.py" "$dir" -- \ + "$compiler" build main.align --profile release --no-rt-lto)" + "$dir/main" >"$dir/program.stdout" + + raw_lines="$(wc -l <"$dir/raw.ll" | tr -d ' ')" + object_bytes="$(wc -c <"$dir/main.o" | tr -d ' ')" + echo "$revision/$case_name: frontend_misses=$frontend_misses codegen_misses=$codegen_misses raw_ir_lines=$raw_lines object_bytes=$object_bytes timing=$timing" +} + +measure_case baseline "$baseline" control "$here/control.align" +measure_case candidate "$candidate" control "$here/control.align" +cmp "$work/baseline-control/program.stdout" "$work/candidate-control/program.stdout" +if [ "$(tr '\n' ' ' <"$work/candidate-control/program.stdout")" != "7 1 1 " ]; then + echo "small control must report value 7, one allocation, and one destructor free" >&2 + exit 1 +fi + +measure_case baseline "$baseline" request19 "$request_source" +measure_case candidate "$candidate" request19 "$request_source" +cmp "$work/baseline-request19/program.stdout" "$work/candidate-request19/program.stdout" +grep -Fx "prompt verifier smoke: complete, incomplete, compact, and tamper cases PASS" \ + "$work/candidate-request19/program.stdout" >/dev/null +echo "program output: identical; control destructor count: 1/1; Request 19: PASS" diff --git a/crates/align_codegen_llvm/src/drop_codegen.rs b/crates/align_codegen_llvm/src/drop_codegen.rs index 83a97db8..de2f7075 100644 --- a/crates/align_codegen_llvm/src/drop_codegen.rs +++ b/crates/align_codegen_llvm/src/drop_codegen.rs @@ -78,6 +78,19 @@ impl<'c, 'a> FnGen<'c, 'a> { &self, base: inkwell::values::PointerValue<'c>, ty: Ty, + ) -> Result<(), CodegenError> { + self.emit_drop_at_iterative_in(self.func, base, ty) + } + + /// The target-selectable form used to define a private struct destructor while an ordinary + /// function body is being emitted. Cleanup itself remains one iterative CFG inside `function`; + /// it never calls another generated struct helper, so runtime stack depth does not follow the + /// nominal type depth. + pub(super) fn emit_drop_at_iterative_in( + &self, + function: FunctionValue<'c>, + base: inkwell::values::PointerValue<'c>, + ty: Ty, ) -> Result<(), CodegenError> { enum Work<'c> { Drop { @@ -148,8 +161,8 @@ impl<'c, 'a> FnGen<'c, 'a> { .build_load(self.ctx.i8_type(), tag_ptr, "dropopttag") .map_err(|error| self.err(error))? .into_int_value(); - let some = self.ctx.append_basic_block(self.func, "drop.opt.some"); - let cont = self.ctx.append_basic_block(self.func, "drop.opt.cont"); + let some = self.ctx.append_basic_block(function, "drop.opt.some"); + let cont = self.ctx.append_basic_block(function, "drop.opt.cont"); let is_some = self .builder .build_int_compare( @@ -189,7 +202,7 @@ impl<'c, 'a> FnGen<'c, 'a> { .build_load(self.ctx.i8_type(), tag_ptr, "droprestag") .map_err(|error| self.err(error))? .into_int_value(); - let cont = self.ctx.append_basic_block(self.func, "drop.result.cont"); + let cont = self.ctx.append_basic_block(function, "drop.result.cont"); let candidates = [ (0u64, 1u32, scalar_to_ty(ok), "drop.result.ok"), (1u64, 2u32, scalar_to_ty(err), "drop.result.err"), @@ -206,7 +219,7 @@ impl<'c, 'a> FnGen<'c, 'a> { ) }) .map(|(tag, field, ty, name)| { - (tag, field, ty, self.ctx.append_basic_block(self.func, name)) + (tag, field, ty, self.ctx.append_basic_block(function, name)) }) .collect::>(); let cases = branches @@ -287,14 +300,14 @@ impl<'c, 'a> FnGen<'c, 'a> { .build_load(self.ctx.i32_type(), tag_ptr, "droptagv") .map_err(|error| self.err(error))? .into_int_value(); - let cont = self.ctx.append_basic_block(self.func, "drop.enum.cont"); + let cont = self.ctx.append_basic_block(function, "drop.enum.cont"); let branches = owned .into_iter() .map(|(tag, fields)| { ( tag, fields, - self.ctx.append_basic_block(self.func, "drop.enum.v"), + self.ctx.append_basic_block(function, "drop.enum.v"), ) }) .collect::>(); @@ -341,9 +354,9 @@ impl<'c, 'a> FnGen<'c, 'a> { .get(id as usize) .ok_or_else(|| self.err(format!("struct type id {id} is missing")))?; let i64_type = self.ctx.i64_type(); - let head = self.ctx.append_basic_block(self.func, "dropdeep.head"); - let body = self.ctx.append_basic_block(self.func, "dropdeep.body"); - let done = self.ctx.append_basic_block(self.func, "dropdeep.done"); + let head = self.ctx.append_basic_block(function, "dropdeep.head"); + let body = self.ctx.append_basic_block(function, "dropdeep.body"); + let done = self.ctx.append_basic_block(function, "dropdeep.done"); let predecessor = self .builder .get_insert_block() diff --git a/crates/align_codegen_llvm/src/lib.rs b/crates/align_codegen_llvm/src/lib.rs index c797a288..7a79d5e4 100644 --- a/crates/align_codegen_llvm/src/lib.rs +++ b/crates/align_codegen_llvm/src/lib.rs @@ -34,7 +34,7 @@ use align_mir::{ }; use align_sema::{ ArrayBuilderElem, DropPlan, ERROR_VARIANT_CODE, EnumDef, FloatTy, IntTy, Layout, Scalar, StructDef, TupleDef, Ty, - drop_plan, enum_is_move, hir, scalar_to_ty, struct_is_move, ty_to_scalar, + drop_plan, hir, scalar_to_ty, struct_is_move, ty_to_scalar, }; use inkwell::AddressSpace; @@ -2807,7 +2807,10 @@ fn build_module<'c>( } } - // Pass 2: define bodies. + // Pass 2: define bodies. Move-struct destructors are created lazily while these bodies are + // emitted, but the authoritative handles are module-wide: every function that drops the same + // nominal struct must call the one helper rather than cloning its recursive cleanup CFG. + let drop_helpers = std::cell::RefCell::new(HashMap::new()); for f in &program.fns { let builder = ctx.create_builder(); let stack_headers = stack_header_plan(f); @@ -2845,6 +2848,7 @@ fn build_module<'c>( callable_preflight: &callable_preflight, program, runtime_funcs: &runtime_funcs, + drop_helpers: &drop_helpers, fn_sigs: &fn_sigs, extern_abi: &extern_abi, structs: &program.structs, @@ -8512,6 +8516,10 @@ struct FnGen<'c, 'a> { /// Typed handles for every fixed keyed native declaration. Runtime calls never share the /// program-call namespace, even when their logical spellings happen to match. runtime_funcs: &'a HashMap>, + /// One lazily defined private destructor per module-local Move struct. The map handle, rather + /// than a symbol-name lookup, is authoritative so an extern with the same spelling cannot be + /// mistaken for a compiler helper. + drop_helpers: &'a std::cell::RefCell>>, /// Semantic signatures for every direct callable. LLVM's physical integer types do not retain /// signedness, so the range-kernel boundary checks these `Ty`s as well as the generated LLVM /// function type before emitting a direct call. @@ -11526,9 +11534,9 @@ impl<'c, 'a> FnGen<'c, 'a> { self.drop_ty_at(elem_ptr, scalar_to_ty(*s))?; } } else if let Ty::Struct(sid) = ty { - // A Move struct: recursively free each owned field's buffer, in declared order, - // recursing into nested Move-struct fields (null-safe — a moved-out struct was - // zeroed, and Copy fields are skipped). + // A Move struct: call the module-private iterative destructor shared by every + // Drop site for this root type. It frees owned fields in declaration order, + // including nested Move structs (null-safe — moved-out storage was zeroed). self.drop_struct_fields(self.slots[slot], sid)?; } else if let Ty::StructArray(sid, n) = ty { // A fixed array of a Move struct: drop each element's owned fields in turn @@ -16552,170 +16560,89 @@ impl<'c, 'a> FnGen<'c, 'a> { Ok(ptr) } - /// Recursively free the owned fields of a Move struct at `base` (a pointer to the struct value), - /// in declared order. A `string` field's `{ptr,len}` buffer is freed; a nested Move-struct field - /// recurses. Null-safe: an unconstructed / moved-out struct was zeroed (`DropFlagInit`), so each - /// owned leaf reads `{null,0}` and `free(null)` is a no-op. Copy fields (scalars, `str` borrows, - /// plain-data nested structs) are skipped. (Slice 3 of `08-nested-structs.md`.) + /// Call the module's one private iterative destructor for a Move struct at `base`. + /// Null-safe: an unconstructed / moved-out struct was zeroed (`DropFlagInit`), so each owned + /// leaf reads `{null,0}` and `free(null)` is a no-op. Copy structs need no helper or call. fn drop_struct_fields(&self, base: inkwell::values::PointerValue<'c>, struct_id: u32) -> Result<(), CodegenError> { - let st = self.struct_types[struct_id as usize]; - // Snapshot (index, field type) so we don't hold a borrow of `self.structs` across the - // builder/recursion calls (`Ty` is `Copy`). - let fields: Vec<(u32, Ty)> = self.structs[struct_id as usize].fields.iter().enumerate().map(|(i, f)| (i as u32, f.ty)).collect(); - for (i, fty) in fields { - // `i` is the logical field index; the GEP needs its physical (reordered) slot. - let pi = self.pfield(struct_id, i); - match fty { - // An owned `string` field — free its heap buffer (field 0 of the `{ptr,len}`). - Ty::String => { - let fp = self.builder.build_struct_gep(st, base, pi, "dropfld").map_err(|e| self.err(e))?; - let agg = self - .builder - .build_load(slice_struct_type(self.ctx), fp, "dropfldv") - .map_err(|e| self.err(e))? - .into_struct_value(); - let ptr = self.builder.build_extract_value(agg, 0, "dropfldptr").map_err(|e| self.err(e))?; - self.builder.build_call(self.runtime(RuntimeKey::Free), &[ptr.into()], "").map_err(|e| self.err(e))?; - } - // A tagged field owns only its active payload. Reuse the canonical recursive - // destructor so `Option` and later nested owned shapes cannot diverge - // from standalone Option/Result cleanup. - ty @ (Ty::Option(_) | Ty::Result(..) | Ty::Tagged(_)) - if drop_plan( - ty, - self.structs, - self.enums, - self.tagged_defs, - ) - .needs_drop() => - { - let fp = self.builder.build_struct_gep(st, base, pi, "dropopt").map_err(|e| self.err(e))?; - self.drop_ty_at(fp, ty)?; - } - // A nested Move struct — recurse into it (a plain-data nested struct is Copy → skip). - Ty::Struct(nid) - if struct_is_move(nid, self.structs, self.enums, self.tagged_defs) => - { - let fp = self - .builder - .build_struct_gep(st, base, pi, "dropnest") - .map_err(|e| self.err(e))?; - self.drop_ty_at(fp, Ty::Struct(nid))?; - } - // A Move sum-type field (J3) — an owned `array` payload variant makes the enclosing - // struct Move through the recursive DropPlan. Tag-switch and free the live variant's - // owned buffer via `drop_enum` (a non-Move enum owns nothing → not a Move struct field → - // never reaches here). `DropFlagInit` zeroes the aggregate, so a moved-out / unconstructed - // enum field reads tag 0 and frees `null` — null-safe, single-free every path. - Ty::Enum(eid) if enum_is_move(eid, self.structs, self.enums, self.tagged_defs) => { - let fp = self - .builder - .build_struct_gep(st, base, pi, "dropenumfld") - .map_err(|e| self.err(e))?; - self.drop_ty_at(fp, Ty::Enum(eid))?; - } - // An owned `array` field (J3b) — the `Chat { messages: array }` - // shape, where each element owns a buffer (a `string`/owned-array field, or a Move-enum - // field like `Message`'s `content`). Deep-free each element then the AoS (`free(null)` is - // a no-op for an empty array) via the shared helper. - ty @ Ty::DynStructArray(eid, _) - if struct_is_move(eid, self.structs, self.enums, self.tagged_defs) => - { - let fp = self - .builder - .build_struct_gep(st, base, pi, "dropdeeparr") - .map_err(|e| self.err(e))?; - self.drop_ty_at(fp, ty)?; - } - // A direct `array` field owns every element buffer as well as the outer - // array. Route it through the canonical dispatcher so aggregate cleanup is the - // same deep `FreeStringArray` operation as a standalone local or tagged payload. - ty @ Ty::DynArray(Scalar::String) => { - let fp = self - .builder - .build_struct_gep(st, base, pi, "dropstrarr") - .map_err(|e| self.err(e))?; - self.drop_ty_at(fp, ty)?; - } - // An owned `array` field (REST-gateway runway Slice C) with a **non-owned** element — - // free its single heap buffer (field 0 of the `{ptr,len}`; `free(null)` is a no-op for an - // empty array). A scalar / `str`-view / plain-data-struct element owns nothing, so this is - // one flat free — no per-element deep free — and `array`'s `str` fields are - // borrowed views into the input, not freed here. (A Move-struct element is deep-freed by - // the arm above.) - Ty::DynArray(_) - | Ty::DynVecArray(..) - | Ty::DynMaskArray(..) - | Ty::DynFixedArray(..) - | Ty::DynFixedStructArray(..) - | Ty::DynStructArray(..) - | Ty::DynSliceArray(_) => { - let fp = self.builder.build_struct_gep(st, base, pi, "droparr").map_err(|e| self.err(e))?; - let agg = self - .builder - .build_load(slice_struct_type(self.ctx), fp, "droparrv") - .map_err(|e| self.err(e))? - .into_struct_value(); - let ptr = self.builder.build_extract_value(agg, 0, "droparrptr").map_err(|e| self.err(e))?; - self.builder.build_call(self.runtime(RuntimeKey::Free), &[ptr.into()], "").map_err(|e| self.err(e))?; - } - // A nested Move-struct *array* field — drop each element (defensive: struct fields - // reject array types today — `is_field_ok` — so this is unreachable, but keeping the - // owned case here means a future array-valued field can't silently fail-open and leak). - Ty::StructArray(eid, n) - if struct_is_move(eid, self.structs, self.enums, self.tagged_defs) => - { - let fp = self - .builder - .build_struct_gep(st, base, pi, "dropnestarr") - .map_err(|e| self.err(e))?; - let arr_ty = self.struct_types[eid as usize].array_type(n); - let zero = self.ctx.i64_type().const_zero(); - for e in 0..n { - let idx = self.ctx.i64_type().const_int(e as u64, false); - let ep = unsafe { - self.builder.build_in_bounds_gep(arr_ty, fp, &[zero, idx], "dropnestel").map_err(|e| self.err(e))? - }; - self.drop_ty_at(ep, Ty::Struct(eid))?; - } - } - // A Move **handle** field (F1②): a bare pointer handle — `http_request_ctx`, `file`, - // a reader/writer/buffer, a socket, an http request/response/client/server/stream, a - // cli command/parsed. Load the pointer and call its null-safe `*_free`, exactly like a - // standalone handle local's `Stmt::Drop` (shared `handle_free_key` — one source of - // truth). A moved-out / zeroed field reads a null handle, so the free is a no-op — - // the resource is closed at most once. `is_field_ok` admits exactly this handle set, - // so no allowed field type reaches the `_` arm below and silently leaks. - ty if handle_free_key(ty).is_some() => { - let free_key = handle_free_key(ty).expect("guarded by the arm pattern"); - let fp = self.builder.build_struct_gep(st, base, pi, "drophandle").map_err(|e| self.err(e))?; - let p = self - .builder - .build_load(self.ctx.ptr_type(AddressSpace::default()), fp, "drophandlev") - .map_err(|e| self.err(e))?; - self.builder.build_call(self.runtime(free_key), &[p.into()], "").map_err(|e| self.err(e))?; - } - // A package-defined resource field uses its declaration-owned hidden thunk. Keep - // aggregate cleanup on the same pointer-based dispatcher as a standalone resource - // local so adding a nominal wrapper cannot suppress the package destructor. - ty @ Ty::Resource(_) => { - let fp = self - .builder - .build_struct_gep(st, base, pi, "dropresourcefield") - .map_err(|error| self.err(error))?; - self.drop_ty_at(fp, ty)?; - } - _ => {} - } + // Validate every parallel type table before the first indexed lookup. Producer-valid MIR + // always has all three rows; malformed/hand-built MIR must diagnose instead of panicking. + self.structs + .get(struct_id as usize) + .ok_or_else(|| self.err(format!("struct definition id {struct_id} is missing")))?; + self.struct_types + .get(struct_id as usize) + .ok_or_else(|| self.err(format!("struct LLVM type id {struct_id} is missing")))?; + self.field_perm + .get(struct_id as usize) + .ok_or_else(|| self.err(format!("struct layout id {struct_id} is missing")))?; + if !struct_is_move(struct_id, self.structs, self.enums, self.tagged_defs) { + return Ok(()); } + + let existing = self.drop_helpers.borrow().get(&struct_id).copied(); + let helper = if let Some(helper) = existing { + helper + } else { + let helper_ty = self.ctx.void_type().fn_type( + &[self.ctx.ptr_type(AddressSpace::default()).into()], + false, + ); + let helper = self + .module + .add_function(&format!("__align_drop_struct${struct_id}"), helper_ty, None); + mark_nounwind(self.ctx, helper); + mark_private_helper(helper); + // Publish the handle before body construction. The iterative emitter does not call a + // generated Drop helper, but early insertion still makes the one-helper invariant + // explicit and prevents a later refactor from duplicating a self-reachable helper. + self.drop_helpers.borrow_mut().insert(struct_id, helper); + + let saved = self.builder.get_insert_block(); + let saved_debug = self + .dibuilder + .is_some() + .then(|| self.builder.get_current_debug_location()) + .flatten(); + if self.dibuilder.is_some() { + // The private helper has no DISubprogram. Retaining the outer function's location + // would attach wrong-scope metadata and make the module fail verification. + self.builder.unset_current_debug_location(); + } + let emitted = (|| -> Result<(), CodegenError> { + let entry = self.ctx.append_basic_block(helper, "entry"); + self.builder.position_at_end(entry); + let pointer = helper + .get_nth_param(0) + .ok_or_else(|| self.err("struct Drop helper lost its pointer parameter"))? + .into_pointer_value(); + self.emit_drop_at_iterative_in(helper, pointer, Ty::Struct(struct_id))?; + self.builder + .build_return(None) + .map_err(|error| self.err(error))?; + Ok(()) + })(); + match saved { + Some(block) => self.builder.position_at_end(block), + None => self.builder.clear_insertion_position(), + } + if let Some(location) = saved_debug { + self.builder.set_current_debug_location(location); + } else if self.dibuilder.is_some() { + // A debug-enabled outer function always needs a location on later inlinable calls. + // Fall back defensively if helper construction was reached before one was active. + self.set_line(self.fn_line, 0); + } + emitted?; + helper + }; + + self.builder + .build_call(helper, &[base.into()], "") + .map_err(|error| self.err(error))?; Ok(()) } - /// Recursively drop the owned value stored at `base`. - /// - /// This is the codegen counterpart of sema's canonical [`drop_plan`]: tagged containers inspect - /// only their live arm, nominal values recurse, deep arrays run their element destructor, and - /// leaves use the same null-safe runtime free as standalone locals. + /// Recursively drop the owned value stored at `base` using the canonical iterative Drop plan. fn drop_ty_at( &self, base: inkwell::values::PointerValue<'c>, @@ -16727,9 +16654,9 @@ impl<'c, 'a> FnGen<'c, 'a> { /// Deep-free an owned `array` (J3b) whose `{ptr,len}` aggregate lives at `slice_ptr`: /// loop over the `len` elements, recursively `drop_struct_fields` each (freeing its own owned /// fields — a `string`/owned-array/Move-enum field, transitively), then free the AoS buffer itself. - /// A flat free alone would leak every element's owned buffer. `drop_struct_fields` may append basic - /// blocks (a Move-enum element's `drop_enum`), so the loop back-edge branches from the block current - /// *after* the recursive call (`get_insert_block`). An empty array (len 0 / null ptr) skips the loop + /// A flat free alone would leak every element's owned buffer. `drop_struct_fields` emits one helper + /// call in the loop body; helper definition temporarily changes the shared builder's insertion point + /// but restores this outer block before returning. An empty array (len 0 / null ptr) skips the loop /// and frees null. Shared by the struct-field drop (`drop_struct_fields`) and the standalone-local /// drop (`Stmt::Drop`), so a bare `array` local and an `array` field free /// identically. @@ -21908,6 +21835,150 @@ fn main() -> i32 = 0 emit_llvm_ir(&program, &BuildTarget::Baseline, false, &[], None) } + fn test_struct(name: &str, fields: &[Ty]) -> StructDef { + StructDef { + name: name.to_owned(), + source_name: name.to_owned(), + fields: fields + .iter() + .enumerate() + .map(|(index, &ty)| align_sema::FieldDef { + name: format!("field{index}"), + ty, + }) + .collect(), + align: None, + c_repr: false, + } + } + + #[test] + fn move_struct_drop_sites_share_one_private_iterative_helper() { + let i64_ty = Ty::Int(IntTy { bits: 64, signed: true }); + let i32_ty = Ty::Int(IntTy { bits: 32, signed: true }); + let second_function = Function { + name: program_call("drop_in_another_function"), + params: vec![], + param_modes: vec![], + borrow_mut_cleanup_slots: vec![], + return_borrow: hir::ReturnBorrowSummary::None, + return_region: hir::ReturnRegionSummary::None, + return_cleanup: hir::ReturnCleanupAbi::None, + ret: i32_ty, + slots: vec![Ty::Struct(1)], + slot_align: vec![None], + value_tys: vec![], + blocks: vec![Block { + id: 0, + stmts: vec![Stmt::DropFlagInit(0), Stmt::Drop(0)], + stmt_lines: vec![(0, 0), (0, 0)], + term: Term::Return(Some(Operand::Const(Const::Int(0, i32_ty)))), + }], + entry: 0, + exportable: false, + }; + let ir = codegen_program( + vec![ + Stmt::DropFlagInit(0), + Stmt::DropFlagInit(1), + Stmt::DropFlagInit(2), + Stmt::DropFlagInit(3), + // Reach the helper first from a dynamic-array element loop; helper construction + // must restore that loop body's insertion point before its back-edge is emitted. + Stmt::Drop(3), + Stmt::Drop(0), + Stmt::Drop(1), + Stmt::Drop(2), + Stmt::DropElem( + 2, + Operand::Const(Const::Int(0, i64_ty)), + 1, + ), + ], + vec![], + vec![ + Ty::Struct(1), + Ty::Struct(1), + Ty::StructArray(1, 2), + Ty::DynStructArray(1, Layout::Aos), + ], + vec![ + test_struct("Inner", &[Ty::String]), + test_struct("Outer", &[Ty::Struct(0), Ty::String]), + ], + vec![], + vec![second_function], + ) + .expect("shared Drop helper must lower"); + + let helper_name = "__align_drop_struct$1"; + assert_eq!( + ir.lines() + .filter(|line| line.starts_with("define private") && line.contains(helper_name)) + .count(), + 1, + "one helper definition per Drop-site root:\n{ir}" + ); + assert_eq!( + ir.lines() + .filter(|line| line.contains("call void") && line.contains(helper_name)) + .count(), + 7, + "all functions and direct, fixed-array, replacement, and dynamic-array sites must share the helper:\n{ir}" + ); + let helper = function_body(&ir, helper_name); + assert_eq!( + helper.matches("align_rt_free").count(), + 2, + "direct and nested owned string leaves must both be dropped:\n{helper}" + ); + assert!( + !helper.contains("__align_drop_struct$"), + "the iterative helper must not call generated Drop helpers:\n{helper}" + ); + assert!( + !ir.contains("__align_drop_struct$0"), + "a nested child is part of the root helper and must not create a helper on its own" + ); + } + + #[test] + fn copy_struct_drop_emits_no_helper() { + let i32_ty = Ty::Int(IntTy { bits: 32, signed: true }); + let ir = codegen_program( + vec![Stmt::Drop(0)], + vec![], + vec![Ty::Struct(0)], + vec![test_struct("CopyRecord", &[i32_ty])], + vec![], + vec![], + ) + .expect("Copy struct Drop is a no-op"); + assert!(!ir.contains("__align_drop_struct$"), "Copy structs need no Drop helper:\n{ir}"); + } + + #[test] + fn malformed_drop_helper_struct_id_is_diagnosed() { + let i64_ty = Ty::Int(IntTy { bits: 64, signed: true }); + let error = codegen_program( + vec![Stmt::DropElem( + 0, + Operand::Const(Const::Int(0, i64_ty)), + 1, + )], + vec![], + vec![Ty::StructArray(0, 1)], + vec![test_struct("Element", &[Ty::String])], + vec![], + vec![], + ) + .expect_err("an out-of-range Drop helper id must fail closed"); + assert!( + error.to_string().contains("struct definition id 1 is missing"), + "unexpected malformed-id diagnostic: {error}" + ); + } + #[test] fn relocation_bearing_static_data_fails_closed_before_llvm() { let emit = |data: StaticData, result: Ty| { @@ -24733,9 +24804,12 @@ fn main() -> i32 = 0 .into_iter() .flatten() .find_map(|candidate| { - let unquoted = format!(" @{candidate}("); - let quoted = format!(" @\"{candidate}\"("); - ir.find(&unquoted).or_else(|| ir.find("ed)) + let unquoted = format!("@{candidate}("); + let quoted = format!("@\"{candidate}\"("); + ir.match_indices("define ").find_map(|(start, _)| { + let header = &ir[start..ir[start..].find('\n').map_or(ir.len(), |end| start + end)]; + (header.contains(&unquoted) || header.contains("ed)).then_some(start) + }) }) .map(|start| &ir[start..]) .and_then(|tail| tail.split_once("{\n").map(|(_, body)| body)) @@ -25232,10 +25306,20 @@ fn main() -> i32 = 0 vec![], ) .expect("deep recursive Drop emission must use compiler-owned frames"); + let main = function_body(&llvm, "main"); assert!( - function_body(&llvm, "main").contains("@align_rt_free"), + main.contains("__align_drop_struct$0"), + "the deep root Drop site must call its shared helper" + ); + let helper = function_body(&llvm, "__align_drop_struct$0"); + assert!( + helper.contains("@align_rt_free"), "the deepest owned leaf must still be dropped" ); + assert!( + !helper.contains("__align_drop_struct$"), + "deep nominal type depth must not become helper call-stack depth" + ); }) .expect("spawn deep type codegen owner") .join() diff --git a/crates/align_driver/tests/large_drop_codegen.rs b/crates/align_driver/tests/large_drop_codegen.rs new file mode 100644 index 00000000..c8d1848c --- /dev/null +++ b/crates/align_driver/tests/large_drop_codegen.rs @@ -0,0 +1,168 @@ +//! Shared recursive-Drop codegen owner (`docs/impl/21-build-perf-plan.md`, item 3a). +//! +//! A deep finite by-value graph must compile and execute without turning nominal type depth into +//! generated-program call-stack depth. The one root helper expands the existing iterative Drop CFG +//! and therefore never calls another compiler-generated destructor. + +mod common; +use common::*; + +use align_mir::{Block, Const, Function, Operand, Program, ProgramCall, Stmt, Term}; +use align_sema::{FieldDef, IntTy, StructDef, Ty}; +use std::time::{Duration, Instant}; + +struct TempArtifacts([std::path::PathBuf; 2]); + +impl Drop for TempArtifacts { + fn drop(&mut self) { + for path in &self.0 { + let _ = std::fs::remove_file(path); + } + } +} + +fn run_bounded(executable: &std::path::Path) -> std::process::ExitStatus { + let mut child = std::process::Command::new(executable) + .spawn() + .expect("spawn deep Drop executable"); + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if let Some(status) = child.try_wait().expect("poll deep Drop executable") { + return status; + } + if Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + panic!("deep Drop executable exceeded its 10-second deadline"); + } + std::thread::sleep(Duration::from_millis(10)); + } +} + +fn deep_drop_program(depth: usize) -> Program { + assert!(depth > 0); + let mut structs = Vec::with_capacity(depth); + structs.push(StructDef { + name: "Deep0".to_owned(), + source_name: "Deep0".to_owned(), + fields: vec![FieldDef { + name: "text".to_owned(), + ty: Ty::String, + }], + align: None, + c_repr: false, + }); + for index in 1..depth { + structs.push(StructDef { + name: format!("Deep{index}"), + source_name: format!("Deep{index}"), + fields: vec![FieldDef { + name: "next".to_owned(), + ty: Ty::Struct((index - 1) as u32), + }], + align: None, + c_repr: false, + }); + } + + let i32_ty = Ty::Int(IntTy { + bits: 32, + signed: true, + }); + Program { + fns: vec![Function { + name: ProgramCall::try_from_logical("main").expect("valid program call"), + params: vec![], + param_modes: vec![], + borrow_mut_cleanup_slots: vec![], + ret: i32_ty, + return_borrow: align_sema::hir::ReturnBorrowSummary::None, + return_region: align_sema::hir::ReturnRegionSummary::None, + return_cleanup: align_sema::hir::ReturnCleanupAbi::None, + slots: vec![Ty::Struct((depth - 1) as u32)], + slot_align: vec![None], + value_tys: vec![], + blocks: vec![Block { + id: 0, + stmts: vec![Stmt::DropFlagInit(0), Stmt::Drop(0)], + stmt_lines: vec![(0, 0), (0, 0)], + term: Term::Return(Some(Operand::Const(Const::Int(0, i32_ty)))), + }], + entry: 0, + exportable: false, + }], + structs, + ..Program::default() + } +} + +#[test] +fn deep_finite_drop_graph_executes_with_one_helper_frame() { + if !backend_available() { + return; + } + + const DEPTH: usize = 4_096; + let program = deep_drop_program(DEPTH); + let ir = emit_llvm_ir(&program, BuildTarget::Baseline, false, &[], false) + .expect("deep Drop graph must emit raw LLVM"); + let helper_name = format!("__align_drop_struct${}", DEPTH - 1); + assert_eq!( + ir.lines() + .filter(|line| line.starts_with("define private") && line.contains(&helper_name)) + .count(), + 1, + "the root Drop helper must be defined exactly once" + ); + assert_eq!( + ir.lines() + .filter(|line| line.starts_with("define private") && line.contains("__align_drop_struct$")) + .count(), + 1, + "nested records stay in the root helper's iterative CFG" + ); + assert_eq!( + ir.lines() + .filter(|line| line.contains("call void") && line.contains(&helper_name)) + .count(), + 1, + "main must execute the root Drop helper exactly once" + ); + let helper_start = ir + .match_indices("define private") + .find_map(|(start, _)| { + let header_end = ir[start..].find('\n').map_or(ir.len(), |end| start + end); + ir[start..header_end].contains(&helper_name).then_some(start) + }) + .expect("root helper definition"); + let helper_body = ir[helper_start..] + .split_once("{\n") + .and_then(|(_, body)| body.split("\n}").next()) + .expect("root helper body"); + assert!( + !helper_body.contains("__align_drop_struct$"), + "a generated helper must not call another generated helper" + ); + + let stem = std::env::temp_dir().join(format!( + "align-deep-drop-{}-{}", + std::process::id(), + std::thread::current().name().unwrap_or("worker") + )); + let object = stem.with_extension("o"); + let executable = stem.with_extension(std::env::consts::EXE_EXTENSION); + let _artifacts = TempArtifacts([object.clone(), executable.clone()]); + emit_object_file( + &program, + &object, + BuildTarget::Baseline, + Profile::Release, + &[], + false, + ) + .expect("deep Drop object emission"); + link_objects(&[object.as_path()], &executable, &[], Profile::Release) + .expect("deep Drop executable link"); + let status = run_bounded(&executable); + assert_eq!(status.code(), Some(0), "deep Drop executable must finish normally"); +} diff --git a/docs/impl/21-build-perf-plan.md b/docs/impl/21-build-perf-plan.md index f5f8984d..d96c8efd 100644 --- a/docs/impl/21-build-perf-plan.md +++ b/docs/impl/21-build-perf-plan.md @@ -21,7 +21,7 @@ Order is priority. | 2a | Required DB owner build-once/run-many | Shipped in #882 — exact-set concurrent execution across four isolated CI shards; required wall time fell from about 60 minutes to 15:25 while every shard kept the hard 30-minute budget | | 2b | DB CI changed-function scope | Implemented — direct DB/gate and dedicated DB-production paths remain unconditional, while mixed compiler sources provision PostgreSQL only when a changed zero-context hunk or its function header names the database boundary | | 3 | Pipelined compilation | Shipped as #884. A dependent unit's frontend starts as soon as each dependency interface summary exists while already-ready codegen runs within the same `-j` budget; validation, publication, and retry follow the ledger below | -| 3a | Shared recursive-Drop codegen | Implementing for align-llm Request 19 — emit one private pointer-based destructor per reachable Move struct instead of cloning its recursive cleanup CFG at every Drop site | +| 3a | Shared recursive-Drop codegen | Implementing for align-llm Request 19 — emit one private pointer-based destructor per Move struct reached as a Drop-site root instead of cloning its recursive cleanup CFG at every site | | 4 | Prebuilt optimized cache distribution | Design settled below; implementation pending — ship warmed first-party `pkg` entries with each exact native compiler (compiler-provided `core`/`std` imports have no cacheable source unit) | | 5 | Daemon / watch mode | Keep the in-process memo alive across builds; the main lever for AI-agent edit-compile loops. `align-repl` (`docs/impl/22-repl-plan.md`) is the first consumer of this lever: it is already a long-lived process, so it realizes memo residency with no daemon machinery | | 6 | Function-level incremental compilation | Heaviest; requires its own design ledger before any implementation | @@ -51,7 +51,7 @@ source-visible diagnostics and their order, MIR, interfaces, runtime ABI, package ABI, allocation, generated-program effects, cleanup eligibility, field order, active tagged-arm selection, element order, and exactly-once Drop remain unchanged. Codegen emits one private `nounwind void(ptr)` helper for each Move -struct whose destructor is reached in a module. Each ordinary struct Drop, +struct reached as a Drop-site root in a module. Each ordinary struct Drop, replacement Drop, fixed-array element Drop, and dynamic-array element Drop passes the exact existing storage pointer to that helper. The helper contains the existing canonical pointer-based **iterative** Drop plan once and returns @@ -72,9 +72,9 @@ valid because HIR, MIR, interfaces, and their formats do not change. | Axis | Required closure | Owner | | --- | --- | --- | -| Formation and construction | Copy structs emit no helper. Every reachable Move struct gets at most one private, defined `void(ptr)` helper, even when first reached from nested, tagged, fixed-array, or dynamic-array cleanup. A missing/out-of-range type record remains a diagnosed lowering error rather than a panic. | `align_codegen_llvm` helper inventory and malformed-id unit owners | +| Formation and construction | Copy structs emit no helper. Every Move struct reached as a Drop-site root gets at most one private, defined `void(ptr)` helper, including roots reached from fixed-array and dynamic-array cleanup; nested and tagged children stay inside that root helper's iterative CFG. A missing/out-of-range type record remains a diagnosed lowering error rather than a panic. | `align_codegen_llvm` helper inventory and malformed-id unit owners | | Move-in, move-out, and source nulling | Moves keep the existing aggregate transfer and cleanup-bit behavior. The helper receives only the selected live storage pointer; moved or uninitialized storage remains zeroed before a possible call, so null-safe leaves stay null-safe. | existing Move struct transfer/nulling owners plus helper IR assertions | -| Normal Drop and replacement | Standalone struct Drop, reassignment, whole-field replacement, fixed Move-struct array element replacement, and dynamic Move-struct array element cleanup all call the same helper. Field and element order is byte-for-semantic identical to the former inline plan. | focused codegen IR owner; existing nested/owned-array runtime owners | +| Normal Drop and replacement | Standalone struct Drop and reassignment, fixed Move-struct array Drop and element replacement, and dynamic Move-struct array element cleanup all call the same helper. Field and element order is semantically identical to the former inline plan. | focused codegen IR owner; existing nested/owned-array runtime owners | | Control exits | `if`, `match`, `else`, `?`, `map_err`, branch joins, loop back edges and breaks, return, and early error exits retain their existing cleanup guards and call the helper only on the same live paths. A terminating path manufactures no helper call. | existing ownership/control regression targets; Request 19 raw-IR call-count bound | | Nested and tagged graphs | Nested Move structs, `Option`, `Result`, user sums, `array`, `array`, handles, resources, and recursively owned record arrays retain active-arm selection, loop bounds, native thunk choice, and exact child-before-parent restoration order. Helper bodies use the existing compiler-owned iterative worklist/CFG and never call a generated Drop helper, so a 4,096-record valid acyclic graph executes with one helper stack frame rather than a type-depth call chain. | parameterized Drop-plan/codegen owners, an executable deep finite graph stack-bound owner, and runtime exactly-once controls | | Direct, imported, generic, and function-value paths | Whole-program and per-unit compilation emit equivalent private helpers in each owning module. Generic instances follow their concrete module-local struct ids. Calls, returns, imports, and function-value ABI are unchanged. | whole/per-unit IR and executable parity owners | @@ -82,6 +82,22 @@ valid because HIR, MIR, interfaces, and their formats do not change. | Cache and artifact identity | The running compiler-byte hash invalidates every affected object/prelink/backend key; no persisted field changes. Same compiler and inputs remain byte-deterministic, including parallel per-unit builds. | cache edit/revert and deterministic-object owners | | Resource promise | The Request 19 fixture's raw IR no longer scales with cleanup sites times the recursive Drop graph. Its optimized build completes within the consumer's per-target budget with peak memory well below the recorded 1,525,732 KiB, and output remains byte-identical. A representative small one-shot Move-record program is measured before and after for frontend/codegen work counts, wall time, peak memory, object size, and cleanup runtime; the optimization is not accepted if that unaffected path shows a material regression outside run-to-run spread. Counts come from actual compiler/cache outcomes and executed destructor counters, not an expected source/unit count. | local `bench/large_drop_codegen` pathological and unaffected controls plus align-llm `make prompt-verifier-smoke`; final consumer lane/fresh-worker proof belongs to align-llm | +### Candidate evidence + +Measured on 2026-08-26 on Linux x86_64 with release compilers. The Request 19 +raw-IR lens fell from 1,517,324 lines / 113.6 MB to 109,992 lines / 5.96 MB. +Its three-unit default-runtime-LTO cold build fell from 471.074 seconds with an +observed resident set above 832,704 KiB to 13.555 seconds and 266,400 KiB peak +RSS. Fresh cache outcomes reported exactly three frontend misses and three +codegen misses. The resulting executable prints the exact required PASS line. + +The one-shot Move-record control retained one frontend miss, one codegen miss, +a 1,240-byte release object, and an executed allocation/free count of 1 / 1. +Its single-run wall/RSS observation changed from 0.283 seconds / 87,144 KiB to +0.294 seconds / 86,852 KiB, within run-to-run noise. The focused codegen owner +pins one private helper across functions and all struct-root Drop-site shapes; +the 4,096-record executable owner pins stack-bounded generated cleanup. + The implementation boundary is one codegen capability because helper creation and every consuming Drop site must agree in the same module. Splitting a dormant helper producer from call-site conversion would add unreachable code without a From b0147301d830d2b2c71254796541b9b9c1156b0d Mon Sep 17 00:00:00 2001 From: Hironobu Sano Date: Wed, 26 Aug 2026 11:18:06 +0900 Subject: [PATCH 4/4] fix(review): close recursive Drop codegen findings --- HANDOFF.md | 14 ++++-- bench/large_drop_codegen/README.md | 13 ++--- bench/large_drop_codegen/run.sh | 5 +- crates/align_codegen_llvm/src/drop_codegen.rs | 6 ++- crates/align_codegen_llvm/src/lib.rs | 50 ++++++++++++++++--- docs/impl/21-build-perf-plan.md | 13 ++++- scripts/lint-ratchet-baseline.txt | 2 +- 7 files changed, 81 insertions(+), 22 deletions(-) diff --git a/HANDOFF.md b/HANDOFF.md index 0692e32b..099a57d3 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -7,7 +7,12 @@ per-PR journals are preserved in [`docs/archive/HANDOFF-2026-07-25.md`](docs/archive/HANDOFF-2026-07-25.md); neither is a source of current status. -_Last updated: 2026-08-25._ Request 18's retained-root regular-file constructors are implemented +_Last updated: 2026-08-26._ Request 19's shared recursive-Drop codegen is implemented against +`docs/impl/21-build-perf-plan.md` item 3a. On its real-client fixture, raw IR fell from 1,517,324 +lines / 113.6 MB to 109,992 lines / 5.96 MB, while the cold three-unit release build fell from +471.074 seconds and more than 832,704 KiB observed RSS to 13.555 seconds and 266,400 KiB peak RSS; +the executable prints the exact required PASS line. Merge and align-llm's consumer-owned pin/lane +restoration remain. Request 18's retained-root regular-file constructors are implemented against the accepted design in `docs/impl/29-fs-retained-root-plan.md` and real-client verified by align-llm PR #99 at `78eae459fd1f88bad1c3c3ca7b86921a08ecf168`, pinned to Align merge `19c3db144c462bf7d6784f88d64cc124229b7ec2`; C6d is complete. Request 16's sum-payload projection and @@ -359,11 +364,12 @@ facts must live in this repository. surface, ownership model, limit, exact pin, focused adoption owner, and final capable integration evidence is recorded there. The latest closure wave covers Request 14 through align-llm PR #100, Requests 16–17 through #98, and Request 18 through #99. - Request 20 is merged in PR #887: the required `macos-15` PR leg now runs the existing + Request 20 is merged in PR #887 and real-client verified; its align-llm publication record is + merged in align-llm PR #107, so the register can close it. The required `macos-15` PR leg runs the existing `m5_owned_json` owner, and the discovered storage-generation regression no longer falsely retains a `JsonOwnedDecode` input/arena fact. The complete owner passed locally on Apple Silicon and in the - required macOS CI leg. Request 19's large-by-value-struct code-generation cost is now the - highest-priority align-llm request and should start on the other development machine. + required macOS CI leg. Request 19's Align-side implementation is complete; its merge and the + consumer-owned pin, hosted-lane restoration, and fresh-worker proof remain. Consumer-gated deferrals that remain intentional: diff --git a/bench/large_drop_codegen/README.md b/bench/large_drop_codegen/README.md index d43b0f3f..931947ca 100644 --- a/bench/large_drop_codegen/README.md +++ b/bench/large_drop_codegen/README.md @@ -21,9 +21,10 @@ bench/large_drop_codegen/run.sh ``` The harness uses fresh caches to report actual frontend/codegen miss counts, -then reports raw-IR lines, release object bytes, wall time, and peak compiler -RSS. It requires byte-identical program output, the exact `1 / 1` control -allocation/free result, and Request 19's PASS line. Both arms use `--no-rt-lto` -so the allocation counters come from the instrumented archive rather than the -ordinary runtime bitcode. Run the real consumer build separately with its -default runtime-LTO policy before accepting the item. +then reports raw-IR lines, release object bytes, build wall time and peak +compiler RSS, and generated-program cleanup wall time and peak RSS. It requires +byte-identical program output, the exact `1 / 1` control allocation/free result, +and Request 19's PASS line. Both arms use `--no-rt-lto` so the allocation +counters come from the instrumented archive rather than the ordinary runtime +bitcode. Run the real consumer build separately with its default runtime-LTO +policy before accepting the item. diff --git a/bench/large_drop_codegen/run.sh b/bench/large_drop_codegen/run.sh index b8b6efa2..211238d0 100755 --- a/bench/large_drop_codegen/run.sh +++ b/bench/large_drop_codegen/run.sh @@ -64,13 +64,14 @@ measure_case() { "$compiler" emit-llvm main.align --stage raw --no-rt-lto >raw.ll "$compiler" emit-obj main.align main.o --profile release --no-rt-lto ) - timing="$(ALIGNC_CACHE=off python3 "$here/measure.py" "$dir" -- \ + build_timing="$(ALIGNC_CACHE=off python3 "$here/measure.py" "$dir" -- \ "$compiler" build main.align --profile release --no-rt-lto)" "$dir/main" >"$dir/program.stdout" + cleanup_timing="$(python3 "$here/measure.py" "$dir" -- "$dir/main")" raw_lines="$(wc -l <"$dir/raw.ll" | tr -d ' ')" object_bytes="$(wc -c <"$dir/main.o" | tr -d ' ')" - echo "$revision/$case_name: frontend_misses=$frontend_misses codegen_misses=$codegen_misses raw_ir_lines=$raw_lines object_bytes=$object_bytes timing=$timing" + echo "$revision/$case_name: frontend_misses=$frontend_misses codegen_misses=$codegen_misses raw_ir_lines=$raw_lines object_bytes=$object_bytes build_timing=$build_timing cleanup_timing=$cleanup_timing" } measure_case baseline "$baseline" control "$here/control.align" diff --git a/crates/align_codegen_llvm/src/drop_codegen.rs b/crates/align_codegen_llvm/src/drop_codegen.rs index de2f7075..5510895b 100644 --- a/crates/align_codegen_llvm/src/drop_codegen.rs +++ b/crates/align_codegen_llvm/src/drop_codegen.rs @@ -485,7 +485,11 @@ impl<'c, 'a> FnGen<'c, 'a> { Ty::String | Ty::DynArray(_) | Ty::DynStructArray(..) - | Ty::DynSliceArray(_) => { + | Ty::DynSliceArray(_) + | Ty::DynVecArray(..) + | Ty::DynMaskArray(..) + | Ty::DynFixedArray(..) + | Ty::DynFixedStructArray(..) => { let aggregate = self .builder .build_load(slice_struct_type(self.ctx), base, "dropslicev") diff --git a/crates/align_codegen_llvm/src/lib.rs b/crates/align_codegen_llvm/src/lib.rs index 7a79d5e4..8974aad3 100644 --- a/crates/align_codegen_llvm/src/lib.rs +++ b/crates/align_codegen_llvm/src/lib.rs @@ -21877,7 +21877,7 @@ fn main() -> i32 = 0 entry: 0, exportable: false, }; - let ir = codegen_program( + let result = codegen_program( vec![ Stmt::DropFlagInit(0), Stmt::DropFlagInit(1), @@ -21908,8 +21908,9 @@ fn main() -> i32 = 0 ], vec![], vec![second_function], - ) - .expect("shared Drop helper must lower"); + ); + assert!(result.is_ok(), "shared Drop helper must lower: {result:?}"); + let ir = result.unwrap_or_default(); let helper_name = "__align_drop_struct$1"; assert_eq!( @@ -21942,18 +21943,55 @@ fn main() -> i32 = 0 ); } + #[test] + fn shared_struct_drop_frees_every_dynamic_aggregate_array_field() { + let i64_scalar = Scalar::Int(IntTy { bits: 64, signed: true }); + let result = codegen_program( + vec![Stmt::DropFlagInit(0), Stmt::Drop(0)], + vec![], + vec![Ty::Struct(1)], + vec![ + test_struct("Element", &[Ty::Int(IntTy { bits: 64, signed: true })]), + test_struct( + "AggregateArrays", + &[ + Ty::DynVecArray(i64_scalar, 4), + Ty::DynMaskArray(i64_scalar, 4), + Ty::DynFixedArray(i64_scalar, 3), + Ty::DynFixedStructArray(0, 2), + ], + ), + ], + vec![], + vec![], + ); + assert!( + result.is_ok(), + "every admitted dynamic aggregate-array field must lower through shared Drop: {result:?}" + ); + let ir = result.unwrap_or_default(); + + let helper = function_body(&ir, "__align_drop_struct$1"); + assert_eq!( + helper.matches("call void @align_rt_free(").count(), + 4, + "every dynamic aggregate-array field owns one buffer:\n{helper}" + ); + } + #[test] fn copy_struct_drop_emits_no_helper() { let i32_ty = Ty::Int(IntTy { bits: 32, signed: true }); - let ir = codegen_program( + let result = codegen_program( vec![Stmt::Drop(0)], vec![], vec![Ty::Struct(0)], vec![test_struct("CopyRecord", &[i32_ty])], vec![], vec![], - ) - .expect("Copy struct Drop is a no-op"); + ); + assert!(result.is_ok(), "Copy struct Drop is a no-op: {result:?}"); + let ir = result.unwrap_or_default(); assert!(!ir.contains("__align_drop_struct$"), "Copy structs need no Drop helper:\n{ir}"); } diff --git a/docs/impl/21-build-perf-plan.md b/docs/impl/21-build-perf-plan.md index d96c8efd..72962642 100644 --- a/docs/impl/21-build-perf-plan.md +++ b/docs/impl/21-build-perf-plan.md @@ -21,7 +21,7 @@ Order is priority. | 2a | Required DB owner build-once/run-many | Shipped in #882 — exact-set concurrent execution across four isolated CI shards; required wall time fell from about 60 minutes to 15:25 while every shard kept the hard 30-minute budget | | 2b | DB CI changed-function scope | Implemented — direct DB/gate and dedicated DB-production paths remain unconditional, while mixed compiler sources provision PostgreSQL only when a changed zero-context hunk or its function header names the database boundary | | 3 | Pipelined compilation | Shipped as #884. A dependent unit's frontend starts as soon as each dependency interface summary exists while already-ready codegen runs within the same `-j` budget; validation, publication, and retry follow the ledger below | -| 3a | Shared recursive-Drop codegen | Implementing for align-llm Request 19 — emit one private pointer-based destructor per Move struct reached as a Drop-site root instead of cloning its recursive cleanup CFG at every site | +| 3a | Shared recursive-Drop codegen | Implemented for align-llm Request 19 — one private pointer-based destructor per Move struct reached as a Drop-site root replaces cloned recursive cleanup CFGs; merge and consumer lane restoration remain | | 4 | Prebuilt optimized cache distribution | Design settled below; implementation pending — ship warmed first-party `pkg` entries with each exact native compiler (compiler-provided `core`/`std` imports have no cacheable source unit) | | 5 | Daemon / watch mode | Keep the in-process memo alive across builds; the main lever for AI-agent edit-compile loops. `align-repl` (`docs/impl/22-repl-plan.md`) is the first consumer of this lever: it is already a long-lived process, so it realizes memo residency with no daemon machinery | | 6 | Function-level incremental compilation | Heaviest; requires its own design ledger before any implementation | @@ -96,7 +96,16 @@ a 1,240-byte release object, and an executed allocation/free count of 1 / 1. Its single-run wall/RSS observation changed from 0.283 seconds / 87,144 KiB to 0.294 seconds / 86,852 KiB, within run-to-run noise. The focused codegen owner pins one private helper across functions and all struct-root Drop-site shapes; -the 4,096-record executable owner pins stack-bounded generated cleanup. +the 4,096-record executable owner pins stack-bounded generated cleanup. Seven +separate executions of the control reported 0.001 seconds in both revisions; +peak RSS was 12,112–12,380 KiB before and 12,380 KiB after. + +### Implementation review closure + +| Finding | Class-wide closure | +| --- | --- | +| P1: the extracted iterative leaf dispatcher omitted four admitted dynamic aggregate-array field types that the deleted inline struct path freed | Add `DynVecArray`, `DynMaskArray`, `DynFixedArray`, and `DynFixedStructArray` to the canonical flat-buffer free arm. One helper owner places all four in the same Move struct and requires four runtime frees, so another sibling omission fails the owner. | +| P2: the benchmark measured compiler build time but only captured the generated program's output | Measure the executable separately for every baseline/candidate case and report `cleanup_timing` beside `build_timing`; retain the independent output and destructor-count checks. | The implementation boundary is one codegen capability because helper creation and every consuming Drop site must agree in the same module. Splitting a dormant diff --git a/scripts/lint-ratchet-baseline.txt b/scripts/lint-ratchet-baseline.txt index c62bbb18..14d7fff7 100644 --- a/scripts/lint-ratchet-baseline.txt +++ b/scripts/lint-ratchet-baseline.txt @@ -1,5 +1,5 @@ # crate kind count — maintained by scripts/lint-ratchet.sh --update align_mir panics 370 -align_codegen_llvm panics 426 +align_codegen_llvm panics 425 align_runtime casts 289 workspace fixture-bake 51