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
20 changes: 18 additions & 2 deletions .github/workflows/gc-root-dominance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -402,25 +402,41 @@ jobs:
# This step is minutes, not seconds -- the scan is superlinear in
# instruction count and the dependency corpus is 62 MB. That is the price
# of checking the population that actually breaks.
#
# 39 -> 2 at #11604. The checker stopped counting a use reached only
# through a phi edge that REPLACES the source (the `&&`/`??` short-circuit
# join, and S2's template-coercion join from #11554). Those uses were
# never below the replacing arm's collector. Measured on the corpus
# before the change: 8 on 62ab59233, and 32 on 7401e6c0d, because #11554
# added 24 template-join uses. After the change: 2, the two
# `InOrder.run` uses in test_gap_class_field_named_stack_index. Pinned
# at the measured value, because a ratchet is only a ratchet if it is
# pinned where it stands.
- name: Stale-register budget (curated)
run: |
set -euo pipefail
python3 scripts/gc_root_dominance_check.py ir-corpus \
--stale-registers --moving-only \
--min-files 90 --min-binds 1500 --min-funcs 1200 \
--max-stale 39
--max-stale 2

# 86 -> 104 at #7616: widening POLL_CAPABLE_RUNTIME by 77 symbols makes
# windows MOVING that `--moving-only` previously dropped. Still inside
# the pinned 118, so the budget is deliberately NOT raised — a ratchet
# you loosen every time it gets closer is not a ratchet.
#
# 118 -> 10 at #11604, for the same phi-edge reason as the curated step.
# Main was at 457, over this budget, and nothing reported it: the
# dominance step above was red on the same #11554 join, so this step
# never ran. 436 of those uses were S2 template joins and 11 were
# `??`/`&&` joins. After the change: 10.
- name: Stale-register budget (dependency-scale)
run: |
set -euo pipefail
python3 scripts/gc_root_dominance_check.py ir-corpus-dep \
--stale-registers --moving-only \
--min-files 60 --min-binds 4000 --min-funcs 5000 \
--max-stale 118
--max-stale 10

- name: Upload the IR corpus on failure
if: failure()
Expand Down
3 changes: 3 additions & 0 deletions changelog.d/11606-gc-root-dominance-phi-replacing-edge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- **gc-root-dominance: a phi edge that replaces the value is no longer counted in its window (#11604).** The 96 `js_jsvalue_to_string_method_box` violations on the dependency-scale corpus were a checker false positive. S2 (#11554) joins `${v}`'s inline string test and its collecting cold arm as `phi [ %v, %entry ], [ %coerced, %tmpl_coerce.slow ]`. On the only path through the collecting call, the slot receives `%coerced`, not `%v`. The bind-anchored check (`check_func`) followed the phi back to `%v` and then counted that path. It now drops, per origin, the phi edges whose operand is not derived from the origin (`phi_replacing_edges`), which is the shadow-mode twin of #7664's `--statepoints` refinement. A collector on an edge that carries the value, or between the join and the bind, is still reported, and `--self-test` asserts both plus the safe join. The new `test_gap_gc_template_coerce_join` puts the shape in the curated corpus: 4 violations under the old checker, 0 now. On the dependency corpus the unfiltered count goes from 146 to 50: exactly the 96 template-coercion fingerprints are removed, and no new ones appear.

`--stale-registers` had the same over-approximation, so it now applies the same phi-edge rule (`_stale_use_replacing_edges`), and the self-test asserts it in both directions. #11554's template join had pushed the stale counts past budget: curated 8 → 32, and dependency-scale 21 → 457 against a budget of 118. Nothing reported it, because the dominance step failed before the stale step could run. The new gap test added 17 more curated uses, which is what made this PR red. After the change the counts are 2 (curated) and 10 (dependency-scale), a subset of the old reports with no new ones, and both budgets are pinned there (39 → 2, 118 → 10). I checked the dropped non-template uses (`x && x.p`, `a ?? o.p`) by hand in the IR: the source reaches the join on a collector-free edge.
17 changes: 17 additions & 0 deletions docs/src/internals/gc-rooting-invariant.md
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,23 @@ dominate the bind, so the register being rooted really is the one that
instruction produced on every path. It is one-sided: an unrecognised call counts
as collecting, so a gap in its model costs a false positive, never a missed bug.

**Phi edges that replace the value are not part of its window (#11604).** When
the bound register reaches the producing instruction through a `phi`, a path
that enters the join through an edge whose operand is something *else* (another
register or a constant) delivers that other value to the slot, not the one being
checked. So the window walk does not take such an edge: a collector reachable
from the producer only through it is not reported. This is the shadow-mode twin
of #7664's `--statepoints` refinement. The shape that needed it is S2's
template coercion (#11554): `phi [ %v, %entry ], [ %coerced, %tmpl_coerce.slow ]`,
where the only collecting call is on the arm that replaces `%v`. Before the
refinement that one join read as 96 violations. A collector on an edge that
*carries* the value, or between the join and the bind, is still reported, and
`--self-test` asserts both (`_SELFTEST_PHI_SAFE_EDGE` / `_SELFTEST_PHI_HAZARD`).
`--stale-registers` applies the same rule when a use reaches its source only
through such a phi (`_stale_use_replacing_edges`). That took the stale budgets
from 39 to 2 (curated) and from 118 to 10 (dependency-scale). The
dependency-scale corpus had been at 457 on `main`, and no step reported it.

For a single file you are iterating on:

```bash
Expand Down
Loading
Loading