Stream: git-wasmtime

Topic: wasmtime / PR #1444 Avoid `infer_rex` and `w` on the same...

Wasmtime GitHub notifications bot (Mar 31 2020 at 17:12):

abrown opened PR #1444 from fix-1342 to master:

This relates to #1342; in #1335, @bnjbvr and I discussed that we should disallow code like infer_rex().w() in the meta crate since the presence of a REX.W by necessity requires a REX prefix. To avoid ambiguity, we now require rex().w() instead.

Though all the tests pass, the tricky part of this PR is deciding whether the subsequent two commits, the "Skip extra work..." optimizations, are always correct. What these should do is avoid checks on the REX.W bit when we are dealing with a recipe that has inferred REX prefixes; since infer_rex() and w() can no longer go together, the thought is that we could avoid some (negligible) extra work checking for this bit when determining if a REX prefix is needed--i.e. we only need to check the RXB bits. I am open to removing these commits or adding tests (but which ones?) to prove this is correct since I am rather wary that the "we are dealing with a recipe that has inferred REX prefixes" assumption always holds.

I'll tag multiple people as reviewers for extra eyes.

<!--

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 (Mar 31 2020 at 17:13):

abrown requested iximeow for a review on PR #1444.

Wasmtime GitHub notifications bot (Mar 31 2020 at 17:13):

abrown requested bnjbvr and iximeow for a review on PR #1444.

Wasmtime GitHub notifications bot (Apr 02 2020 at 18:46):

iximeow submitted PR Review.

Wasmtime GitHub notifications bot (Apr 02 2020 at 21:37):

abrown updated PR #1444 from fix-1342 to master:

This relates to #1342; in #1335, @bnjbvr and I discussed that we should disallow code like infer_rex().w() in the meta crate since the presence of a REX.W by necessity requires a REX prefix. To avoid ambiguity, we now require rex().w() instead.

Though all the tests pass, the tricky part of this PR is deciding whether the subsequent two commits, the "Skip extra work..." optimizations, are always correct. What these should do is avoid checks on the REX.W bit when we are dealing with a recipe that has inferred REX prefixes; since infer_rex() and w() can no longer go together, the thought is that we could avoid some (negligible) extra work checking for this bit when determining if a REX prefix is needed--i.e. we only need to check the RXB bits. I am open to removing these commits or adding tests (but which ones?) to prove this is correct since I am rather wary that the "we are dealing with a recipe that has inferred REX prefixes" assumption always holds.

I'll tag multiple people as reviewers for extra eyes.

<!--

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 (Apr 02 2020 at 23:20):

abrown updated PR #1444 from fix-1342 to master:

This relates to #1342; in #1335, @bnjbvr and I discussed that we should disallow code like infer_rex().w() in the meta crate since the presence of a REX.W by necessity requires a REX prefix. To avoid ambiguity, we now require rex().w() instead.

Though all the tests pass, the tricky part of this PR is deciding whether the subsequent two commits, the "Skip extra work..." optimizations, are always correct. What these should do is avoid checks on the REX.W bit when we are dealing with a recipe that has inferred REX prefixes; since infer_rex() and w() can no longer go together, the thought is that we could avoid some (negligible) extra work checking for this bit when determining if a REX prefix is needed--i.e. we only need to check the RXB bits. I am open to removing these commits or adding tests (but which ones?) to prove this is correct since I am rather wary that the "we are dealing with a recipe that has inferred REX prefixes" assumption always holds.

I'll tag multiple people as reviewers for extra eyes.

<!--

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 (Apr 02 2020 at 23:50):

abrown merged PR #1444.

Last updated: Nov 22 2024 at 16:03 UTC