fitzgen opened PR #14163 from fitzgen:remove-narrow-and-extending-stores-and-loads to bytecodealliance:main:
The former is identical to an
ireducefollowed by a regular store, and the
latter is identical to a regular load of the narrow width followed by an
extend. No need to have fused versions of these operations in CLIF, our lowering
backends can match on these patterns to generate the desired machine code just
fine.Large diff, but it's mostly just golden test expectations.
Depends on https://github.com/bytecodealliance/wasmtime/pull/14162
fitzgen requested cfallin for a review on PR #14163.
fitzgen requested wasmtime-compiler-reviewers for a review on PR #14163.
fitzgen requested wasmtime-compiler-s390x-reviewers for a review on PR #14163.
fitzgen requested wasmtime-core-reviewers for a review on PR #14163.
alexcrichton commented on PR #14163:
I have a vague recollection that in the past we attempted this and concluded that the increase in compile times and IR size meant that it wasn't worth the change, but I also can't seem to find that context now. Perhaps this'd be worth at least running through sightglass though?
cfallin commented on PR #14163:
I agree that this is a good simplification in principle but also want to make sure we minimize performance impact -- so +1 to a Sightglass run, but also skimming through the test expectations, I see at least a few cases (e.g., here) where load becomes a load+extend in machine code so we should make sure we have all the equivalent lowerings to re-fuse load+extend (and reduce+store).
github-actions[bot] added the label cranelift:area:aarch64 on PR #14163.
github-actions[bot] added the label cranelift:area:x64 on PR #14163.
github-actions[bot] added the label cranelift:meta on PR #14163.
github-actions[bot] added the label cranelift on PR #14163.
:cross_mark: fitzgen closed without merge PR #14163.
fitzgen commented on PR #14163:
Huh. I had forgot that we investigated this before.
I agree that this is a good simplification in principle but also want to make sure we minimize performance impact -- so +1 to a Sightglass run, but also skimming through the test expectations, I see at least a few cases (e.g., here) where load becomes a load+extend in machine code so we should make sure we have all the equivalent lowerings to re-fuse load+extend (and reduce+store).
This only happens when the load and extend are across blocks, because our sinking doesn't work across blocks.
I have a vague recollection that in the past we attempted this and concluded that the increase in compile times and IR size meant that it wasn't worth the change, but I also can't seem to find that context now. Perhaps this'd be worth at least running through sightglass though?
I ran it through sightglass and it was indeed about a 0.5% regression in walltime for compilation and up to ~5% regression for execution on some benchmarks. The latter is much worse than I expected, so I think we can't proceed here until we either teach lowering to sink across blocks or implement block merging in the midend. Will close this PR accordingly.
Last updated: Aug 30 2026 at 10:08 UTC