Stream: git-wasmtime

Topic: wasmtime / PR #1665 Add initial scalar FP operations (add...

Wasmtime GitHub notifications bot (May 14 2020 at 19:20):

jlb6740 edited PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (May 14 2020 at 22:01):

jlb6740 updated PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (May 15 2020 at 00:13):

jlb6740 updated PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (May 20 2020 at 20:03):

jlb6740 updated PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (May 27 2020 at 07:47):

julian-seward1 submitted PR Review.

Wasmtime GitHub notifications bot (May 27 2020 at 07:47):

julian-seward1 submitted PR Review.

Wasmtime GitHub notifications bot (May 27 2020 at 07:47):

julian-seward1 created PR Review Comment:

is_64 doesn't need to be here; pls rm (also for SSE_R_R)

Wasmtime GitHub notifications bot (May 27 2020 at 07:47):

julian-seward1 created PR Review Comment:

Need to also remove F_PREFIX_66 (or whatever it is called), no?

Wasmtime GitHub notifications bot (May 28 2020 at 02:26):

jlb6740 submitted PR Review.

Wasmtime GitHub notifications bot (May 28 2020 at 02:26):

jlb6740 created PR Review Comment:

I wanted to remove this but kept it around for functions like show_ireg_sized and show_rru_sized. After seeing what it looks like, I don't think it makes sense to create sse variants of basically the same logic. I will simply pass in size=8 for these functions as it seems that may have been the original intent anyway.

Wasmtime GitHub notifications bot (May 28 2020 at 03:45):

jlb6740 submitted PR Review.

Wasmtime GitHub notifications bot (May 28 2020 at 03:45):

jlb6740 created PR Review Comment:

This is one of the SIMD prefixes. We will need it at somepoint. Are you suggesting add it only when needed?

Wasmtime GitHub notifications bot (May 28 2020 at 22:57):

jlb6740 updated PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (May 28 2020 at 23:52):

jlb6740 updated PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (May 29 2020 at 21:36):

jlb6740 updated PR #1665 from vcode-add-movss-addss-subss to master:

• Need to better separate data structures for sse and regular regs

• Lowering is not right .. made sure all paths towards lowering are
  covered but still need to review what is there and correct

• Need to emit code. Currently have placeholders.

<!--

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 (Jun 05 2020 at 08:48):

bnjbvr requested julian-seward1 for a review on PR #1665.

Wasmtime GitHub notifications bot (Jun 07 2020 at 07:22):

julian-seward1 submitted PR Review.

Wasmtime GitHub notifications bot (Jun 07 2020 at 07:22):

julian-seward1 created PR Review Comment:

Oh, my mistake. I misread the diff -- I thought you hadn't deleted the definition of F_PREFIX_66, but you had. So then the patch is correct as it stands.

Wasmtime GitHub notifications bot (Jun 07 2020 at 07:28):

julian-seward1 submitted PR Review.

Wasmtime GitHub notifications bot (Jun 07 2020 at 07:28):

julian-seward1 created PR Review Comment:

An XMM reg shouldn't be passed to show_ireg_sized (at least, going from memory) -- that's for printing integer register names. Printing XMM reg names is different in that they are always 16 bytes and there are no names for smaller parts, unlike for the integer registers. Hence the desire not to have any 'size' qualifiers associated with storage or printing of XMM registers.

Wasmtime GitHub notifications bot (Jun 07 2020 at 07:39):

julian-seward1 submitted PR Review.

Wasmtime GitHub notifications bot (Jun 10 2020 at 12:41):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Jun 10 2020 at 12:41):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Jun 10 2020 at 12:41):

bnjbvr created PR Review Comment:

I think we'd match on the currently unused ty type variable, and if it's F32, use movss, etc.

Wasmtime GitHub notifications bot (Jun 10 2020 at 12:41):

bnjbvr created PR Review Comment:

Out of curiosity, could we avoid the mov here, by defining that the regD register is modified, in both get_regs and map_regs?

Wasmtime GitHub notifications bot (Jun 10 2020 at 12:41):

bnjbvr created PR Review Comment:

This whole expression could be simplified to vreg.get_index() + 1, right?

Wasmtime GitHub notifications bot (Jun 10 2020 at 12:41):

bnjbvr created PR Review Comment:

nit: second check is redundant here (we can address this in a separate PR)

Wasmtime GitHub notifications bot (Jun 10 2020 at 16:24):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Jun 10 2020 at 16:24):

bnjbvr created PR Review Comment:

My previous comment was wrong: we're operating at the vreg level here, so we are required to do this. We can teach regalloc to coalesce these moves, in the future, though.

Wasmtime GitHub notifications bot (Jun 10 2020 at 16:36):

bnjbvr merged PR #1665.

Last updated: Nov 22 2024 at 16:03 UTC