Stream: git-wasmtime

Topic: wasmtime / issue #3976 cranelift: remove `load_complex` a...


view this post on Zulip Wasmtime GitHub notifications bot (Mar 30 2022 at 23:29):

abrown commented on issue #3976:

cc: @bjorn3, @bnjbvr, @sunfishcode (people I think who may have used these instructions at some point): this PR came out of a discussion with @cfallin as I was thinking about porting the x64 memory access instructions to ISLE. He suggested we try removing these *_complex variants and I thought that seemed a good idea--less complexity! Please let me know if you have seen uses of these in the wild or if the rationale for their original inclusion is something other than the one I came up with in the description.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 30 2022 at 23:30):

abrown commented on issue #3976:

cc: @alexcrichton, this is a significant change to the CLIF surface; if merged, we may want to think carefully about how this is versioned/released/communicated/etc.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 30 2022 at 23:47):

github-actions[bot] commented on issue #3976:

Subscribe to Label Action

cc @cfallin, @fitzgen

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

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 (Mar 30 2022 at 23:51):

cfallin commented on issue #3976:

I think this makes a lot of sense; as Andrew and I discussed, these opcodes are unused at least in the Wasm frontend, and a quick grep through cg\_clif appears to show load_complex and store_complex are also unused there (@bjorn3 please correct us if wrong). Given no known users, and complexity in handling the variadic arguments in pattern-matching, and the non-orthogonality (the operation can be expressed as iadds to compute the address, then a simple load/store), it seems reasonable to make this refactor. But anyone please let us know if there is a good reason not to!

view this post on Zulip Wasmtime GitHub notifications bot (Mar 30 2022 at 23:53):

cfallin commented on issue #3976:

Also, re: CLIF and communicating changes: we don't make any stability guarantees about the CLIF opcode set between releases (yet -- this will change when we get closer to thinking about a 1.0 release I suppose). So I don't think we need to do anything other than note the change in release notes. (Ensuring that no known users will be inconvenienced is a good step that we usually like to take though, if we can!)

view this post on Zulip Wasmtime GitHub notifications bot (Mar 31 2022 at 04:36):

bjorn3 commented on issue #3976:

Cg_clif indeed doesn't use load_complex and store_complex.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 04 2022 at 16:15):

bnjbvr commented on issue #3976:

Retroactive +1 to this change! This whole family of instructions existed because ISLE didn't.


Last updated: Nov 22 2024 at 17:03 UTC