Stream: git-wasmtime

Topic: wasmtime / PR #3011 Fix `bint` on x64, and make `bextend`...


view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 18:09):

cfallin requested bnjbvr for a review on PR #3011.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 18:09):

cfallin opened PR #3011 from bint-x64 to main:

There has been occasional confusion with the representation that we use
for bool-typed values in registers, at least when these are wider than
one bit. Does a b8 store true as 1, or as all-ones (0xff)?

We've settled on the latter because of some use-cases where the wide
bool becomes a mask -- see #2058 for more on this.

This is fine, and transparent, to most operations within CLIF, because
the bool-typed value still has only two semantically-visible states,
namely true and false.

However, we have to be careful with bool-to-int conversions. bint on
aarch64 correctly masked the all-ones value down to 0 or 1, as required
by the instruction specification, but on x64 it did not. This PR fixes
that bug and makes x64 consistent with aarch64.

While staring at this code I realized that bextend was also not
consistent with the all-ones invariant: it should do a sign-extend, not
a zero-extend as it previously did. This is also rectified and tested.
(Aarch64 also already had this case implemented correctly.)

Fixes #3003.

<!--

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 21 2021 at 18:09):

cfallin requested akirilov-arm for a review on PR #3011.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 18:51):

akirilov-arm submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 18:51):

akirilov-arm submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 18:51):

akirilov-arm created PR review comment:

For the sake of completeness, shouldn't we also have tests for false?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 19:09):

cfallin updated PR #3011 from bint-x64 to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 19:09):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 21 2021 at 19:09):

cfallin created PR review comment:

Done!

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 15:41):

bnjbvr submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 15:41):

bnjbvr submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 15:41):

bnjbvr created PR review comment:

Can you use an immediate operand here for the constant 1, instead?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 17:34):

akirilov-arm submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 17:57):

cfallin updated PR #3011 from bint-x64 to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 17:57):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 17:57):

cfallin created PR review comment:

Ah, yeah, dunno why I did it this way instead -- fixed!

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2021 at 18:26):

cfallin merged PR #3011.


Last updated: Dec 23 2024 at 12:05 UTC