Stream: git-wasmtime

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

Wasmtime GitHub notifications bot (Jun 21 2021 at 18:09):

cfallin requested bnjbvr for a review on PR #3011.

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.

• [ ] 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 (Jun 21 2021 at 18:09):

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

Wasmtime GitHub notifications bot (Jun 21 2021 at 18:51):

akirilov-arm submitted PR review.

Wasmtime GitHub notifications bot (Jun 21 2021 at 18:51):

akirilov-arm submitted PR review.

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?

Wasmtime GitHub notifications bot (Jun 21 2021 at 19:09):

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

Wasmtime GitHub notifications bot (Jun 21 2021 at 19:09):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Jun 21 2021 at 19:09):

cfallin created PR review comment:

Done!

Wasmtime GitHub notifications bot (Jun 22 2021 at 15:41):

bnjbvr submitted PR review.

Wasmtime GitHub notifications bot (Jun 22 2021 at 15:41):

bnjbvr submitted PR review.

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?

Wasmtime GitHub notifications bot (Jun 22 2021 at 17:34):

akirilov-arm submitted PR review.

Wasmtime GitHub notifications bot (Jun 22 2021 at 17:57):

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

Wasmtime GitHub notifications bot (Jun 22 2021 at 17:57):

cfallin submitted PR review.

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!

Wasmtime GitHub notifications bot (Jun 22 2021 at 18:26):

cfallin merged PR #3011.

Last updated: Nov 22 2024 at 16:03 UTC