Stream: git-wasmtime

Topic: wasmtime / PR #11790 Constant propagation opts

Wasmtime GitHub notifications bot (Oct 04 2025 at 09:55):

KGrewal1 edited PR #11790.

Wasmtime GitHub notifications bot (Oct 04 2025 at 09:57):

KGrewal1 updated PR #11790.

Wasmtime GitHub notifications bot (Oct 04 2025 at 10:22):

KGrewal1 commented on PR #11790:

will run cargo +nightly fuzz run differential and see if it spots anything too

Wasmtime GitHub notifications bot (Oct 04 2025 at 10:34):

KGrewal1 edited a comment on PR #11790:

will run cargo +nightly fuzz run differential and see if it spots anything too

• OOMd on my machine before it found anything...

Wasmtime GitHub notifications bot (Oct 04 2025 at 12:53):

github-actions[bot] commented on PR #11790:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "isle"

Thus the following users have been cc'd because of the following labels:

• cfallin: isle
• fitzgen: isle

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Oct 06 2025 at 14:47):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 14:47):

alexcrichton created PR review comment:

How come this was removed? Was this an incorrect optimization?

Wasmtime GitHub notifications bot (Oct 06 2025 at 16:37):

KGrewal1 submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 16:37):

KGrewal1 created PR review comment:

Yes - in combination with the urem const optimization led to optimizing imin rem -1 to 0 instead of having it trap which it should

Wasmtime GitHub notifications bot (Oct 06 2025 at 16:44):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 16:44):

alexcrichton created PR review comment:

Ah that's actually a point where Rust and WebAssembly differ. Rust panicks on i32::MIN % -1 while WebAssembly returns 0. CLIF's semantics generally follow wasms, so I believe that this is still a correct optimization.

Wasmtime GitHub notifications bot (Oct 06 2025 at 17:17):

KGrewal1 submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 17:17):

KGrewal1 created PR review comment:

Is that sufficient for CLIF (I guess at the least could cause issues in the rust cranelift backend if this occurs)?

Wasmtime GitHub notifications bot (Oct 06 2025 at 18:30):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 18:30):

alexcrichton created PR review comment:

I'm not really sure what you mean by your question?

Regardless this isn't something we can really change. Wasm i32.rem_s is directly translated to a CLIF srem instruction. The CLIF instruction needs to mach the wasm instruction as a result, where the semantics of wasm is that i32::MIN % -1 is 0.

Wasmtime GitHub notifications bot (Oct 06 2025 at 18:33):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Oct 06 2025 at 18:33):

cfallin created PR review comment:

@KGrewal1 I don't think we'll cause problems for cg_clif if we define srem to work with Wasm's semantics: we're defining it over a superset of the domain that Rust expects. Because other ISAs also work this way (e.g. aarch64 does not trap at all in divide/remainder operations), Rust already needs to reify its trapping semantics at a higher level; so the IR produced by cg_clif must necessarily contain the logic to panic on i32::MIN % -1, but that's already the case.

Wasmtime GitHub notifications bot (Oct 07 2025 at 05:58):

KGrewal1 updated PR #11790.

Wasmtime GitHub notifications bot (Oct 07 2025 at 06:08):

KGrewal1 updated PR #11790.

Wasmtime GitHub notifications bot (Oct 07 2025 at 06:09):

KGrewal1 updated PR #11790.

Wasmtime GitHub notifications bot (Oct 07 2025 at 14:32):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Oct 07 2025 at 14:55):

alexcrichton merged PR #11790.

Last updated: Dec 06 2025 at 07:03 UTC