wasmtime / PR #15022 fix: honor CLIF settings and RISC-V ... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #15022 fix: honor CLIF settings and RISC-V ...

Wasmtime GitHub notifications bot (Oct 27 2026 at 03:01):

mvanhorn opened PR #15022 from mvanhorn:fix/11050-clif-util-riscv64-settings to bytecodealliance:main:

Motivation

It addresses the behavior reported in issue #11050.

What this changes

This change edits cranelift/src/compile.rs and cranelift/src/run.rs. The edited code sits in Options, run and handle_module. Across 3 files the diff adds 868 lines and removes 48.

How it was tested

Tests for the wasmtime Options change live in cranelift/tests/clif-util.rs. The targeted tests for the wasmtime Options change pass locally. The tests in cranelift/tests/clif-util.rs fail without the fix in the wasmtime Options change (checked by reverting the source changes).

Fixes #11050

AI was used for assistance.

Wasmtime GitHub notifications bot (Oct 27 2026 at 03:02):

mvanhorn removed the review request for wasmtime-compiler-reviewers from PR #15022.

Wasmtime GitHub notifications bot (Oct 27 2026 at 03:02):

mvanhorn requested cfallin for a review on PR #15022.

Wasmtime GitHub notifications bot (Oct 27 2026 at 03:02):

mvanhorn requested wasmtime-compiler-reviewers for a review on PR #15022.

Wasmtime GitHub notifications bot (Oct 27 2026 at 05:50):

github-actions[bot] added the label cranelift on PR #15022.

Wasmtime GitHub notifications bot (Oct 28 2026 at 23:44):

:memo: cfallin submitted PR review:

Thanks for the PR, @mvanhorn. However I have to admit I'm having trouble wrapping my head around this -- it's an absolutely enormous PR for the functionality and adds a lot of special cases for something that (IMHO) we'd prefer to be simple and predictable. A few thoughts:

The original riscv64 vs riscv64gc mismatch in the reported issue is I think one of the root problems. Those are different ISAs from a target-triple perspective and it's probably more predictable if our tools don't try to fudge one to become the other; I don't like that run has an existing special case and I don't want to propagate it further.

The absolutely enormous blob of (presumably LLM-generated) unit-tests here is out-of-place and unnecessary, I think.

Could you see if you could try to resolve the issue in a simpler way? Unifying behavior between compile and run is a totally reasonable goal, for what it's worth! I just don't want to see +868 lines of code and riscv-specific test cases and all this complexity to resolve it, is all. Thanks!

Wasmtime GitHub notifications bot (Oct 28 2026 at 23:44):

:memo: cfallin submitted PR review:

Thanks for the PR, @mvanhorn. However I have to admit I'm having trouble wrapping my head around this -- it's an absolutely enormous PR for the functionality and adds a lot of special cases for something that (IMHO) we'd prefer to be simple and predictable. A few thoughts:

The original riscv64 vs riscv64gc mismatch in the reported issue is I think one of the root problems. Those are different ISAs from a target-triple perspective and it's probably more predictable if our tools don't try to fudge one to become the other; I don't like that run has an existing special case and I don't want to propagate it further.

The absolutely enormous blob of (presumably LLM-generated) unit-tests here is out-of-place and unnecessary, I think.

Could you see if you could try to resolve the issue in a simpler way? Unifying behavior between compile and run is a totally reasonable goal, for what it's worth! I just don't want to see +868 lines of code and riscv-specific logic and all this complexity to resolve it, is all. Thanks!

Wasmtime GitHub notifications bot (Oct 29 2026 at 13:19):

mvanhorn commented on PR #15022:

That's fair, and I agree on both points. Treating riscv64 and riscv64gc as the same ISA is the wrong direction, and the test file is far more than this needs.

Looking at it again, the actual gap in compile is small: when --target is given, handle_module builds the ISA from the command line only, so the file's set lines (here enable_multi_ret_implicit_sret=true) are dropped. Passing --set enable_multi_ret_implicit_sret=true on the command line already works around it.

The smallest change I can see is: if the file has a target line whose triple matches --target exactly, use that ISA from the file (which carries its flags), otherwise keep today's behavior. No architecture aliasing, and the riscv64 vs riscv64gc mismatch on run stays an error. That should be a few lines in compile.rs plus at most one filetest. Does that match what you had in mind, or would you rather compile stay as is and this be documented as using --set?

Last updated: Oct 31 2026 at 23:03 UTC