Stream: git-wasmtime

Topic: wasmtime / PR #13237 Type-guard bitop optimization rules ...

Wasmtime GitHub notifications bot (Apr 30 2026 at 05:17):

bongjunj opened PR #13237 from bongjunj:fix-13229 to bytecodealliance:main:

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Cause 1

bitops such as band, bor, ... are not constrained to receive only integer inputs like iadd, isub, and icmp.
The IR builder shows this fact:

• https://docs.rs/cranelift-codegen/latest/cranelift_codegen/ir/trait.InstBuilder.html#method.band
• https://docs.rs/cranelift-codegen/latest/cranelift_codegen/ir/trait.InstBuilder.html#method.iadd

Note that there is no float input for iadd, whereas band can take float inputs.

Cause 2

iconst_s and iconst_u provide convenient ways to construct constants on RHS, but they are the main contract breaker of ISLE type system. They are declared as non-partial terms, while their definition is not total. They only work for integer and vec128 types and this is why we need type guards to use them. This fact is not explicitly encoded in the declaration of the terms. I guess this is not a proper way to use ISLE, since we cannot trust the ISLE compiler and the ISLE ruleset in this way... But, to fix this, I guess we need more discussions that could easily go out of the scope of fix, so I want to leave this as a future work.

Fix

• I added ty_iconst partial extractor which rules constructing terms with iconst_s and iconst_u can safely use.
• The widened rules with bitops in #13199 are now guarded with ty_iconst.
• The reported regression case is added, which tests a float input for band term.
  

Wasmtime GitHub notifications bot (Apr 30 2026 at 05:17):

bongjunj requested alexcrichton for a review on PR #13237.

Wasmtime GitHub notifications bot (Apr 30 2026 at 05:17):

bongjunj requested wasmtime-compiler-reviewers for a review on PR #13237.

Wasmtime GitHub notifications bot (Apr 30 2026 at 05:18):

bongjunj edited PR #13237:

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

This resolves #13229

Cause 1

bitops such as band, bor, ... are not constrained to receive only integer inputs like iadd, isub, and icmp.
The IR builder shows this fact:

• https://docs.rs/cranelift-codegen/latest/cranelift_codegen/ir/trait.InstBuilder.html#method.band
• https://docs.rs/cranelift-codegen/latest/cranelift_codegen/ir/trait.InstBuilder.html#method.iadd

Note that there is no float input for iadd, whereas band can take float inputs.

Cause 2

iconst_s and iconst_u provide convenient ways to construct constants on RHS, but they are the main contract breaker of ISLE type system. They are declared as non-partial terms, while their definition is not total. They only work for integer and vec128 types and this is why we need type guards to use them. This fact is not explicitly encoded in the declaration of the terms. I guess this is not a proper way to use ISLE, since we cannot trust the ISLE compiler and the ISLE ruleset in this way... But, to fix this, I guess we need more discussions that could easily go out of the scope of fix, so I want to leave this as a future work.

Fix

• I added ty_iconst partial extractor which rules constructing terms with iconst_s and iconst_u can safely use.
• The widened rules with bitops in #13199 are now guarded with ty_iconst.
• The reported regression case is added, which tests a float input for band term.
  

Wasmtime GitHub notifications bot (Apr 30 2026 at 09:25):

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

Wasmtime GitHub notifications bot (Apr 30 2026 at 09:25):

github-actions[bot] added the label isle on PR #13237.

Wasmtime GitHub notifications bot (Apr 30 2026 at 09:26):

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

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 (Apr 30 2026 at 14:03):

:thumbs_up: alexcrichton submitted PR review:

Thanks! To bikeshed this a bit, maybe ty_int_or_vec128? The name ty_iconst can make sense in retrospect once it's understood what it does but just by reading it it's not clear.

Another possibility is ty_int_lane because in Cranelift I believe all types are considered to have lanes, and scalars just have one lane

Wasmtime GitHub notifications bot (May 04 2026 at 01:55):

bongjunj updated PR #13237.

Wasmtime GitHub notifications bot (May 04 2026 at 01:56):

bongjunj commented on PR #13237:

@alexcrichton
I'd prefer ty_int_or_vec128 because ty_int_lane would imply for all integer-lane types which semantically cover more than 128-bit vectors which iconst_s/u constructors only cover 128-bit vectors.
I pushed this change. Thanks for the review!

Wasmtime GitHub notifications bot (May 04 2026 at 03:40):

bongjunj edited a comment on PR #13237:

@alexcrichton
I'd prefer ty_int_or_vec128 because ty_int_lane would imply for all integer-lane types which semantically cover more than 128-bit vector, but iconst_s/u constructors only cover 128-bit vectors.
I pushed this change. Thanks for the review!

Wasmtime GitHub notifications bot (May 04 2026 at 03:40):

bongjunj edited a comment on PR #13237:

@alexcrichton
I'd prefer ty_int_or_vec128 because ty_int_lane would imply for all integer-lane types which semantically cover more than 128-bit vector (in my knowledge), but iconst_s/u constructors only cover 128-bit vectors.
I pushed this change. Thanks for the review!

Wasmtime GitHub notifications bot (May 04 2026 at 16:26):

:thumbs_up: alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (May 04 2026 at 16:26):

alexcrichton added PR #13237 Type-guard bitop optimization rules (#13229) to the merge queue

Wasmtime GitHub notifications bot (May 04 2026 at 16:52):

:check: alexcrichton merged PR #13237.

Wasmtime GitHub notifications bot (May 04 2026 at 16:52):

alexcrichton removed PR #13237 Type-guard bitop optimization rules (#13229) from the merge queue

Last updated: Jun 01 2026 at 09:49 UTC