Break the rematerialization work into ISS-404 through ISS-407, and fix two invalid issue types - #497
Merged
Merged
Conversation
ISS-403 recorded the defect and, after reading Cranelift, the shape of the fix. This splits the work so the ordering is a dependency graph rather than prose. - ISS-404 adds the materialization-cost predicate. This is the piece Cranelift structurally lacks: its remat rules sit in the target-independent mid-end and so cannot consult a cost, which is why they mark every iconst and float const regardless of what materializing one costs. Measured on AArch64, a constant costs 1 instruction in the single-movz form, 2 or 4 for wider values, and 5 for an f64 — against 1 for the reload it would replace. The first cut is deliberately conservative and target-independent, with a cost callback seam left for the caller that already knows the target. - ISS-405 is the change that fixes the measured doubling: rebuild marked values once per using block instead of reusing a dominating definition. Depends on ISS-404. - ISS-406 folds the AArch64-only EnvironmentField remat into that shared mechanism so future targets do not reimplement it. Depends on ISS-405. - ISS-407 folds constant offsets into load and store displacements. No dependency; adjacent to ISS-405 and measured separately so the two effects stay distinguishable. ISS-403 now depends on all four and closes on the observable outcome. Two things are recorded as non-goals rather than left implicit, both places where copying Cranelift would be wrong here: the ALU-with-one-constant-operand remat rules, which swap which value is live rather than reducing the count, and float and vector constant remat, which loses to a four-byte reload at five instructions on AArch64.
Milky2018
enabled auto-merge
August 7, 2026 07:11
Milky2018
disabled auto-merge
August 7, 2026 07:13
Both were typed `refactor`, which the tracker does not define. Its five types are bug, feature, task, epic and chore, and `task` explicitly covers "implementation, tests, docs, or refactoring work", so that is where these belong. The effect was a warning on every index regeneration and a bogus `refactor` entry in the type column for ISS-399, which is open and therefore listed. ISS-398 is closed, so it only ever showed up in the warnings. Warnings are now empty, and a sweep over the other 402 issue files finds no further invalid types.
Milky2018
enabled auto-merge
August 7, 2026 07:15
This was referenced Aug 11, 2026
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ISS-403 (merged in #495) recorded the defect and, after reading Cranelift, the shape of the fix. This splits the work so the ordering is a dependency graph rather than prose. Tracker files only — no code changes.
EnvironmentFieldremat into the shared mechanismISS-403 now depends on all four and closes on the observable outcome. ISS-404 and ISS-407 are in the ready queue.
Why ISS-404 exists at all
It is the piece Cranelift structurally lacks. Their remat rules live in the target-independent mid-end, so they cannot consult a cost — which is why
remat.islemarks everyiconst,f32constandf64constregardless of what materializing one actually costs on the target.Measured on AArch64, against the 1-instruction reload it would replace:
f64.const 3.14159At one instruction, rebuilding matches the reload and additionally saves the spill store, the stack slot and the cross-block live range — a strict win. At four or five it is several times the reload. The measured fixture's addresses are all in the 1-instruction band, so a cost-gated rule captures the full 2× while staying safe on wide constants.
The first cut is conservative and target-independent (single-
movzform, also one instruction on x64), with a cost-callback seam left for the caller that already knows the target. Wasmoon has an advantage here that Cranelift does not:optimize_with_levelis a plain function call from a JIT that already resolved the target, where Cranelift's ISLE rules are compiled ahead of time.Two non-goals, recorded rather than left implicit
Both are places where copying Cranelift would be wrong here:
y = a + kswaps which value is live rather than reducing the count, and since remat does not recurse, it can move the long live range ontoainstead. Cranelift's own comment only claims "neutral" for this case.Also worth noting for whoever picks this up: Cranelift's
saturating_sub(1)LICM hoist-level default is a magic constant compensating for the missing cost model — the comment says as much, citingvconstnot being easily rematerialized. With a cost predicate that special case is unnecessary.Second commit: two invalid issue types
ISS-398andISS-399were typedrefactor, which the tracker does not define — its five types are bug, feature, task, epic and chore, andtaskexplicitly covers "implementation, tests, docs, or refactoring work".The effect was a warning on every index regeneration plus a bogus
refactorentry in the type column for ISS-399, which is open and therefore listed. ISS-398 is closed, so it only appeared in the warnings. Warnings are now empty, and a sweep over the other 402 issue files finds no further invalid types.Carried on this branch rather than its own: both changes rewrite the derived
issues/README.md, and separate PRs would conflict the moment either merges.