Stream: git-wasmtime

Topic: wasmtime / PR #14793 aarch64: remove `MInst::Extend`

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:44):

Rafferty97 opened PR #14793 from Rafferty97:remove-sextend-aarm64 to bytecodealliance:main:

Motivation

My last PR (#14187) introduced MInst::BitfieldMove to represent all instances of the ubfm and sbfm instructions.
As a result, zero- and sign-extensions can now be expressed as MInst::BitfieldMove or MInst::Mov directly, making
the MInst::Extend variant redundant, so this PR removes it.

Changes

I have removed the MInst::Extend variant, and replaced sites that constructed it with calls to MInst::gen_extend,
which produces a MInst::BitfieldMove or MInst::Mov as necessary.

I've also reimplemented the extend helper function defiled in ISLE in a similar fashion, and added a helper
in the process (u8_into_uimm6).

The code that performed code generation for MInst::Extend had special-cased the case where from_bits == 1,
producing an and instruction rather than a ubfm instruction on performance grounds.
I haven't carried this over to the new gen_extend function as it would be unreachable in practice.
(Claude tells me this is a relic from when boolean types like b1 existed and was now effectively dead code,
but I haven't confirmed this myself.)

I've changed the test cases in codegen/src/isa/aarch64/inst/emit_tests.rs that exercised MInst::Extend
to use MInst::BitfieldMove instead, and removed the cases that lowered to different instructions, as those
cases should already be covered elsewhere. I also added a few extra cases with immr != 0 for better coverage.

To preserve the behaviour of print_with_state, I've added a special case to the MInst::BitfieldMove branch
that prints the appropriate aliases for ubfm/sbfm such that all the file tests still pass without adjustment.

Finally, in order to get isle/veri/isaspec/script/generate.sh -l to work, I had to replace a few
instances of std::fmt with core::fmt.

Future work

I dug a bit deeper into the optimisation of using and in place of ubfm when zero-extending a 1-bit input,
and it seems the rationale is sound and extends to zero-extending 8-bit and 16-bit inputs. According to Claude,
LLVM does this for all targets, which is evidence that it's a worthwhile optimisation to consider for a future PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:44):

Rafferty97 requested alexcrichton for a review on PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:44):

Rafferty97 requested wasmtime-compiler-reviewers for a review on PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:49):

Rafferty97 edited PR #14793:

Motivation

My last PR (#14187) introduced MInst::BitfieldMove to represent all instances of the ubfm and sbfm instructions.
As a result, zero- and sign-extensions can now be expressed as MInst::BitfieldMove or MInst::Mov directly, making
the MInst::Extend variant redundant, so this PR removes it.

Changes

I have removed the MInst::Extend variant, and replaced sites that constructed it with calls to MInst::gen_extend,
which produces a MInst::BitfieldMove or MInst::Mov as necessary.

I've also reimplemented the extend helper function defiled in ISLE in a similar fashion, and added a helper
in the process (u8_into_uimm6).

The code that performed code generation for MInst::Extend had special-cased the case where from_bits == 1,
producing an and instruction rather than a ubfm instruction on performance grounds.
I haven't carried this over to the new gen_extend function as it would be unreachable in practice.
(Claude tells me this is a relic from when boolean types like b1 existed and was now effectively dead code,
but I haven't confirmed this myself.)

I've changed the test cases in codegen/src/isa/aarch64/inst/emit_tests.rs that exercised MInst::Extend
to use MInst::BitfieldMove instead, and removed the cases that lowered to different instructions, as those
cases should already be covered elsewhere. I also added a few extra cases with immr != 0 for better coverage.

To preserve the behaviour of print_with_state, I've added a special case to the MInst::BitfieldMove branch
that prints the appropriate aliases for ubfm/sbfm such that all the file tests still pass without adjustment.

Finally, in order to get isle/veri/isaspec/script/generate.sh -l to work, I had to replace a few
instances of std::fmt with core::fmt.

Future work

I dug a bit deeper into the optimisation of using and in place of ubfm when zero-extending a 1-bit input,
and it seems the rationale is sound and extends to zero-extending any input. According to Claude,
LLVM does this for all aarch64 targets, which is evidence that it's a worthwhile optimisation to consider for a future PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:52):

Rafferty97 edited PR #14793:

Motivation

My last PR (#14187) introduced MInst::BitfieldMove to represent all instances of the ubfm and sbfm instructions. As a result, zero- and sign-extensions can now be expressed as MInst::BitfieldMove or MInst::Mov directly, making the MInst::Extend variant redundant, so this PR removes it.

Changes

I have removed the MInst::Extend variant, and replaced sites that constructed it with calls to MInst::gen_extend,
which produces a MInst::BitfieldMove or MInst::Mov as necessary.

I've also reimplemented the extend helper function defined in ISLE in a similar fashion, and added a helper
in the process (u8_into_uimm6).

The code that performed code generation for MInst::Extend had special-cased the case where from_bits == 1,
producing an and instruction rather than a ubfm instruction on performance grounds. I haven't carried this over to the new gen_extend function as it would be unreachable in practice. (Claude tells me this is a relic from when boolean types like b1 existed and was now effectively dead code, but I haven't confirmed this myself.)

I've changed the test cases in codegen/src/isa/aarch64/inst/emit_tests.rs that exercised MInst::Extend
to use MInst::BitfieldMove instead, and removed the cases that lowered to different instructions, as those
cases should already be covered elsewhere. I also added a few extra cases with immr != 0 for better coverage.

To preserve the behaviour of print_with_state, I've added a special case to the MInst::BitfieldMove branch
that prints the appropriate aliases for ubfm/sbfm such that all the file tests still pass without adjustment.

Finally, in order to get isle/veri/isaspec/script/generate.sh -l to work, I had to replace a few
instances of std::fmt with core::fmt. This isn't related to these changes, so I can separate it into a separate PR if that's preferred.

Future work

I dug a bit deeper into the optimisation of using and in place of ubfm when zero-extending a 1-bit input,
and it seems the rationale is sound and extends to zero-extending any input. According to Claude,
LLVM does this for all aarch64 targets, which is evidence that it's a worthwhile optimisation to consider for a future PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:54):

Rafferty97 edited PR #14793:

Motivation

My last PR (#14187) introduced MInst::BitfieldMove to represent all instances of the ubfm and sbfm instructions. As a result, zero- and sign-extensions can now be expressed as MInst::BitfieldMove or MInst::Mov directly, making the MInst::Extend variant redundant, so this PR removes it.

Changes

I have removed the MInst::Extend variant, and replaced sites that constructed it with calls to MInst::gen_extend,
which produces a MInst::BitfieldMove or MInst::Mov as necessary.

I've also reimplemented the extend helper function defined in ISLE in a similar fashion, and added a helper
in the process (u8_into_uimm6).

The code that performed code generation for MInst::Extend had special-cased the case where from_bits == 1,
producing an and instruction rather than a ubfm instruction on performance grounds. I haven't carried this over to the new gen_extend function as it would be unreachable in practice. (Claude tells me this is a relic from when boolean types like b1 existed and was now effectively dead code, but I haven't confirmed this myself.)

I've changed the test cases in codegen/src/isa/aarch64/inst/emit_tests.rs that exercised MInst::Extend
to use MInst::BitfieldMove instead, and removed the cases that lowered to different instructions, as those
cases should already be covered elsewhere. I also added a few extra cases with immr != 0 for better coverage.

To preserve the behaviour of print_with_state, I've added a special case to the MInst::BitfieldMove branch
that prints the appropriate aliases for ubfm/sbfm such that all the file tests still pass without adjustment.

Finally, in order to get isle/veri/isaspec/script/generate.sh -l to work, I had to replace a few
instances of std::fmt with core::fmt. This isn't related to these changes, so I can separate it into a separate PR if that's preferred.

Future work

I dug a bit deeper into the optimisation of using and in place of ubfm when zero-extending a 1-bit input,
and it seems the rationale is sound and extends to zero-extending any input (other than 32-bit to 64-bit, which is a mov). According to Claude,
LLVM does this for all aarch64 targets, which is evidence that it's a worthwhile optimisation to consider for a future PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 06:54):

Rafferty97 edited PR #14793:

Motivation

My last PR (#14187) introduced MInst::BitfieldMove to represent all instances of the ubfm and sbfm instructions. As a result, zero- and sign-extensions can now be expressed as MInst::BitfieldMove or MInst::Mov directly, making the MInst::Extend variant redundant, so this PR removes it.

Changes

I have removed the MInst::Extend variant, and replaced sites that constructed it with calls to MInst::gen_extend,
which produces a MInst::BitfieldMove or MInst::Mov as necessary.

I've also reimplemented the extend helper function defined in ISLE in a similar fashion, and added a helper
in the process (u8_into_uimm6).

The code that performed code generation for MInst::Extend had special-cased the case where from_bits == 1,
producing an and instruction rather than a ubfm instruction on performance grounds. I haven't carried this over to the new gen_extend function as it would be unreachable in practice. (Claude tells me this is a relic from when boolean types like b1 existed and was now effectively dead code, but I haven't confirmed this myself.)

I've changed the test cases in codegen/src/isa/aarch64/inst/emit_tests.rs that exercised MInst::Extend
to use MInst::BitfieldMove instead, and removed the cases that lowered to different instructions, as those
cases should already be covered elsewhere. I also added a few extra cases with immr != 0 for better coverage.

To preserve the behaviour of print_with_state, I've added a special case to the MInst::BitfieldMove branch
that prints the appropriate aliases for ubfm/sbfm such that all the file tests still pass without adjustment.

Finally, in order to get isle/veri/isaspec/script/generate.sh -l to work, I had to replace a few
instances of std::fmt with core::fmt. This isn't related to these changes, so I can separate it into a separate PR if that's preferred.

Future work

I dug a bit deeper into the optimisation of using and in place of ubfm when zero-extending a 1-bit input,
and it seems the rationale is sound and extends to zero-extending any input (other than 32-bit to 64-bit, which is a mov). It seems LLVM does this for all aarch64 targets, which is evidence that it's a worthwhile optimisation to consider for a future PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 07:23):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 07:55):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 08:39):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 09:28):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 09:56):

Rafferty97 edited PR #14793:

Motivation

My last PR (#14187) introduced MInst::BitfieldMove to represent all instances of the ubfm and sbfm instructions. As a result, zero- and sign-extensions can now be expressed as MInst::BitfieldMove or MInst::Mov directly, making the MInst::Extend variant redundant, so this PR removes it.

Changes

I have removed the MInst::Extend variant, and replaced sites that constructed it with calls to MInst::gen_extend,
which produces a MInst::BitfieldMove or MInst::Mov as necessary.

I've also reimplemented the extend helper function defined in ISLE in a similar fashion, and added a helper
in the process (u8_into_uimm6).

The code that performed code generation for MInst::Extend had special-cased the case where from_bits == 1,
producing an and instruction rather than a ubfm instruction on performance grounds. I haven't carried this over to the new gen_extend function as it would be unreachable in practice. (Claude tells me this is a relic from when boolean types like b1 existed and was now effectively dead code, but I haven't confirmed this myself.)

I've changed the test cases in codegen/src/isa/aarch64/inst/emit_tests.rs that exercised MInst::Extend
to use MInst::BitfieldMove instead, and removed the cases that lowered to different instructions, as those
cases should already be covered elsewhere. I also added a few extra cases with immr != 0 for better coverage.

To preserve the behaviour of print_with_state, I've added a special case to the MInst::BitfieldMove branch
that prints the appropriate aliases for ubfm/sbfm such that all the file tests still pass without adjustment.

Finally, in order to get isle/veri/isaspec/script/generate.sh -l to work, I had to replace a few
instances of std::fmt with core::fmt. This isn't related to these changes, so I can separate it into a separate PR if that's preferred.

Verification

In removing MInst::Extend, I necessarily also had to remove the code that generated the extend.isle spec file. To avoid a verification regression, I've added limited verification of MInst::BitfieldMove, restricted to just the cases needed for zero-/sign-extension.

This is also the part I'm most unsure about, so I'm more than open to feedback.

Future work

I dug a bit deeper into the optimisation of using and in place of ubfm when zero-extending a 1-bit input,
and it seems the rationale is sound and extends to zero-extending any input (other than 32-bit to 64-bit, which is a mov). It seems LLVM does this for all aarch64 targets, which is evidence that it's a worthwhile optimisation to consider for a future PR.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 10:01):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:49):

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

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:49):

github-actions[bot] added the label cranelift:meta on PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:49):

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

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:49):

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

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:49):

github-actions[bot] added the label winch on PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 17 2026 at 12:50):

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

Subscribe to Label Action

cc @avanhatt, @cfallin, @fitzgen, @mmcloughlin, @saulecabrera

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

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

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Oct 19 2026 at 15:07):

alexcrichton commented on PR #14793:

This all looks reasonable to me, but I'm also not a great reviewer yet for the (spec ...) bits. @cfallin could you take a look and review those bits perhaps?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 19 2026 at 15:07):

alexcrichton requested cfallin for a review on PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 19 2026 at 15:21):

:thumbs_up: cfallin submitted PR review:

This looks fine to me -- thanks a bunch for the work here improving our instruction definitions!

One thought below but not critical if you want to defer.

The spec changes I verified mostly by looking at the generated output of isaspec; the updates to the generator itself are also a little outside my wheelhouse but I'm happy to see this merge with the (eyeballed-)correct spec output. cc @mmcloughlin @avanhatt for thought on handling arbitrary bitfield instructions as a next step -- I don't think we need them for Wasm's footprint but it might be nice?

view this post on Zulip Wasmtime GitHub notifications bot (Oct 19 2026 at 15:21):

:speech_balloon: cfallin created PR review comment:

I know this is pre-existing but we should probably assert!(to_bits >= 32) here; the logic here makes clear that we can only support 32- and 64-bit cases (OperandSize only has those two cases). Also given that, it may be a bit clearer to say OperandSize::Size64 here directly?

(Actually, there's a slightly deeper refactor we could do where we carry to_bits: OperandSize and from_bits: UnitSize or similar, where UnitSize is an enum that has 8/16/32/64-bit cases. I don't think we have an enum for that yet. That makes the unsupported cases unrepresentable. Feel free to defer that if you'd rather not do deeper surgery though.)

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 03:31):

:memo: Rafferty97 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 03:31):

:speech_balloon: Rafferty97 created PR review comment:

I'm always in favour of making invalid inputs unrepresentable, so I'll add the UnitSize enum etc. and update the PR :)

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 04:53):

:speech_balloon: Rafferty97 created PR review comment:

I've gone further and replaced the signed and from_bits parameters with the ExtendOp enum that already exists, and this has had some nice upstream effects, simplifying some call sites.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 04:53):

:memo: Rafferty97 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 04:57):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 05:29):

Rafferty97 updated PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 20 2026 at 06:20):

Rafferty97 requested cfallin for a review on PR #14793.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 18:23):

:thumbs_up: cfallin submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 18:23):

cfallin added PR #14793 aarch64: remove MInst::Extend to the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 22:33):

cfallin removed PR #14793 aarch64: remove MInst::Extend from the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Oct 23 2026 at 22:33):

:check: cfallin merged PR #14793.

Last updated: Oct 31 2026 at 23:03 UTC