Stream: git-wasmtime

Topic: wasmtime / PR #2291 Adds support for signed packed intege...

Wasmtime GitHub notifications bot (Oct 15 2020 at 00:50):

jlb6740 opened PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 15 2020 at 00:54):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 15 2020 at 00:56):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 15 2020 at 00:57):

jlb6740 submitted PR Review.

Wasmtime GitHub notifications bot (Oct 15 2020 at 00:57):

jlb6740 created PR Review Comment:

These white spaces are curious. I do not see them indicated on the file I push?

Wasmtime GitHub notifications bot (Oct 15 2020 at 00:57):

jlb6740 edited PR Review Comment.

Wasmtime GitHub notifications bot (Oct 15 2020 at 18:28):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 15 2020 at 23:10):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 15 2020 at 23:15):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 15 2020 at 23:28):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 16 2020 at 00:15):

jlb6740 requested bnjbvr for a review on PR #2291.

Wasmtime GitHub notifications bot (Oct 16 2020 at 00:15):

jlb6740 requested cfallin for a review on PR #2291.

Wasmtime GitHub notifications bot (Oct 16 2020 at 01:31):

jlb6740 requested cfallin, abrown and bnjbvr for a review on PR #2291.

Wasmtime GitHub notifications bot (Oct 16 2020 at 01:31):

jlb6740 requested cfallin, abrown and bnjbvr for a review on PR #2291.

Wasmtime GitHub notifications bot (Oct 16 2020 at 17:46):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Oct 16 2020 at 17:46):

abrown submitted PR Review.

Wasmtime GitHub notifications bot (Oct 16 2020 at 17:46):

abrown created PR Review Comment:

                    _ => unimplemented!("unable to use type {} for op {}", ctx.input_ty(insn, 0), op),

My reasoning: more information is usually better with these errors and fcvt_from_sint can accept more than the I32X4 type (so other types are unimplemented, not unreachable).

Wasmtime GitHub notifications bot (Oct 16 2020 at 17:46):

abrown created PR Review Comment:

It's fine: rustfmt probably bumped them in a bit since they are now nested inside if !output_ty.is_vector().

Wasmtime GitHub notifications bot (Oct 16 2020 at 19:56):

jlb6740 submitted PR Review.

Wasmtime GitHub notifications bot (Oct 16 2020 at 19:56):

jlb6740 created PR Review Comment:

@abrown Yes .. agree to more information. The unreachable vs unimplemented I um .. kind of have a different take. Right now, until cliff instructions are added, it is unreachable (I think?). There is no clif instruction (I think?) that can get here .. as this code is in a branch that has already checked for a vector type. Taken in context, the unreachable doesn't refer to just fcvt_from_sint it refers to vector types of fcvt_from_sint in which case there is only one afaik? I will change it though because taken out of context it is reachable and in any case I think with this kind of ambiguity, for convenience power goes to the who reviews :-) .. and of course precedence/consistency. :+1:

Wasmtime GitHub notifications bot (Oct 16 2020 at 20:37):

jlb6740 updated PR #2291 from packed-convert to main:

f32x4.convert_i32x4_s

<!--

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.
-->

Wasmtime GitHub notifications bot (Oct 16 2020 at 21:16):

jlb6740 merged PR #2291.

Wasmtime GitHub notifications bot (Oct 16 2020 at 21:48):

abrown created PR Review Comment:

it refers to vector types of fcvt_from_sint in which case there is only one afaik

Take a look at the types of the fcvt_from_sint instruction I linked to: x is Int, which makes that CLIF instruction polymorphic over all integer and lane sizes. So someone could theoretically write a CLIF instruction with a type that would trigger the error, even if the Wasm translation would never create it.

Wasmtime GitHub notifications bot (Oct 16 2020 at 21:48):

abrown submitted PR Review.

Last updated: Nov 22 2024 at 16:03 UTC