Stream: git-wasmtime

Topic: wasmtime / PR #1862 Vcode add store


view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2020 at 06:53):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2020 at 06:53):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2020 at 06:53):

bjorn3 created PR Review Comment:

ty.is_float()

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2020 at 07:20):

jlb6740 submitted PR Review.

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2020 at 07:21):

jlb6740 edited PR Review Comment.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 11 2020 at 07:21):

jlb6740 edited PR Review Comment.

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):

bnjbvr submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):

bnjbvr created PR Review Comment:

Should this comment be removed?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):

bnjbvr created PR Review Comment:

Out of curiosity, is movd better than movsd in this case?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):

bnjbvr submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 25 2020 at 14:45):

bnjbvr created PR Review Comment:

nit: TODO here

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 26 2020 at 07:52):

jlb6740 submitted PR Review.

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 26 2020 at 07:52):

jlb6740 edited PR Review Comment.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 26 2020 at 07:54):

jlb6740 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 26 2020 at 07:54):

jlb6740 created PR Review Comment:

Looks out of place. Will remove.

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Jun 29 2020 at 04:20):

jlb6740 requested bnjbvr for a review on PR #1862.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):

bnjbvr submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 30 2020 at 13:19):

bnjbvr submitted PR Review.

view this post on Zulip 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)

view this post on Zulip 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.

view this post on Zulip 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?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 30 2020 at 13:45):

bnjbvr submitted PR Review.

view this post on Zulip 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)

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Jul 01 2020 at 21:21):

jlb6740 submitted PR Review.

view this post on Zulip 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.

view this post on Zulip Wasmtime GitHub notifications bot (Jul 01 2020 at 21:55):

jlb6740 merged PR #1862.


Last updated: Dec 23 2024 at 13:07 UTC