Stream: git-wasmtime

Topic: wasmtime / PR #11748 Constant propagation opts

Wasmtime GitHub notifications bot (Sep 27 2025 at 13:18):

KGrewal1 edited PR #11748.

Wasmtime GitHub notifications bot (Sep 27 2025 at 13:18):

KGrewal1 edited PR #11748:

Add opts for ctz and clz - issues #11725
Add opts for srem and urem

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:43):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:59):

cfallin submitted PR review:

Thanks!

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:59):

cfallin created PR review comment:

I think we need to match ty here too, otherwise the sign-extends in the helpers are wrong, right?

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:59):

cfallin created PR review comment:

I'm curious why the checked_sub is necessary here, rather than an assert that ty.bits() <= 64 (which should be the case for any Imm64)?

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:59):

cfallin created PR review comment:

i64::from here, preferably? (u32 should have an infallible From impl for this)

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:59):

cfallin created PR review comment:

Can we extract the sign-extend pattern here into a helper? Something like

fn sign_extend(raw: i64, from_bits: u32, to_bits: u32) -> i64 { ... }

then we can use it below too.

Wasmtime GitHub notifications bot (Sep 29 2025 at 18:59):

cfallin created PR review comment:

Can we add tests for:

• INT_MIN % -1, for narrow and wide types (i8, i16, i32, i64)
• negative and positive values in numerator and denominator for narrow and wide types

and runtests that are known to trigger these optimizations as well? (For the runtests, set opt_level; we won't be able to directly assert on the compilation output, but we should at least lock down that the computations produce the correct value)

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:12):

KGrewal1 submitted PR review.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:12):

KGrewal1 created PR review comment:

This was for consistency with the sdiv helper - agreed this makes sense for both

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:19):

KGrewal1 submitted PR review.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:19):

KGrewal1 created PR review comment:

yes - this sign extends both are the same width (think this needs to change in sdiv too)O

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:19):

KGrewal1 edited PR review comment.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:24):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:24):

KGrewal1 edited PR review comment.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:25):

KGrewal1 submitted PR review.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:25):

KGrewal1 created PR review comment:

where is the best place to put helpers for this code - can stick it at the top / bottom outside the macro, but seems unideal

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:27):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 29 2025 at 19:27):

cfallin created PR review comment:

Ah, right, it's a little awkward because of that -- I would perhaps put it on the Imm64 itself then? Something like Imm64::sign_extended(&self, from_bits: u32, to_bits: u32) -> i64 and `Imm64::from_narrow(bits: i64, bits: i64) to go the other way and encapsulate the masking as well?

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:08):

KGrewal1 requested dicej for a review on PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:08):

KGrewal1 requested wasmtime-core-reviewers for a review on PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:08):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:10):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:13):

cfallin submitted PR review:

Two more things I noticed -- one below, and then the submodules for spec_testsuite and wasi-common have been changed as well, which is probably unintentional (easy footgun with git forgetting to update submodules when merging upstream).

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:13):

cfallin created PR review comment:

It looks like there's commented-out code here -- can we remove it?

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:14):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:25):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:26):

KGrewal1 submitted PR review.

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:26):

KGrewal1 created PR review comment:

Realized there are already functions for this so used those :)

Wasmtime GitHub notifications bot (Sep 29 2025 at 20:30):

KGrewal1 updated PR #11748.

Wasmtime GitHub notifications bot (Oct 01 2025 at 20:11):

cfallin submitted PR review:

Thanks!

Wasmtime GitHub notifications bot (Oct 01 2025 at 20:37):

cfallin merged PR #11748.

Wasmtime GitHub notifications bot (Oct 03 2025 at 16:09):

alexcrichton commented on PR #11748:

This ended up having a bug detected via fuzzing so I'm posting a revert of this at https://github.com/bytecodealliance/wasmtime/pull/11785 as the release branch is pretty near. @KGrewal1 would you be up for investigating the fuzz failure (linked in https://github.com/bytecodealliance/wasmtime/pull/11785) and resubmitting this PR?

Last updated: Dec 06 2025 at 07:03 UTC