Stream: git-wasmtime

Topic: wasmtime / PR #14374 cranelift: lower i128 umin/umax/smin...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 02:39):

agourakis82 opened PR #14374 from agourakis82:users/agourakis/cranelift-x64-umin-i128 to bytecodealliance:main:

Summary

The mid-end rewrites select(icmp ugt/ult/..., a, b) into integer min/max. On x86_64 those forms were only lowered for fits_in_64, so optimized i128 code hit:

Unsupported feature: should be implemented in ISLE: inst = umin.i128 ...

Fix

Lower umin / umax / smin / smax for $I128 through the existing i128 emit_cmp + lower_select path (same shape as AArch64's scalar-int lowering).

Test plan

Fixes #13790

AI assistance

Developed with AI tool assistance (Claude Code) under human direction and review. I am the author of record, reviewed the generated diffs before opening the PR, and can answer questions about the change during review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 02:39):

agourakis82 requested fitzgen for a review on PR #14374.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 02:39):

agourakis82 requested wasmtime-compiler-reviewers for a review on PR #14374.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 03:48):

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

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 03:48):

github-actions[bot] added the label cranelift:area:x64 on PR #14374.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 14:51):

agourakis82 commented on PR #14374:

Ready for review — this one is independent of the #14323 region semantics debate.

x64 was missing i128 umin/umax/smin/smax lowering that the mid-end select→minmax rewrite can produce. Mirrors the AArch64 scalar-int approach via existing emit_cmp + lower_select.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 15:34):

agourakis82 commented on PR #14374:

@fitzgen gentle bump — this one’s unrelated to the region discussion on #14323. Happy to tweak if you’d rather expand mid-end-side instead of adding the i128 lowerings.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 15:37):

agourakis82 edited a comment on PR #14374:

Independent of #14323.

x64 didn’t lower i128 umin/umax/smin/smax; mid-end select→minmax can produce them. Patch adds those lowerings via the existing i128 cmp/select path.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 15:38):

agourakis82 edited PR #14374:

Mid-end select→minmax can produce umin/umax/smin/smax on i128. x64 only lowered those for fits_in_64, so optimized code hit an unsupported-ISLE trap (#13790).

Adds i128 lowerings through the existing emit_cmp + lower_select path.

Tests:

Fixes #13790

Assisted-by: Claude

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 16:01):

alexcrichton commented on PR #14374:

@agourakis82 please turn down the frequency to which you post and comment on things. It's been 13 hours since this PR was posted and for most of that we were asleep. This is an open source project and we can't respond to PRs immediately, so we ask for patience on your end while time is found to review things.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 16:42):

agourakis82 commented on PR #14374:

Sorry about the noise — I’ll leave it alone and wait for review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 17:20):

:memo: fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 17:20):

:speech_balloon: fitzgen created PR review comment:

This comment is not really useful for code readers. The first part is a historical tidbit that is ultimately incidental, and the second is obvious for ISLE code.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 17:20):

:speech_balloon: fitzgen created PR review comment:

Let's make this test compile precise-output and re-bless. As currently written this is doing nothing other than ensuring that the programs compile successfully. It is not actually checking that the relevant lowerings or rewrites happen and are reflected in the output.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 17:20):

:speech_balloon: fitzgen created PR review comment:

Can you enable all runtest ISAs for this, since the test is not x86-64 specific? And maybe test it for a handful of different inputs (as function parameters) instead of hard-coding constants in the function body.

Also this comment is not really useful. It needs to be lowerable everywhere, not just x64, where there happened to be a bug. Please do a better job of cleaning up LLM comments, here and elsewhere, and now and in the future. Making maintainers do that for you is not a good use of maintainer time.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 18:35):

agourakis82 commented on PR #14374:

Addressed the review notes: removed the incidental comments, blessed precise-output on the x64 compile test, and broadened the runtest to the other ISAs with parameterized inputs.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 18:35):

agourakis82 updated PR #14374.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 18:38):

:memo: agourakis82 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 22 2026 at 18:38):

:speech_balloon: agourakis82 created PR review comment:

Sorry...doing my best!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2026 at 21:06):

dustinCodes84600 commented on PR #14374:

I hit this same unsupported-ISLE path in a Cranelift embedding that emits i128 compare/select pairs on x86_64 with speed_and_size. Mid-end conversion to umin made it behave differently from aarch64, so direct lowering plus shared runtime coverage matters for that setup.

also made a visual walkthrough while reading through the diff: https://flyovers.dev/flyover/6e577e4e-f061-4bf8-a95b-ad4a68c3eadc

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 16:25):

:thumbs_up: fitzgen submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 16:26):

fitzgen added PR #14374 cranelift: lower i128 umin/umax/smin/smax on x86_64 to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 16:52):

:check: fitzgen merged PR #14374.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 16:52):

fitzgen removed PR #14374 cranelift: lower i128 umin/umax/smin/smax on x86_64 from the merge queue.

Last updated: Oct 11 2026 at 04:10 UTC