Stream: git-wasmtime

Topic: wasmtime / PR #5465 Remove vconcat and vsplit clif instru...

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:30):

MozarellaMan opened PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

This change is for #5463.

These instructions are not currently implemented in any cranelift backend and as the discussion in the issue clarified were only necessary for Cranelift's legalisation pass.

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:36):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:42):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:44):

MozarellaMan edited PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main:

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [x] This has been discussed in issue #..., or if not, please tell us why
  here.

• [x] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

This change is for #5463.

These instructions are not currently implemented in any cranelift backend and as the discussion in the issue clarified were only necessary for Cranelift's legalisation pass. Also removed the double_vector and half_vector helper functions as it seems they were only used for generating the docs and code for vsplit and vconcat

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:49):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:49):

bjorn3 created PR review comment:

Should we also remove them from the Type type?

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:57):

MozarellaMan submitted PR review.

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:57):

MozarellaMan created PR review comment:

Looks like they are used in fn image(&self, derived_func: DerivedFunc) -> TypeSet on L#385. I'll admit, I'm not sure about the description of the image function or what it does

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:57):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 17 2022 at 19:58):

MozarellaMan edited PR review comment.

Wasmtime GitHub notifications bot (Dec 17 2022 at 20:00):

bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Dec 17 2022 at 20:00):

bjorn3 created PR review comment:

It is fine to keep it then.

Wasmtime GitHub notifications bot (Dec 19 2022 at 16:52):

jameysharp created PR review comment:

I'm not 100% sure here, but I think you were right the first time, @bjorn3. Although TypeSet::half_vector and TypeSet::double_vector are still convenient for the implementation of DerivedFunc::MergeLanes and DerivedFunc::SplitLanes, the Type::half_vector method in cranelift/codegen/src/ir/types.rs doesn't seem to be used anywhere after this so I think it can be removed.

Wasmtime GitHub notifications bot (Dec 19 2022 at 16:52):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:27):

MozarellaMan created PR review comment:

Ah I see what bjorn meant now, makes sense. I'll just remove the method and inline its contents in Type::merge_lanes

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:27):

MozarellaMan submitted PR review.

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:30):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:37):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:47):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:53):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 19 2022 at 17:59):

jameysharp has enabled auto merge for PR #5465.

Wasmtime GitHub notifications bot (Dec 19 2022 at 18:38):

jameysharp has disabled auto merge for PR #5465.

Wasmtime GitHub notifications bot (Dec 19 2022 at 18:38):

MozarellaMan updated PR #5465 from remove-vconcat-and-vsplit-clif-instructions to main.

Wasmtime GitHub notifications bot (Dec 19 2022 at 19:32):

jameysharp has enabled auto merge for PR #5465.

Wasmtime GitHub notifications bot (Dec 20 2022 at 00:41):

jameysharp merged PR #5465.

Last updated: Dec 23 2024 at 12:05 UTC