Wasmtime GitHub notifications bot (Jan 14 2021 at 02:47): Wasmtime GitHub notifications bot (Jan 14 2021 at 02:47):jlb6740 edited PR #2581 from fix-simd-flt-nan-bug-for-2432 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 (Jan 14 2021 at 03:01):jlb6740 updated PR #2581 from fix-simd-flt-nan-bug-for-2432 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 (Jan 14 2021 at 03:18):cfallin submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 03:44):jlb6740 merged PR #2581.
Wasmtime GitHub notifications bot (Jan 14 2021 at 16:56):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 16:56):abrown created PR Review Comment:
Looks like this should have been
FloatCC::UnorderedOrGreaterThanOrEqualwhich usesFcmpImm::UnorderedOrGreaterThanOrEqualto encode to the0x05immediate. Using that might avoid the extraXORPS.
Wasmtime GitHub notifications bot (Jan 14 2021 at 16:59):abrown created PR Review Comment:
In the old backend we were using
PCMPEQBto generate the "all ones vector" so this wasn't a problem, but I think in this new backend we should check around and make sure no more of the "all ones vectors" we create will have this same issue. Before Christmas I was working on a PR to simplify constant emission so that we only have one place to check... looks like I should have finished that.
Wasmtime GitHub notifications bot (Jan 14 2021 at 16:59):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 17:55):cfallin submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 17:55):cfallin created PR Review Comment:
Yes, my understanding (after some more reading) is that the i8x16 (pcmpeqb) variant is more canonical; would prefer that over reasoning about FP comparison edge cases. +1 to centralizing this logic!
Wasmtime GitHub notifications bot (Jan 14 2021 at 18:01):jlb6740 submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 18:01):jlb6740 created PR Review Comment:
UnorderedOrGreaterThanOrEqual
Yes, I suppose any unordered instruction that also checks for equality should work. Just depends on what that actually lowers to and the supported CPU feature. That encoding to 0x05 ( NLT_US) looks good except it signals. Not really sure what that means for cranelift .. maybe @cfallin @abrown you have comment. Perhaps instead we'd want to encode as 0x15 or NLT_UQ which does not signal.
Wasmtime GitHub notifications bot (Jan 14 2021 at 18:01):jlb6740 submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 18:01):jlb6740 created PR Review Comment:
Wasmtime GitHub notifications bot (Jan 14 2021 at 18:47):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Jan 14 2021 at 18:47):abrown created PR Review Comment:
That does look better :+1:
Last updated: Oct 23 2024 at 20:03 UTC