Stream: git-wasmtime

Topic: wasmtime / PR #4819 Port `fcmp` to ISLE (AArch64)

Wasmtime GitHub notifications bot (Aug 30 2022 at 12:21):

dheaton-arm opened PR #4819 from isle-fcmp to main:

Ported the existing implementation of fcmp for AArch64 to ISLE.

This also ports the lower_vector_comparison method to ISLE.

Copyright (c) 2022 Arm Limited

<!--

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 (Aug 30 2022 at 16:06):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:06):

cfallin created PR review comment:

This change switches from an output of all-ones to just 1 for true -- I guess that's OK, since the return type is b1? We have a separate TODO to check over our bool representation consistency anyway so I guess this is fine for now.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:06):

cfallin created PR review comment:

Extra test for fcvt_low_from_sint here -- any particular reason to add it? I guess it's always fine to add more tests, but curious if this came from a particular issue you found...

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:06):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:06):

cfallin merged PR #4819.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:09):

dheaton-arm submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:09):

dheaton-arm created PR review comment:

Yes - materialize_bool_result returns all 1s/0s for the length of the output type - so this happens because the return type is b1.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:10):

dheaton-arm submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:10):

dheaton-arm created PR review comment:

Ah, no, don't worry :) This was for working on fcvt_low_from_sint myself, but you've now done that.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:29):

akirilov-arm submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:29):

akirilov-arm created PR review comment:

@cfallin This used to be a harmless mistake in the previous implementation of the vany_true operation, which checked the input type to determine the width of the output boolean.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:30):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 30 2022 at 16:30):

cfallin created PR review comment:

Makes sense, thanks!

Last updated: Oct 23 2024 at 20:03 UTC