Wasmtime GitHub notifications bot (Apr 18 2023 at 21:59): Wasmtime GitHub notifications bot (Apr 18 2023 at 21:59):elliottt opened PR #6237 from elliottt:trevor/bump-regalloc2 to bytecodealliance:main:
- Bump RA2 to 0.7.0
- Certify the RA2 update
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 18 2023 at 22:15):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 18 2023 at 22:45):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 18 2023 at 22:56):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 18 2023 at 23:01):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 18 2023 at 23:03):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers. This bump also brings with it a new dependency on
hashbrown-0.13.2andrustc-hash. The latter had an audit that we could import, while the former is currently awaiting merge in #6238.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 18 2023 at 23:19):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:16):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:20):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:20):elliottt created PR review comment:
This is a little surprising, but also a totally reasonable consequence of no longer communicating program moves to RA2.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:25):elliottt has marked PR #6237 as ready for review.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:25):elliottt requested wasmtime-compiler-reviewers for a review on PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:25):elliottt requested abrown for a review on PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:25):elliottt requested wasmtime-default-reviewers for a review on PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 00:26):elliottt requested cfallin for a review on PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:14):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:15):elliottt created PR review comment:
This was easily resolved by copying the sret register in
Lower::new, instead of generating a temporary that we would move into later.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:17):elliottt created PR review comment:
This is a great example of something the redundant move eliminator would have fixed before.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:20):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:20):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:20):alexcrichton created PR review comment:
This may be similar to the s390x variant in that something riscv-specific may be needed?
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:20):alexcrichton created PR review comment:
I don't know much about the ra2 changes side of things, but if this wasn't already noted it might be worthwhile to look into this perhaps?
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:20):alexcrichton created PR review comment:
Random drive-by comment, but this looks like something s390x-specific may be required here?
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:33):jameysharp submitted PR review.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:33):jameysharp submitted PR review.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:33):jameysharp created PR review comment:
I'm hoping that whatever is causing
mv a0,a0in the riscv backend is easy to fix.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:33):jameysharp created PR review comment:
Here the VCode pretty-printing changed without changing the disassembly listing. The same instruction was originally printed as
mv a0,a1and now is printed asmv a1,a0, but in both cases it's disassembled asori a0, a1, 0. Any idea what's going on there? I don't see any Rust-side changes that explain this. There are more cases like this below.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:33):jameysharp created PR review comment:
It's neat that this is actually removing moves in some cases compared to the older RA2!
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:33):jameysharp created PR review comment:
Similarly, I hope whatever is causing
lgr %r2, %r2in s390x is easy to fix.
Wasmtime GitHub notifications bot (Apr 19 2023 at 16:44):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 19 2023 at 17:03):elliottt created PR review comment:
That's actually due to my change to
sretregister handling :sweat_smile:I'll land it as a separate PR once I've resolved all of the new move issues.
Wasmtime GitHub notifications bot (Apr 19 2023 at 22:03):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 16:35):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 16:56):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 17:03):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 17:18):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 17:21):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 18:12):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers. This bump also brings with it a new dependency on
hashbrown-0.13.2andrustc-hash. The latter had an audit that we could import, while the former is currently awaiting merge in #6238.This currently has commits from #6245, #6253, and #6252.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 20 2023 at 19:12):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers. This bump also brings with it a new dependency on
hashbrown-0.13.2andrustc-hash. The latter had an audit that we could import, while the former is currently awaiting merge in #6238.This currently has commits from #6253.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 20 2023 at 19:13):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 19:14):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers. This bump also brings with it a new dependency on
hashbrown-0.13.2andrustc-hash. The latter had an audit that we could import, while the former was udpated in #6238.This currently has commits from #6253.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 20 2023 at 19:14):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers. This bump also brings with it a new dependency on
hashbrown-0.13.2andrustc-hash. The latter had an audit that we could import, while the former was updated in #6238.This currently has commits from #6253.
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 20 2023 at 22:46):elliottt edited PR #6237:
Bump regalloc2 to version 0.7.0, which removes program moves and adds back in support for scratch registers. This bump also brings with it a new dependency on
hashbrown-0.13.2andrustc-hash. The latter had an audit that we could import, while the former was updated in #6238.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 20 2023 at 22:47):elliottt updated PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 23:19):elliottt created PR review comment:
This pattern shows up across all backends. We insert a copy of the ref-typed values before calling a function with them as an argument, as the values themselves will be constrained to live on the stack.
https://github.com/bytecodealliance/wasmtime/blob/e6339b27254d9e9c4c2fe7bdf1a3a06589c74847/cranelift/codegen/src/machinst/abi.rs#L2243-L2253
With program moves no longer being special for RA2, the move that's inserted by cranelift on line 95 doesn't end up being redundant with the one that RA2 inserts. The store to the stack can use that value without any issue, however we still need to copy the value back tox0to pass as the first argument tofn0. Supporting overlap in RA2 will help a lot here, as the value will be allowed to exist on the stack and in a register at the same time, giving us a path forward for skipping the initial copy when setting up the arg.
Wasmtime GitHub notifications bot (Apr 20 2023 at 23:19):elliottt has marked PR #6237 as ready for review.
Wasmtime GitHub notifications bot (Apr 20 2023 at 23:20):elliottt requested alexcrichton for a review on PR #6237.
Wasmtime GitHub notifications bot (Apr 20 2023 at 23:20):elliottt requested jameysharp for a review on PR #6237.
Wasmtime GitHub notifications bot (Apr 21 2023 at 00:04):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Apr 21 2023 at 00:06):jameysharp submitted PR review:
Ooh, I wonder if this will work for me now:
/bench_x64
Wasmtime GitHub notifications bot (Apr 21 2023 at 00:13):cfallin submitted PR review:
Updates look good now, diffs are minimal and well-understood -- thanks for the diligence here!
Wasmtime GitHub notifications bot (Apr 21 2023 at 01:31):elliottt merged PR #6237.
Last updated: Nov 22 2024 at 16:03 UTC