Stream: git-wasmtime

Topic: wasmtime / PR #3105 Add support for Saturating Rounding Q...

Wasmtime GitHub notifications bot (Jul 21 2021 at 18:25):

jlb6740 opened PR #3105 from implement-sat-int-q-round-mul 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.
-->

Wasmtime GitHub notifications bot (Jul 21 2021 at 18:30):

jlb6740 updated PR #3105 from implement-sat-int-q-round-mul to main.

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:01):

jlb6740 updated PR #3105 from implement-sat-int-q-round-mul to main.

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:02):

jlb6740 requested abrown for a review on PR #3105.

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:02):

jlb6740 requested cfallin for a review on PR #3105.

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:06):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:06):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:06):

cfallin created PR review comment:

s/optimial/optimal/

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:06):

cfallin created PR review comment:

Why was movdqa removed here?

Wasmtime GitHub notifications bot (Jul 26 2021 at 21:06):

cfallin created PR review comment:

Could you remove the added space here?

Wasmtime GitHub notifications bot (Jul 27 2021 at 02:56):

jlb6740 submitted PR review.

Wasmtime GitHub notifications bot (Jul 27 2021 at 02:56):

jlb6740 created PR review comment:

@cfallin I am actually a little confused here because I thought I introduced this during the same patch only to remove it during an amend. In any case I removed it because I questioned the correct encoding. I avoided using the instruction directly and just used the gen_move call.

About my concern on the encoding, specifically the encodings are:

66 0F 6F /r | MOVDQA xmm1, xmm2/m128

or

66 0F 7F /r | MOVDQA xmm2/m128, xmm1

I think the former should be used, not the later. I also noticed that Movdqu also uses the later. This frankly was a todo to understand but in the moment I just used the gen_move as I noted above.

Wasmtime GitHub notifications bot (Jul 27 2021 at 02:58):

jlb6740 updated PR #3105 from implement-sat-int-q-round-mul to main.

Wasmtime GitHub notifications bot (Jul 27 2021 at 02:59):

jlb6740 submitted PR review.

Wasmtime GitHub notifications bot (Jul 27 2021 at 02:59):

jlb6740 created PR review comment:

Given the approval I'll push this assuming your fine with it still being removed.

Wasmtime GitHub notifications bot (Jul 27 2021 at 03:12):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jul 27 2021 at 03:12):

cfallin created PR review comment:

OK, makes sense -- in any case it's clear it's not used since the thing still compiles and works in CI!

Wasmtime GitHub notifications bot (Jul 27 2021 at 03:32):

jlb6740 merged PR #3105.

Last updated: Dec 23 2024 at 12:05 UTC