perf(repsel): refuse canonical i32 when every hot consumer wants a double (#7128)

[proggeramlug]

Fixes the 15_mandelbrot finding in #7128.

benchmarks/suite/15_mandelbrot.ts regressed +14.87% instructions retired
at #7121 — measured on a quiet Raspberry Pi 5 with perf stat, bisected to
#7121 by binary hash, and invisible in wall time because the workload is
FP-latency-bound.

The root cause, read out of the emitted AArch64

The brief's working hypothesis (sitofp/fptosi on a hot path) is part of
the answer, and the smaller part. The innermost loop is

while (x * x + y * y <= 4.0 && iter < MAX_ITER) { …; iter = iter + 1; }
…
totalIter = totalIter + iter;

iter as a boxed double — 12 instructions, one basic block, one branch:

e4: fsub  d16, d17, d16     f0: fmul  d6, d17, d6       fc: fmul  d17, d18, d18
e8: fadd  d17, d18, d18     f4: fadd  d6, d4, d6       100: fmul  d16, d6, d6
ec: fadd  d18, d7, d16      f8: fadd  d5, d5, d2       104: fadd  d19, d17, d16
                                                       108: fcmp  d19, d0
                                                       10c: fccmp d5, d3, #0, ls
                                                       110: b.mi  e4

Both exit tests are FP, so LLVM fuses them with fccmp into a single block.

iter in a canonical i32 slot — 14 instructions, two blocks, two branches:

100: fsub d5, d6, d5       10c: fmul d3, d6, d3       118: fmul d6, d7, d7
104: fadd d6, d7, d7       110: fadd d3, d2, d3       11c: fmul d5, d3, d3
108: fadd d7, d4, d5       114: add  w11, w10, #1     120: fadd d16, d6, d5
                                                      124: fcmp d16, d0
                                                      128: b.hi c4      ← exit
                           12c: cmp  w10, #0x63       ← second block
                           130: mov  w10, w11
                           134: b.cc 100

An integer compare cannot fuse with an FP compare. The loop splits, gaining a
compare, a branch and a register copy for the induction phi — and separately
totalIter = totalIter + iter now needs a ucvtf (c4: ucvtf d3, w11) it did
not need before.

2 instructions × 8,011,148 innermost iterations ≈ 16.0M, against a measured
+15,628,722. That is the whole regression, arithmetically.

px and py are the same shape one level out: const cx = (px - WIDTH / 2.0) * 4.0 / WIDTH reads an i32 px into an f64 chain twice per inner iteration.
(LLVM's IndVarSimplify already narrows both counters to w-registers in both
arms, so promoting them explicitly bought nothing even before the fusion loss.)

Where the fix belongs, and why

Not in the proof. #7122's monotone loop-induction interval proves
iter ∈ [0, 100], and that is true. The Let-site gate in stmt/let_stmt.rs is a
conjunction of "may we?" terms — integer_locals, index_used_locals,
strictly_i32_bounded_locals, loop_bounded_i32_locals, int_valued_ta_locals
— with no "should we?" term anywhere in it. So widening a proof
automatically widens the emission, which is exactly what #7110 → #7121 did.

A cost model consulted by the collector, not a rule inside one. The new
collectors/repsel_benefit.rs computes a per-local profitability verdict from
the same HIR the other collectors walk, and the Let-site gate consults it as one
more conjunct. Three reasons for that shape rather than folding the condition
into loop_bounded_i32.rs:

1. They are different questions with different failure modes. An unsound
   selection is a wrong answer; an unprofitable one is a slower answer that no
   correctness test can see. Folding them means the next widening of a range
   proof silently changes the benefit verdict, and vice versa.
2. The denial has to be nameable. --opt-report now says
   no_i32_consuming_use where it used to say nothing at all — or worse, would
   have said module_init_context, the rule perf(codegen): module-init / program-entry bodies select canonical i32/u32/Str (#7109) #7121 removed, sending the next
   reader back to a bug that is already fixed.
3. The other representations have the same disease. repsel: the coverage work converted exactly as predicted, and almost none of it is faster (one −4.1% win, one +14.9% regression) #7128's findings C and
   D: every __pshape clone is dead-stripped before the object, and
   Ptr<NumArray> emits nothing outside its own fixture. Both are "proven,
   selected, no byte changed". This is the module they plug into. I did not
   widen it to them here — that is a separate measurement.

The rule, and whether it generalises

For an i32-range value, double is a lossless and equal-cost representation
of +, - and comparison — one instruction either way on every target Perry
ships. I32 only buys something where the consumer cannot take a double
without a conversion: array/typed-array indexing, bitwise operands,
Math.imul. That is the same list docs/representation-selection-rfc.md §5.3
already gives for where I32 semantics are exact — the benefit set and the
exactness set coincide, which is why this is a rule and not a heuristic. It
becomes a cost the moment a hot consumer needs the double back.

A local that is written after its declaration, has no i32-consuming read
anywhere, and has at least one double-consuming read inside a loop does
not select canonical i32.

Each conjunct earns its place, and each has a test that turns red without it:

• written after declaration — a single-assignment local is loop-invariant at
  every read, so LICM hoists any conversion out of the loop. const WIDTH = 800
  is read as a double twice per inner iteration of 15_mandelbrot and still
  costs nothing; judging it would be pure census churn.
• no i32-consuming read — one array index pays for the representation.
  11_prime_sieve's counters are all index-used, which is why its win is
  untouched.
• a double-consuming read inside a loop — a conversion outside every loop runs
  once. return iter after the loop is not a reason to refuse.

It is not a special case for 15_mandelbrot. The refusal fires on 8 locals
across 6 benchmark programs (plus hit in the fixture), and the ones outside
15_mandelbrot are the same shape
reached by different syntax: result = result + (1.0 / i) (06_math_intensive),
new Point(i, i + 1) (07_object_create, 12_binary_trees), compute(i)
(14_closure), sum + (i % 1000) (13_factorial). REFUSAL_FLOORS pins two
different programs plus a fixture, so a later narrowing to "mandelbrot only"
goes red.

Deliberate under-approximation, since the model can only ever refuse: an
expression form it does not model contributes neither side; comparison is
neutral on both sides (otherwise for (let i = 0; i < n; i++) with a number
parameter — the most common loop in JavaScript — would stop promoting, to buy
nothing); and a write of a local into its own slot never costs, though it still
counts as a benefit in an i32 position, because
seed = (Math.imul(seed, K) + C) & 0x7fffffff is a local whose only read is
inside its own assignment.

Review follow-up: the self_target exemption (CodeRabbit, Major)

CodeRabbit asked for a regression test on a self-referencing forced conversion
(x = x / 2, x = f(x)) and predicted it would fail. It does, and it was a
live bug in the first version of this PR. I ran its test before deciding
anything:

=== new tests against the OLD rule ===
    collectors::repsel_benefit::tests::self_divide_is_still_a_cost
    collectors::repsel_benefit::tests::self_referencing_call_argument_is_still_a_cost
    collectors::repsel_benefit::tests::self_referencing_new_argument_is_still_a_cost
test result: FAILED. 398 passed; 3 failed
=== at HEAD ===
test result: ok. 401 passed; 0 failed

The first version keyed the exemption on the syntactic shape t = … t ….
That is wrong for the reason CodeRabbit gives: x = x / 2 and x = f(x) are
self-writes whose value is a double, so scoring them free would promote a
local whose hot consumer wants a double — the exact failure this PR exists to
refuse, re-entering through the fix rather than the original path.

The rule now follows the representation flow, not the shape:
self_target is cleared on the way through any operation that forces a
materialization (Model::forced_double — a / or ** operand, a call or
new argument) and kept only where the Double context is inherited from
the target slot (iter = iter + 1, t = -(t + k)). Inherited-Double is the
slot's own representation; forced-Double is a genuine convert-out-and-back.

The gap is closed, not accepted. All three cases are covered
(/, call argument, new argument — they do not share a code path), plus a
positive control that a representation-preserving chain is still exempt, with
a second local in the same expression asserted to be refused so the green
verdict cannot come from the walk stopping early.

Note for the record: this could not have reached the shipped compiler through
the loop_bounded_i32 path, because that proof requires every write to be a
step (v++, v = v ± k), so x = x / 2 is never a candidate. But
strictly_i32_bounded_locals can admit h = clamp(h / 2)-shaped locals, and
#7123 proposes to widen the proof further. "Unreachable today" is exactly the
reasoning that produced #7128 in the first place — a conjunction whose terms
were widened one at a time — so the rule is fixed rather than argued away.

Measurements

Raspberry Pi 5, aarch64, load 0.23 at start, perf stat -e instructions:u,
11 repeats, warmed. Compiler arms built from one target dir in one session,
cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static;
perry md5s verified distinct, libperry_runtime.a verified identical across
arms (this PR touches only perry-codegen). Every row carries the linked
binary hash, so "the two arms were actually different binaries" — or
deliberately the same one — is on the record next to the number.

workload	arm	binary	instructions	Δ
15_mandelbrot	main	20348084	120,738,701	—
	this PR	9e622913	105,110,087	−12.94%
	at7122 (pre-regression)	9e622913	105,110,029	−12.94%
11_prime_sieve	main	0e9e58ca	2,597,182,143	—
	this PR	0e9e58ca	2,597,177,676	−0.00%
	at7122	bac60649	2,624,762,652	+1.06%
08_string_concat	main	a2e03bb6	30,281,798	—
	this PR	a2e03bb6	30,281,711	−0.00%
	at7122	b9a40645	31,582,031	+4.29%

The two wins the brief said must not regress, re-measured against at7122
with this PR's compiler rather than assumed: canonical Str on
08_string_concat −4.12%, canonical i32 on 11_prime_sieve −1.05%.
Both hold, and hold by construction: this PR's linked binary for each is
byte-identical to main's.

These are the numbers from the FINAL compiler (after the CodeRabbit fix below).
The earlier arm measured identically — the review fix moved 0 of 26 objects
across the corpus, verified rather than assumed.

Controls (same binary in both arms — any reading other than ~0 would mean
the rig is contaminated): 01_startup +0.02%, 02_loop_overhead −0.00%,
13_factorial +0.00%, 06_math_intensive +0.00%; and from the full sweep,
07_object_create −0.00%, 12_binary_trees +0.00%, 14_closure +0.00%,
17_loop_data_dependent −0.00%, 16_matrix_multiply +0.00%, batch +0.05%.
batch is the loosest same-binary row at 1.6M instructions out of 3.37e9; I
quote it as the observed noise ceiling rather than the 0.02% floor #7128
measured on smaller programs.

Emission, corpus-wide. All 26 census workloads compiled with both arms and
compared as objdump -d text. Linux objects are nondeterministic (#7128 finding
E — the LLVM module name embeds pid+nanotime), so the rig proves its own
determinism first by compiling every workload twice in the main arm: 26/26
identical under this comparison, which is what makes the A/B column meaningful.
Exactly two objects change: 15_mandelbrot, and fixture_loop_bounded_i32
(which this PR extends on purpose). For 15_mandelbrot the disassembly is
byte-identical to at7122's, and so is the linked binary. The other refused
promotions were already emitting byte-identical code, which is why the census
counts fall without a single emitted byte moving.

Wall time. 15_mandelbrot reads 48 ms before and 49 ms after (min 48 in both
arms) — 1 ms of timer quantisation on an FP-latency-bound workload, the same
non-signal #7128 recorded in the other direction. I am not claiming a wall win;
the claim is 15.6M fewer instructions.

Census

canonical-i32 falls 64 → 55. Every moved floor, deliberately:

workload	before	after	refused local
suite_15_mandelbrot	6	3	py, px, iter
suite_06_math_intensive	2	1	i in 1.0 / i
suite_07_object_create	2	1	i in new Point(i, i+1)
suite_12_binary_trees	2	1	i in new Point3D(…)
suite_13_factorial	2	1	i in sum + (i % 1000)
suite_14_closure	2	1	i in compute(i)

No other floor moved and no LIVENESS_FLOORS minimum moved: every fixture still
promotes exactly what it was written to promote. The five workloads other than
15_mandelbrot emit identical objects either way, so this is the census
learning to report what codegen was already doing.

Every lowered floor is paired with a refusal minimum (CodeRabbit): all six
workloads above, plus the fixture, are in REFUSAL_FLOORS, with counts read out
of --opt-report per workload rather than inferred from the drop. A floor that
fell because a promotion was deliberately refused must carry the assertion that
it is still being refused — otherwise the lower floor silently accommodates a
different promotion going missing.

The gate — because no floor in this census can catch an EXTRA promotion

Every number in benchmarks/repsel_census is a promotion count and every gate on
it is a floor. A floor cannot go red when a compiler promotes more, which
is precisely the failure this PR fixes. So the refusal gets a minimum of its own:
REFUSAL_FLOORS in scripts/compiler_output_harness/repsel_census.py (in code,
not in the regenerable baseline, for the same reason as LIVENESS_FLOORS),
checked by check_refusal_floors and wired into census --gate.

fixture_loop_bounded_i32.ts now carries the paired case: iterate()'s counter
and mixedWithFloat()'s counter are admitted by the identical #7110 interval
proof and differ only in what consumes them. One must promote (its
canonical-i32: 3 liveness floor) and one must be refused (its refusal floor),
so neither an always-yes nor an always-no rule can satisfy the file.

Sabotage evidence

Direction 1 — the rule stops firing. Not simulated: census --gate run with
the unfixed main compiler against this PR's shipped baseline exits 1, and
the only failure is the new check (REGRESSION count: 0):

REFUSAL NO LONGER FIRING:
  suite_15_mandelbrot:       refused 0, must refuse at least 3.
  suite_06_math_intensive:   refused 0, must refuse at least 1.
  suite_07_object_create:    refused 0, must refuse at least 1.
  suite_12_binary_trees:     refused 0, must refuse at least 1.
  suite_13_factorial:        refused 0, must refuse at least 1.
  suite_14_closure:          refused 0, must refuse at least 1.
  fixture_loop_bounded_i32:  refused 0, must refuse at least 1.

The same command with this PR's compiler prints Census OK. and exits 0.

Direction 2 — the rule over-fires. Each of the three conjuncts was
individually deleted and the tests re-run; each has its own disjoint set of
red tests, so no conjunct is decorative:

conjunct deleted	tests that turn red
int_reads == 0	4 — ..._is_a_benefit ×3, write_into_an_i32_storage_local_is_a_benefit
hot_double_reads >= 1	6 — bare_guard_only_counter_survives, guard_against_a_non_integer_bound_is_not_a_cost, self_step_is_not_a_cost, double_use_outside_a_loop_is_not_a_cost, unmodelled_forms_are_neutral, a_self_write_through_a_preserving_chain_is_still_not_a_cost
writes_after_decl >= 1	1 — write_once_local_is_out_of_scope

Direction 3 — the exemption over-reaches (the CodeRabbit case above): the
three self-referencing forced-conversion tests fail against the previous
version of the rule and pass at HEAD, with no collateral (398 passed / 3 failed
→ 401 passed / 0 failed).

Anti-vacuity: the boundary tests that assert a local is not refused carry a
second local in the same expression that is refused, so a green verdict
cannot come from the walk stopping early.

GC x representation-selection matrix

scripts/gc_repsel_matrix.sh --arms all --pressure 8, run locally on the Pi
against this PR's compiler (CI is hours deep in the runner queue):

summary: PASS=426 UNVER=119 XFAIL=1 FAIL=0
byte-exact vs node 26.5.1: 545/546 cells

FAIL=0, the state the brief records for main. The single XFAIL is the
pre-existing triaged entry (repsel_ptr_shape_locals x rep_ptr_shape_off).
UNVER is the arm-was-inert verdict, unchanged in shape from main: the
non-evacuating arms are inert on the workloads that allocate too little to
trigger a collection, which is what that column exists to say out loud.

This is expected to be a null for this change and is reported as due
diligence rather than as evidence: the refusal moves values from an
unscanned i32 slot back to a GC-scanned double slot, which is the conservative
direction, and 24 of the 26 census objects are byte-identical anyway.

gc-ratchet

Run locally on the Pi, both profiles. The gated (shared_ci) run reports:

gc-ratchet: FAILED
  - platform mismatch: baseline 'darwin-arm64' vs current 'linux-aarch64'
| 05_closure_capture | heap_used_bytes | 1,040,208 | 1,107,552 | +6.47% | ... | REGRESSION |

That is not this PR. The harness itself rejects the comparison (the pinned
baseline is macOS; the only host with perf is Linux), and the control settles
it: the same ratchet, same host, same session, with the unfixed main
compiler produces the byte-identical row — 1,107,552, +6.47%. So the delta
is the platform, and this change contributes zero to it.

Every other gated metric — heap_total_bytes, minor_cycles, step_cycles,
copied_objects, promoted_objects, freed_bytes — reads +0.00% on all
eight probes. The ungated column (rss_bytes, peak_rss_bytes, wall_ms)
moves by 7–150% in both arms alike, which is exactly the cross-platform drift
tolerances.json declines to gate.

The authoritative run is the gc-ratchet CI job on this PR, which executes on
the baseline's own platform.

What I could not measure

• Instructions retired on Apple silicon — no unprivileged counter. Nothing
  here is measured on the Mac mini.
• Whether the fccmp half of the mechanism is AArch64-specific. Both
  available hosts are ARM. The sitofp-at-the-accumulator half is
  target-independent; the fused-exit-test half may be smaller on x86-64. The rule
  does not depend on which half dominates — a representation that only ever
  converts back cannot be cheaper on any target — but the size of the win on
  x86-64 is unmeasured.
• Linux binary sizes — ELF .text quantisation plus nondeterministic
  objects (repsel: the coverage work converted exactly as predicted, and almost none of it is faster (one −4.1% win, one +14.9% regression) #7128 finding E).

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The change adds canonical-i32 profitability analysis, excludes unprofitable locals, reports no_i32_consuming_use denials, and enforces refusal floors in census validation. Fixtures, compiler tests, census tests, baseline data, and documentation cover the new behavior.

Changes

Canonical i32 profitability

Layer / File(s)	Summary
Profitability analysis crates/perry-codegen/src/collectors/repsel_benefit.rs, crates/perry-codegen/src/collectors/repsel_benefit/tests.rs, crates/perry-codegen/src/collectors/mod.rs	The new collector classifies integer and double consumers across HIR expressions and statements. Tests cover loop counters, storage facts, self-updates, calls, arithmetic, and unsupported expressions.
Fact graph and selection integration crates/perry-codegen/src/collectors/hir_facts.rs, crates/perry-codegen/src/stmt/let_stmt.rs	Representation facts expose unprofitable locals. Canonical-i32 selection excludes those locals and records the refusal reason.
Denial reporting and precedence crates/perry-codegen/src/expr/slot_rep.rs	Denial reporting adds no_i32_consuming_use with issue #7128. Precedence tests cover range, profitability, and context denials.
Fixture and census enforcement benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts, benchmarks/repsel_census/README.md, scripts/compiler_output_harness/repsel_census.py, tests/test_repsel_census.py, benchmarks/repsel_census/baseline.json, docs/representation-selection-rfc.md, changelog.d/7132-repsel-profitability.md	The fixture adds a bounded counter consumed by a floating-point accumulator. Census reports count denial rules and enforce configured refusal floors. Baseline data, RFC text, and the changelog describe the updated behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HIRFacts
  participant RepselBenefit
  participant LetStmt
  participant SlotRep
  participant Census
  HIRFacts->>RepselBenefit: collect unprofitable canonical-i32 locals
  RepselBenefit-->>HIRFacts: return refusal local IDs
  HIRFacts->>LetStmt: expose profitability facts
  LetStmt->>SlotRep: evaluate canonical-i32 eligibility
  SlotRep-->>LetStmt: report no_i32_consuming_use when applicable
  Census->>SlotRep: collect denial-rule reports
  Census-->>Census: enforce configured refusal floors

Loading

Possibly related PRs

• PerryTS/perry#6903: Adds the canonical-i32 representation-selection behavior extended by this change.
• PerryTS/perry#7037: Adds related representation-selection diagnostics and opt-report instrumentation.
• PerryTS/perry#7122: Adds the bounded-i32 eligibility and fixture used by the profitability and refusal handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the main representation-selection change and its performance purpose.
Description check	✅ Passed	The description thoroughly explains the fix, motivation, implementation, issue reference, tests, measurements, and limitations, despite not following the template headings.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/7128-repsel-benefit

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/perry-codegen/src/collectors/repsel_benefit/tests.rs (1)

1-467: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add a regression test for a self-referencing forced-conversion (x = x / 2 or x = f(x)).

Every other conjunct of the rule has a boundary test (see the module doc: "a refusal rule that over-fires is a silent coverage loss with no symptom"), but no test covers a self-referencing divide or self-referencing call argument inside a loop. Given the self_target gap flagged in repsel_benefit.rs (Lines 235-311), add a case like:

#[test]
fn self_divide_is_still_a_cost() {
    let stmts = vec![
        let_mut(1, Some(Expr::Integer(64))),
        Stmt::While {
            condition: cmp(get(1), Expr::Integer(1)),
            body: vec![set(1, bin(BinaryOp::Div, get(1), Expr::Number(2.0)))],
        },
    ];
    let out = run(&stmts, &HashSet::new());
    assert!(out.contains(&1), "self-divided counter must still be refused: {out:?}");
}

This test would currently fail against the code as written, which is exactly the signal this file's other tests are designed to give.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/collectors/repsel_benefit/tests.rs` around lines 1 -
467, Add a regression test alongside the existing self-target coverage, using
the `run` helper and a loop whose body assigns a local to itself through
floating-point division (for example, `x = x / 2.0`). Assert that the local
appears in the refused set, ensuring self-target suppression does not hide a
forced-conversion cost; preserve the existing `self_step_is_not_a_cost` behavior
for integer-preserving updates.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/src/collectors/hir_facts.rs`:
- Around line 472-491: Extend I32StorageFacts and its holds_i32() method in
crates/perry-codegen/src/collectors/repsel_benefit.rs:104-139 to include
loop_bounded_i32 as an eligible source, then pass &loop_bounded_i32_locals when
constructing I32StorageFacts in
crates/perry-codegen/src/collectors/hir_facts.rs:472-491.

In `@crates/perry-codegen/src/collectors/repsel_benefit.rs`:
- Around line 200-311: Clear self_target while evaluating expressions at
forced-conversion boundaries so genuine double-conversion costs are counted. In
expr, update BinaryOp::Div and BinaryOp::Pow handling and each argument
evaluation in Expr::Call and Expr::New to suppress the self-write exemption;
preserve self_target for inherited contexts such as Add, Sub, Mul, and Mod.

---

Outside diff comments:
In `@crates/perry-codegen/src/collectors/repsel_benefit/tests.rs`:
- Around line 1-467: Add a regression test alongside the existing self-target
coverage, using the `run` helper and a loop whose body assigns a local to itself
through floating-point division (for example, `x = x / 2.0`). Assert that the
local appears in the refused set, ensuring self-target suppression does not hide
a forced-conversion cost; preserve the existing `self_step_is_not_a_cost`
behavior for integer-preserving updates.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: acaba268-0e18-4cad-8cf5-4e87f1c44121

📥 Commits

Reviewing files that changed from the base of the PR and between 0044b1c and 6f2aada.

📒 Files selected for processing (11)

• benchmarks/repsel_census/README.md
• benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts
• crates/perry-codegen/src/collectors/hir_facts.rs
• crates/perry-codegen/src/collectors/mod.rs
• crates/perry-codegen/src/collectors/repsel_benefit.rs
• crates/perry-codegen/src/collectors/repsel_benefit/tests.rs
• crates/perry-codegen/src/expr/slot_rep.rs
• crates/perry-codegen/src/stmt/let_stmt.rs
• docs/representation-selection-rfc.md
• scripts/compiler_output_harness/repsel_census.py
• tests/test_repsel_census.py

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@benchmarks/repsel_census/baseline.json`:
- Line 378: Remove the baseline canonical-i32 floors for suite_07_object_create,
suite_12_binary_trees, suite_13_factorial, and suite_14_closure, unless each
workload has an explicit no_i32_consuming_use refusal minimum in REFUSAL_FLOORS
within repsel_census.py; retain only floors justified by that guard.

In `@changelog.d/7132-repsel-profitability.md`:
- Around line 18-19: Keep the inline code span containing totalIter entirely on
one physical Markdown line, including its opening and closing backticks, so no
line-break or indentation whitespace appears inside the span and MD038 passes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b846d572-06ab-4a0e-a794-b02aafbf8e1f

📥 Commits

Reviewing files that changed from the base of the PR and between 16f03f4 and 3c71f0e.

📒 Files selected for processing (2)

• benchmarks/repsel_census/baseline.json
• changelog.d/7132-repsel-profitability.md