Wasmtime GitHub notifications bot (Jan 11 2022 at 19:50): Wasmtime GitHub notifications bot (Jan 11 2022 at 19:50):fitzgen opened PR #3681 from port-sshr-to-isle to main:
<!--
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 (Jan 11 2022 at 19:50):fitzgen requested alexcrichton for a review on PR #3681.
Wasmtime GitHub notifications bot (Jan 11 2022 at 20:04):fitzgen updated PR #3681 from port-sshr-to-isle to main.
Wasmtime GitHub notifications bot (Jan 11 2022 at 21:09):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 21:09):alexcrichton created PR review comment:
I think that
imm8_from_valueisn't the right thing here in the sense that all constants work with this instruction, Cranelift's semantics are just to mask off everything except the low bits. (or at least I think that's what this previously did)
Wasmtime GitHub notifications bot (Jan 11 2022 at 21:09):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 21:09):alexcrichton created PR review comment:
Similar to above I think that
put_in_imm8_regisn't what we want here in the sense that it doesn't work for constants bigger than 8 bits, but for those constants we can mask off the upper bits?
Wasmtime GitHub notifications bot (Jan 11 2022 at 21:09):alexcrichton created PR review comment:
Should this add be masked with 0xf? (I realize the previous code didn't do this but is that a bug?)
Wasmtime GitHub notifications bot (Jan 11 2022 at 22:20):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 22:20):fitzgen created PR review comment:
x64 will automatically mask it for us: https://www.felixcloutier.com/x86/psraw:psrad:psraq#psraw--with-64-bit-operand-
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:01):fitzgen updated PR #3681 from port-sshr-to-isle to main.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:01):fitzgen requested alexcrichton for a review on PR #3681.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:10):fitzgen updated PR #3681 from port-sshr-to-isle to main.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:12):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:12):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:12):alexcrichton created PR review comment:
Oh sorry I meant to ask earlier, but what's this doing?
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:12):alexcrichton created PR review comment:
The register allocation here seems kinda poort where the
movdqahere isn't present in the original code, and there's also an extramovdqaafter thepsraw. Would reordering the instructions in isle help at all?I dunno how to keep track of "well regalloc2 will probably fix that" style things.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:12):alexcrichton created PR review comment:
IIRC we there's usage of
(def_inst (iconst N))in other places, should that be used here or should other places be switched to this?
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:21):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:21):fitzgen created PR review comment:
Oh this is just bumping the time limit for each individual test. If I run with
RUST_LOG=tracethen I was running into the time limit for run tests.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:22):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:22):fitzgen created PR review comment:
I can poke at reordering the instructions, but I also think it isn't the biggest deal. We have a few tests that get a couple extra moves and also a few tests where we now elide a couple moves that we used to make. Overall I think it is pretty much a wash.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:37):fitzgen updated PR #3681 from port-sshr-to-isle to main.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:37):fitzgen updated PR #3681 from port-sshr-to-isle to main.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:39):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:39):fitzgen created PR review comment:
Managed to get it down to 12 instructions, instead of 13. Still one additional move compared to before. meh.
Wasmtime GitHub notifications bot (Jan 11 2022 at 23:40):fitzgen requested alexcrichton for a review on PR #3681.
Wasmtime GitHub notifications bot (Jan 12 2022 at 15:13):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Jan 12 2022 at 15:13):alexcrichton merged PR #3681.
Last updated: Dec 23 2024 at 12:05 UTC