Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions benchmarks/repsel_census/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,51 @@ removing either mechanism recorder, counting per access site, folding
proven-`this` consumption into the local column, and deleting the consumed
liveness minimum each turn the gate red.

## Knob isolation (#7128)

The census answers "how much does each representation promote". It cannot, on
its own, answer "is knob X evidence about representation X" — and for two of the
five knobs it was not:

- `PERRY_CANONICAL_I32_LOCALS=0` also turned off **every `Ptr<Shape>`
consumption**, because the four ordinary-body `FnCtx` construction sites
computed the `Ptr<Shape>` context flag from the canonical-i32 env read. Census
under that knob read `ptr-shape: 7 selected, 0 consumed`. On this corpus it
moved the object on `batch`, `suite_09_method_calls` and
`fixture_ptr_shape_sites` for `Ptr<Shape>` reasons alone.
- `PERRY_CANONICAL_STR_LOCALS=0` also turned off three lowerings that never
consult a selected `Str` local, so it changed the emitted object on **23 of
the 26** workloads — 20 of which promote no `canonical-str` at all.

```bash
python3 scripts/compiler_output_regression.py census-knob-isolation \
--perry <path/to/perry> --jobs 4
```

Per knob, with that knob at `0` and every other at its default:

1. no census key outside the knob's own may change;
2. a workload whose representation promotes nothing must emit a
**byte-identical** object;
3. the knob must still be live — take a promotion away somewhere, and change
some object.

Rule 1 catches the first defect, rule 2 the second (it leaves every count
untouched). Two controls guard the diff: the compiler must be deterministic
(**it is not on aarch64 Linux** — the LLVM module name embeds pid + nanotime, so
the emission half is skipped there rather than reporting 26 phantoms), and both
`X=1` and an env var the compiler does not read must reproduce the default
object bit-for-bit.

One documented exception, downward only: `PERRY_INT_VALUED_LOCALS=0` lowers
`canonical-i32` on `fixture_int_valued_ta` (3 → 2), because
`int_valued_ta_locals` is merged into `integer_locals`, the candidate set
canonical-i32 draws from. A withdrawn proof cannot be selected. A knob that
*raises* another representation's count is still a leak.

`--jobs` compiles arms in parallel; the whole corpus × 6 knobs × 4 arms is about
20 s on an M1.

## Editing the fixtures

Don't tidy them. Every one is written against a specific collector's rules and
Expand Down
61 changes: 61 additions & 0 deletions changelog.d/7133-repsel-knob-isolation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
### Fixed

- **Representation-selection bisection knobs now move exactly one representation
each (#7128).** Two of them did not, which is worse than an ordinary bug:
every knob-based A/B taken through them measured two things and attributed the
sum to one.

- `PERRY_CANONICAL_I32_LOCALS=0` **also disabled every `Ptr<Shape>`
consumption.** Phase 5a reused `repsel_context_allows_canonical_i32` as its
context gate; #7121 split the `FnCtx` field but left the four ordinary-body
construction sites (`codegen/function.rs`, `method.rs` ×2, `closure.rs`)
initialising both fields from one `repsel_allows` bool whose first conjunct
was the canonical-i32 env read. Census under that knob read `ptr-shape: 7
selected, 0 consumed`, with all six consumption sites printing
`NEVER FIRES`.
- `PERRY_CANONICAL_STR_LOCALS=0` **was not scoped to `Str` locals.** Three
Phase 3a lowerings key on a value's static string type and never on a
selected local — the inline `StringRef` retag
(`native_value/materialize.rs`), the proven-heap operand arm of
`str_operand_handle_tag_dispatched` (`lower_string_method.rs`), and the
tag-dispatched `.length` (`expr/property_get.rs`). They changed the emitted
object on 23 of the 26 census workloads, 20 of which promote no
`canonical-str` at all.

New `expr::repsel_gates` holds the knob table and a pure
`RepselGates -> RepselContextFlags` derivation that all six `FnCtx`
construction sites go through, so "one knob moves one flag" is a unit-testable
property instead of a convention. The three static-string lowerings move to
their own `PERRY_STATIC_STRING_LOWERING` (keyed into the object cache, with a
`gc_repsel_matrix.sh` arm keeping its off-state exercised).

**The default build is byte-identical**: 26/26 census workloads, same census
counts, on both compilers. The `Str` split is an exact partition — base with
`PERRY_CANONICAL_STR_LOCALS=0` and this build with that knob *plus*
`PERRY_STATIC_STRING_LOWERING=0` emit byte-identical objects on 26/26. The
`Ptr<Shape>`, `Ptr<NumArray>` and int-valued-TA knob arms are 26/26 identical
too.

### Added

- **`census-knob-isolation`** (`scripts/compiler_output_regression.py`) — a gate
for the property every knob-based A/B silently assumes. Per knob, with that
knob at `0`: no census key outside its own may change; a workload whose
representation promotes nothing must emit a **byte-identical** object; and the
knob must still be live (take a promotion away somewhere, change some object).
Two controls guard the diff — the compiler must be deterministic, and both
`X=1` and an env var the compiler does not read must reproduce the default
object bit-for-bit.

It fails on `main` (7 count leaks, 20 emission leaks) and passes here. One
documented, downward-only exception: `PERRY_INT_VALUED_LOCALS=0` lowers
`canonical-i32` by one on `fixture_int_valued_ta`, because
`int_valued_ta_locals` is merged into `integer_locals` — a withdrawn proof
cannot be selected. A knob that *raises* another representation's count is
still a leak.

Object emission is **nondeterministic on aarch64 Linux** (the LLVM temp module
name embeds pid + nanotime and lands in the ELF object — filed as #7131), so
the emission half detects the host and skips rather than reporting 26 phantom
diffs. `--require-emission` turns that into a failure where determinism is
expected.
37 changes: 16 additions & 21 deletions crates/perry-codegen/src/codegen/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,28 +758,22 @@ pub(super) fn compile_closure(
&cross_module.module_dispatch,
);

// Representation-selection Phase 1 context gate (see codegen/function.rs).
// Representation-selection context gates (see codegen/function.rs).
// Async-step closures (CPS-rewritten `async` closures — the rewrite clears
// `is_async`) and generator wrapper funcs route body locals through shared
// cells, so canonical-i32 storage is disallowed there.
let repsel_allows = crate::expr::canonical_i32_locals_enabled()
&& !is_async
&& !cross_module.async_step_closures.contains(&func_id)
&& !cross_module.local_generator_funcs.contains(&func_id);
// Phase 3a: same context restrictions, independent env gate.
let repsel_str_allows = crate::expr::canonical_str_locals_enabled()
&& !is_async
&& !cross_module.async_step_closures.contains(&func_id)
&& !cross_module.local_generator_funcs.contains(&func_id);
// #7106: report the structural context exclusion at the `Stmt::Let` site.
// The closure gate spells its generator/async-step reasons differently from
// the body gate, so map them onto the same rule names by hand.
let repsel_context_denial = crate::expr::body_context_denial(
// cells, so canonical storage is disallowed there. The closure gate spells
// its generator/async-step reasons differently from the body gate, so map
// them onto the same rule names here.
let repsel_flags = crate::expr::RepselContextFlags::for_body(
is_async,
cross_module.local_generator_funcs.contains(&func_id),
cross_module.async_step_closures.contains(&func_id),
);
let report_denial = crate::expr::report_context_denial(repsel_context_denial);
let repsel_allows = repsel_flags.allows_canonical_i32;
let repsel_str_allows = repsel_flags.allows_canonical_str;
// #7106: report the structural context exclusion at the `Stmt::Let` site.
let repsel_context_denial = repsel_flags.canonical_denial;
let report_denial = repsel_flags.report_denial();
let repsel_closure_refs = if repsel_allows || repsel_str_allows || report_denial {
crate::expr::collect_closure_referenced_locals(body)
} else {
Expand Down Expand Up @@ -947,11 +941,12 @@ pub(super) fn compile_closure(
i32_counter_slots: HashMap::new(),
local_slot_reps: HashMap::new(),
repsel_context_allows_canonical_i32: repsel_allows,
// #7109: Phase 5a's Ptr<Shape> context gate, split out of
// `repsel_context_allows_canonical_i32`. Ordinary bodies keep the
// exact pre-split value; only entry bodies diverge.
repsel_context_allows_ptr_shape: repsel_allows,
repsel_ptr_shape_context_denial: repsel_context_denial,
// #7109 split the FIELD out of `repsel_context_allows_canonical_i32`;
// #7128 split the VALUE, which is what the knob actually reads. Until
// then this was still `repsel_allows`, so `PERRY_CANONICAL_I32_LOCALS=0`
// disabled every Ptr<Shape> consumption in the program.
repsel_context_allows_ptr_shape: repsel_flags.allows_ptr_shape,
repsel_ptr_shape_context_denial: repsel_flags.ptr_shape_denial,
repsel_context_denial,
repsel_closure_ref_locals: repsel_closure_refs,
repsel_context_allows_canonical_str: repsel_str_allows,
Expand Down
22 changes: 14 additions & 8 deletions crates/perry-codegen/src/codegen/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,11 @@ pub(super) fn compile_module_entry(
// is no structural context reason to deny — see
// `expr::MODULE_INIT_CONTEXT` for the audit — so the only remaining
// gates are the two bisection env knobs.
let repsel_allows = crate::expr::canonical_i32_locals_enabled();
let repsel_str_allows = crate::expr::canonical_str_locals_enabled();
// #7128: one derivation, each flag reading its own knob. `Entry`
// pins `allows_ptr_shape` off structurally (see below).
let repsel_flags = crate::expr::RepselContextFlags::for_entry();
let repsel_allows = repsel_flags.allows_canonical_i32;
let repsel_str_allows = repsel_flags.allows_canonical_str;
// The two value-level screens the `Stmt::Let` site consults (#7106
// collected them for the report only; now they are load-bearing).
let repsel_closure_refs = if repsel_allows || repsel_str_allows {
Expand Down Expand Up @@ -803,8 +806,8 @@ pub(super) fn compile_module_entry(
// the canonical-i32 gate, and #6991 is a live rooting bug for a
// compiled receiver held across the globalThis-population
// collection — which runs around module init.
repsel_context_allows_ptr_shape: false,
repsel_ptr_shape_context_denial: Some(crate::expr::MODULE_INIT_CONTEXT),
repsel_context_allows_ptr_shape: repsel_flags.allows_ptr_shape,
repsel_ptr_shape_context_denial: repsel_flags.ptr_shape_denial,
repsel_closure_ref_locals: repsel_closure_refs,
repsel_context_allows_canonical_str: repsel_str_allows,
repsel_str_ineligible_locals: repsel_str_ineligible,
Expand Down Expand Up @@ -1334,8 +1337,11 @@ pub(super) fn compile_module_entry(
// is no structural context reason to deny — see
// `expr::MODULE_INIT_CONTEXT` for the audit — so the only remaining
// gates are the two bisection env knobs.
let repsel_allows = crate::expr::canonical_i32_locals_enabled();
let repsel_str_allows = crate::expr::canonical_str_locals_enabled();
// #7128: one derivation, each flag reading its own knob. `Entry`
// pins `allows_ptr_shape` off structurally (see below).
let repsel_flags = crate::expr::RepselContextFlags::for_entry();
let repsel_allows = repsel_flags.allows_canonical_i32;
let repsel_str_allows = repsel_flags.allows_canonical_str;
// The two value-level screens the `Stmt::Let` site consults (#7106
// collected them for the report only; now they are load-bearing).
let repsel_closure_refs = if repsel_allows || repsel_str_allows {
Expand Down Expand Up @@ -1461,8 +1467,8 @@ pub(super) fn compile_module_entry(
// the canonical-i32 gate, and #6991 is a live rooting bug for a
// compiled receiver held across the globalThis-population
// collection — which runs around module init.
repsel_context_allows_ptr_shape: false,
repsel_ptr_shape_context_denial: Some(crate::expr::MODULE_INIT_CONTEXT),
repsel_context_allows_ptr_shape: repsel_flags.allows_ptr_shape,
repsel_ptr_shape_context_denial: repsel_flags.ptr_shape_denial,
repsel_closure_ref_locals: repsel_closure_refs,
repsel_context_allows_canonical_str: repsel_str_allows,
repsel_str_ineligible_locals: repsel_str_ineligible,
Expand Down
39 changes: 18 additions & 21 deletions crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,25 +644,21 @@ pub(super) fn compile_function(
);
}
}
// Representation-selection Phase 1: canonical-i32 locals are allowed in
// plain synchronous function bodies only. Async / generator /
// `was_plain_async` bodies route locals through shared cells (the
// async-to-generator transform), which the canonical model must not touch.
let repsel_allows = crate::expr::canonical_i32_locals_enabled()
&& !f.is_async
&& !f.is_generator
&& !f.was_plain_async;
// Phase 3a: same context restrictions, independent env gate.
let repsel_str_allows = crate::expr::canonical_str_locals_enabled()
&& !f.is_async
&& !f.is_generator
&& !f.was_plain_async;
// Representation selection is allowed in plain synchronous function bodies
// only. Async / generator / `was_plain_async` bodies route locals through
// shared cells (the async-to-generator transform), which the canonical
// model must not touch. #7128: one derivation for all three
// representations, each reading its OWN env knob — see
// `expr::repsel_gates`.
let repsel_flags =
crate::expr::RepselContextFlags::for_body(f.is_async, f.is_generator, f.was_plain_async);
let repsel_allows = repsel_flags.allows_canonical_i32;
let repsel_str_allows = repsel_flags.allows_canonical_str;
// #7106: when the context forbids selection for a STRUCTURAL reason, the
// `Stmt::Let` site still reports one denial per would-be-eligible local, so
// "async bodies are excluded" is a counted rule rather than a silent zero.
let repsel_context_denial =
crate::expr::body_context_denial(f.is_async, f.is_generator, f.was_plain_async);
let report_denial = crate::expr::report_context_denial(repsel_context_denial);
let repsel_context_denial = repsel_flags.canonical_denial;
let report_denial = repsel_flags.report_denial();
let repsel_closure_refs = if repsel_allows || repsel_str_allows || report_denial {
crate::expr::collect_closure_referenced_locals(&f.body)
} else {
Expand Down Expand Up @@ -780,11 +776,12 @@ pub(super) fn compile_function(
.collect(),
i32_counter_slots: spec_i32_param_slots,
repsel_context_allows_canonical_i32: repsel_allows,
// #7109: Phase 5a's Ptr<Shape> context gate, split out of
// `repsel_context_allows_canonical_i32`. Ordinary bodies keep the
// exact pre-split value; only entry bodies diverge.
repsel_context_allows_ptr_shape: repsel_allows,
repsel_ptr_shape_context_denial: repsel_context_denial,
// #7109 split the FIELD out of `repsel_context_allows_canonical_i32`;
// #7128 split the VALUE, which is what the knob actually reads. Until
// then this was still `repsel_allows`, so `PERRY_CANONICAL_I32_LOCALS=0`
// disabled every Ptr<Shape> consumption in the program.
repsel_context_allows_ptr_shape: repsel_flags.allows_ptr_shape,
repsel_ptr_shape_context_denial: repsel_flags.ptr_shape_denial,
repsel_context_denial,
repsel_closure_ref_locals: repsel_closure_refs,
repsel_context_allows_canonical_str: repsel_str_allows,
Expand Down
Loading
Loading