-
-
Notifications
You must be signed in to change notification settings - Fork 156
perf(repsel): refuse canonical i32 when every hot consumer wants a double (#7128) #7132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
5d0e81b
perf(repsel): refuse canonical i32 when every hot consumer wants a do…
487b1f6
test(repsel): gate the profitability refusal, and give it a fixture case
05014c5
fix(repsel): a self-write in an i32 position is a benefit, not a supp…
064f1bc
docs(repsel): record profitability as a first-class selection question
69cbf86
fix(repsel): carry the self-write marker through unmodelled expressio…
e6e47ae
chore(census): record the #7128 refusal in the baseline floors
0ed73fe
test(repsel): give the mandelbrot reduction its outer-loop f64 use
aa41722
fix(repsel): the self-write exemption follows the representation, not…
7643545
test(repsel): fix the New literal in the self-argument case
7d1c85b
docs: final measured numbers in the changelog fragment
66a57da
test(census): pair every lowered canonical-i32 floor with a refusal m…
4ff8434
docs: correct the refusal accounting in the changelog fragment
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| ### Fixed | ||
|
|
||
| - **repsel: canonical i32 is now chosen on benefit, not only on provability | ||
| (#7128).** `benchmarks/suite/15_mandelbrot.ts` regressed **+14.87% | ||
| instructions retired** at #7121, measured on a quiet Raspberry Pi 5 with | ||
| `perf stat` at a 0.02% noise floor and bisected by binary hash. Wall time did | ||
| not move (48 ms vs 49 ms) because the workload is FP-latency-bound, which is | ||
| why nothing caught it. | ||
|
|
||
| **Root cause, read out of the emitted AArch64 rather than inferred.** The | ||
| innermost loop is `while (x*x + y*y <= 4.0 && iter < MAX_ITER)`. With `iter` | ||
| a boxed double, both exit tests are FP and LLVM fuses them into `fcmp` + | ||
| `fccmp`: **one basic block, 12 instructions, one branch**. #7122's monotone | ||
| loop-induction interval proves `iter ∈ [0, 100]` and #7121 let that proof | ||
| reach the module-init body, so `iter` took a canonical i32 slot — and an | ||
| integer compare cannot fuse with an FP compare. The loop splits into **two | ||
| blocks totalling 14 instructions**, plus a `ucvtf` where the accumulator | ||
| joins. 2 instructions × 8,011,148 innermost iterations ≈ 16.0M, against a | ||
| measured +15.63M. `px` and `py` are the same shape one level out. | ||
|
|
||
| **The defect was not the proof.** The proof is correct. The Let-site | ||
| eligibility gate in `stmt/let_stmt.rs` was a conjunction of "may we?" terms | ||
| with no "should we?" term anywhere in it, so widening the proof | ||
| automatically widened the emission. | ||
|
|
||
| **The fix is a profitability model** (`collectors/repsel_benefit.rs`), | ||
| consulted by the selection gate as one more conjunct. For an i32-range value | ||
| a `double` is a lossless, equal-cost representation of `+`, `-` and | ||
| comparison; canonical i32 only *buys* something where the consumer cannot | ||
| take a double without a conversion — array/typed-array indexing, bitwise | ||
| operands, `Math.imul` — and becomes a *cost* the moment a hot consumer needs | ||
| the double back. So 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 stays boxed. The model only ever refuses, so every uncertainty | ||
| resolves toward "not a cost" (comparison is neutral on both sides — | ||
| `for (let i = 0; i < n; i++)` with a `number` parameter must keep | ||
| promoting). | ||
|
|
||
| Measured on the Pi, 11 repeats, `perf stat -e instructions:u`: | ||
|
|
||
| | workload | before | after | Δ | | ||
| |---|---|---|---| | ||
| | `15_mandelbrot` | 120,738,701 | 105,110,087 | **−12.94%** | | ||
| | `11_prime_sieve` (the #7121 win) | 2,597,182,143 | 2,597,177,676 | −0.00% | | ||
| | `08_string_concat` (the #7121 `Str` win) | 30,281,798 | 30,281,711 | −0.00% | | ||
|
|
||
| Both #7121 wins re-measured against the `at7122` arm with this compiler: | ||
| canonical `Str` **−4.12%**, canonical i32 **−1.05%**. They hold by | ||
| construction — the linked binary for each is byte-identical to `main`'s. | ||
|
|
||
| Over the whole 26-workload census corpus the emitted object changes on | ||
| **exactly one** benchmark, and there its disassembly is byte-identical to the | ||
| pre-#7121 compiler's. The other refused promotions were already emitting | ||
| byte-identical code, so the census counts fall (canonical-i32 64 → 55) | ||
| without a single emitted byte moving. Every lowered floor is paired with a | ||
| `no_i32_consuming_use` minimum, so a floor that fell because a promotion was | ||
| refused cannot silently accommodate a different promotion going missing. | ||
|
|
||
| **Gated.** Every other number in the promotion census is a floor, and a floor | ||
| cannot go red when a compiler promotes *more*. `REFUSAL_FLOORS` in | ||
| `scripts/compiler_output_harness/repsel_census.py` gives the refusal its own | ||
| minimum; `benchmarks/repsel_census/fixtures/fixture_loop_bounded_i32.ts` now | ||
| carries `iterate()` and `mixedWithFloat()` side by side — same #7110 interval | ||
| proof, opposite verdict, differing only in what consumes the counter — so | ||
| neither an always-yes nor an always-no rule can satisfy the file. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.