Wasmtime GitHub notifications bot (May 03 2022 at 16:31): Wasmtime GitHub notifications bot (May 03 2022 at 16:31):cfallin requested abrown for a review on PR #4094.
Wasmtime GitHub notifications bot (May 03 2022 at 16:31):cfallin opened PR #4094 from fix-xmm-pretty-printing-arg-order to main:
The pretty-printing had swapped dst and src2; this was introduced when
we moved to RA2 (sorry about that! IMHO we should do something to
automate the mapping between regalloc arg collection and pretty
printing/emission).
src2comes at the end because it has a variable number of register
mentions; this is in line with how many of the other inst formats work.Actual emitted code was never incorrect, just the pretty-printing.
Updated test golden outputs look correct to me now, including the one
that we saw was incorrect in #3945.<!--
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 (May 03 2022 at 16:34):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 03 2022 at 16:34):abrown submitted PR review.
Wasmtime GitHub notifications bot (May 03 2022 at 16:34):abrown created PR review comment:
Why does changing the order of this matter? The format string hasn't changed...
Wasmtime GitHub notifications bot (May 03 2022 at 16:34):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (May 03 2022 at 16:34):bjorn3 created PR review comment:
How does this statement reordering change the format output? The
format!()call is still identical.
Wasmtime GitHub notifications bot (May 03 2022 at 16:37):cfallin submitted PR review.
Wasmtime GitHub notifications bot (May 03 2022 at 16:37):cfallin created PR review comment:
allocsis anAllocationConsumer, so invocation order matters here; the order must match the order in which we provide the register arguments inget_operands. (Yep, it's pretty fragile, and I want to find a better way; this was a result of the "don't mutate instructions post-regalloc" design decision that was part of the RA2 speedup.)
Wasmtime GitHub notifications bot (May 03 2022 at 16:37):cfallin submitted PR review.
Wasmtime GitHub notifications bot (May 03 2022 at 16:37):cfallin created PR review comment:
@bjorn3 see reply to abrown's identical question above :-)
Wasmtime GitHub notifications bot (May 03 2022 at 17:12):cfallin merged PR #4094.
Last updated: Dec 23 2024 at 12:05 UTC