Stream: git-wasmtime

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


view this post on Zulip 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.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Mar 31 2020 at 17:13):

abrown requested iximeow for a review on PR #1444.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 31 2020 at 17:13):

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

view this post on Zulip Wasmtime GitHub notifications bot (Apr 02 2020 at 18:46):

iximeow submitted PR Review.

view this post on Zulip 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.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip 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.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Apr 02 2020 at 23:50):

abrown merged PR #1444.


Last updated: Dec 23 2024 at 12:05 UTC