alexcrichton opened PR #9211 from alexcrichton:rm-mulhi
to bytecodealliance:main
:
Instead of supporting these instructions natively, which for example most source languages don't have direct support for (nor wasm), pattern-match these instructions during lowering. This commit replaces backend-specific lowerings of
umulhi
andsmulhi
with patterns in each backend in respective places. This helps detect when this happens from WebAssembly for example and helps simplify producers as well. The intent of this commit is that when desired the same instructions are emitted as before.The downside of this commit is that CLIF is losing the ability to implement
umulhi
andsmulhi
on vector types. This was only implemented in the riscv64 and s390x backends and was not used by WebAssembly for example.<!--
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
-->
alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9211.
alexcrichton requested pchickey for a review on PR #9211.
alexcrichton requested wasmtime-core-reviewers for a review on PR #9211.
alexcrichton requested cfallin for a review on PR #9211.
alexcrichton commented on PR #9211:
The main motivation for this is that in thinking about https://github.com/WebAssembly/128-bit-arithmetic I'm hoping to be able to add a "multiply wide" instruction for 64x64->128 but also wanted to retain the ability to generate just
umulh
on aarch64 for example. Without adding new opcodes to wasm that requires pattern matching at the underlying lowering level anyway, so my hope was to go ahead and start moving in that direction.
Cg_clif uses these instructions: https://github.com/rust-lang/rustc_codegen_cranelift/blob/7e4cafb653980b4dc1ca95eb67df8ac4608eab61/src/num.rs#L270
alexcrichton commented on PR #9211:
Thanks for pointing that out! While it should still be possible to get the same performance/semantics with this PR, the more I think about this the more I've started to sour on the idea. One of the other main motivations for this PR was that the ISLE optimization today to pattern-match an open-coded
umulhi
doesn't work for i128 whenisplit
is used to acquire the top bits instead of a shift-then-reduce. It might be best to work on that instead and keep these instructions as-is for produces to use and from wasm it'd mostly be the ISLE optimizations that create the instructions.
alexcrichton closed without merge PR #9211.
alexcrichton commented on PR #9211:
Ok I'm going to close this in favor of https://github.com/bytecodealliance/wasmtime/pull/9215 since I now think that's the better approach.
Last updated: Nov 22 2024 at 17:03 UTC