Wasmtime GitHub notifications bot (Jun 11 2020 at 05:53): Wasmtime GitHub notifications bot (Jun 11 2020 at 05:53):jlb6740 opened PR #1862 from vcode-add-store to master:
<!--
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 11 2020 at 06:53):bjorn3 submitted PR Review.
Wasmtime GitHub notifications bot (Jun 11 2020 at 06:53):bjorn3 submitted PR Review.
Wasmtime GitHub notifications bot (Jun 11 2020 at 06:53):bjorn3 created PR Review Comment:
ty.is_float()
Wasmtime GitHub notifications bot (Jun 11 2020 at 07:18):jlb6740 updated PR #1862 from vcode-add-store to master:
<!--
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 11 2020 at 07:20):jlb6740 submitted PR Review.
Wasmtime GitHub notifications bot (Jun 11 2020 at 07:20):jlb6740 created PR Review Comment:
Hi thanks. This was copied over from the Aarch64 for prep for some lowering that is not included in this patch. I will use your suggestion when needed.
Wasmtime GitHub notifications bot (Jun 11 2020 at 07:21):jlb6740 edited PR Review Comment.
Wasmtime GitHub notifications bot (Jun 11 2020 at 07:21):jlb6740 edited PR Review Comment.
Wasmtime GitHub notifications bot (Jun 17 2020 at 19:43):jlb6740 updated PR #1862 from vcode-add-store to master:
<!--
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 17 2020 at 20:29):jlb6740 updated PR #1862 from vcode-add-store to master:
<!--
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 22 2020 at 23:21):jlb6740 updated PR #1862 from vcode-add-store to master:
<!--
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 24 2020 at 21:38):jlb6740 updated PR #1862 from vcode-add-store to master:
<!--
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 25 2020 at 14:45):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):bnjbvr created PR Review Comment:
Should this comment be removed?
Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):bnjbvr created PR Review Comment:
Out of curiosity, is
movdbetter thanmovsdin this case?
Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):bnjbvr created PR Review Comment:
nit: TODO here
Wasmtime GitHub notifications bot (Jun 25 2020 at 18:48):alexcrichton edited PR #1862 from vcode-add-store 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 (Jun 26 2020 at 07:52):jlb6740 submitted PR Review.
Wasmtime GitHub notifications bot (Jun 26 2020 at 07:52):jlb6740 created PR Review Comment:
I was first implementing for movd (set in lower.rs) because this was a 32 bit move, else I'd prefer movsd over movq since movsd is intended for fp.
Wasmtime GitHub notifications bot (Jun 26 2020 at 07:52):jlb6740 edited PR Review Comment.
Wasmtime GitHub notifications bot (Jun 26 2020 at 07:54):jlb6740 submitted PR Review.
Wasmtime GitHub notifications bot (Jun 26 2020 at 07:54):jlb6740 created PR Review Comment:
Looks out of place. Will remove.
Wasmtime GitHub notifications bot (Jun 26 2020 at 21:41):jlb6740 updated PR #1862 from vcode-add-store 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 (Jun 26 2020 at 21:43):jlb6740 updated PR #1862 from vcode-add-store 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 (Jun 26 2020 at 22:02):jlb6740 updated PR #1862 from vcode-add-store 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 (Jun 26 2020 at 22:14):jlb6740 updated PR #1862 from vcode-add-store 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 (Jun 29 2020 at 04:20):jlb6740 requested bnjbvr for a review on PR #1862.
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):bnjbvr created PR Review Comment:
nit: can you keep it in the sections of vcode insts labelled with a "FP instructions" header, please? I tried to order the vcode instructions in logical groups, and it would be nice if we tried to enforce it.
(also nit: code comments are allowed to use 99 chars per line, as in the rest of the code base, so the wrapping around can be reverted too)
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):bnjbvr created PR Review Comment:
Right regarding 32 vs 64 bits; then my question is, is movd better than movss? movss can be encoded with F3 0F 11, so there's no difference in size; and we do use movss for reg to reg moves, and if I'm understanding correctly for f64 we'll use movsd for both RR and RM. I am not asking to change, but trying to understand; the only argument in favor of movss might be consistency with f64 here.
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):bnjbvr created PR Review Comment:
Can you add one test that uses one of the high registers requiring the REX prefix, please?
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:45):bnjbvr submitted PR Review.
Wasmtime GitHub notifications bot (Jun 30 2020 at 13:45):bnjbvr created PR Review Comment:
Actually, when looking at the output of llvm and gcc, both use movss in this case. Also Intel's optimization guide seems to suggest there might be an extra overhead in using movd for FP registers? (3.5.2.3 Bypass between Execution Domains)
So could we change it to use movss instead, please? (and ditto for loads)
Wasmtime GitHub notifications bot (Jul 01 2020 at 21:20):jlb6740 updated PR #1862 from vcode-add-store 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 01 2020 at 21:21):jlb6740 submitted PR Review.
Wasmtime GitHub notifications bot (Jul 01 2020 at 21:21):jlb6740 created PR Review Comment:
Right ... I've done this with a new test for Movss. Thanks for pushing on the movss :+1:, not sure what I was overlooking there.
Wasmtime GitHub notifications bot (Jul 01 2020 at 21:55):jlb6740 merged PR #1862.
Last updated: Oct 23 2024 at 20:03 UTC