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.
-->
cfallin submitted PR review.
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.
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...
cfallin submitted PR review.
cfallin merged PR #4819.
dheaton-arm submitted PR review.
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 isb1
.
dheaton-arm submitted PR review.
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.
akirilov-arm submitted PR review.
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.
cfallin submitted PR review.
cfallin created PR review comment:
Makes sense, thanks!
Last updated: Dec 23 2024 at 12:05 UTC