Stream: git-wasmtime

Topic: wasmtime / PR #6982 winch: Support f32.abs and f64.abs on...

Wasmtime GitHub notifications bot (Sep 08 2023 at 22:09):

elliottt opened PR #6982 from elliottt:trevor/winch-f32abs to bytecodealliance:main (assigned to saulecabrera):

Adds support for f32.abs and f64.abs to winch. Additionally, this adds a gpr_to_xmm function to the x64 backend to allow loading a constant and moving it to an xmm register. It also reserves xmm15 as the scratch xmm register.

<!--
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.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Sep 08 2023 at 22:09):

elliottt assigned PR #6982 to saulecabrera.

Wasmtime GitHub notifications bot (Sep 08 2023 at 22:09):

elliottt requested fitzgen for a review on PR #6982.

Wasmtime GitHub notifications bot (Sep 08 2023 at 22:09):

elliottt requested wasmtime-compiler-reviewers for a review on PR #6982.

Wasmtime GitHub notifications bot (Sep 08 2023 at 22:17):

elliottt unassigned PR #6982.

Wasmtime GitHub notifications bot (Sep 08 2023 at 22:17):

elliottt requested saulecabrera for a review on PR #6982.

Wasmtime GitHub notifications bot (Sep 08 2023 at 23:18):

saulecabrera submitted PR review:

Generally LGTM, thanks! A couple of minor comments:

• Spec tests: we could copy the <f32|f64>.abs test cases from tests/spec_testsuite/<f32|f64>_bitwise.wast into tests/misc_testsuite/winch/<f32|f64>_bitwise.wast to get test for these instructions additional to the filetests. This process is a bit manual at the moment, unfortunately, but the idea is that once Winch reaches support for Core Wasm this process should go away almost entirely.
• Add f32.abs and f64.abs to the list of supported instructions to the differential fuzzer in https://github.com/bytecodealliance/wasmtime/blob/main/fuzz/fuzz_targets/differential.rs#L289 (I run the differential fuzzer locally from time to time. The idea is to enable fuzzing by default once Winch reaches parity with Core Wasm).

Wasmtime GitHub notifications bot (Sep 08 2023 at 23:18):

saulecabrera submitted PR review:

Generally LGTM, thanks! A couple of minor comments:

• Spec tests: we could copy the <f32|f64>.abs test cases from tests/spec_testsuite/<f32|f64>_bitwise.wast into tests/misc_testsuite/winch/<f32|f64>_bitwise.wast to get test for these instructions additional to the filetests. This process is a bit manual at the moment, unfortunately, but the idea is that once Winch reaches support for Core Wasm this process should go away almost entirely.
• Add f32.abs and f64.abs to the list of supported instructions to the differential fuzzer in https://github.com/bytecodealliance/wasmtime/blob/main/fuzz/fuzz_targets/differential.rs#L289 (I run the differential fuzzer locally from time to time. The idea is to enable fuzzing by default once Winch reaches parity with Core Wasm).

Wasmtime GitHub notifications bot (Sep 08 2023 at 23:18):

saulecabrera created PR review comment:

winch/codegen/src/isa/reg.rs re-exports RegClass so you could pull it from there instead. This is something minor, but I'm trying to keep the regalloc2 usage contained in a single place as much possible because the idea is to eventually share the register definitions between regalloc2 and Winch instead of pulling the entire regalloc2 crate into Winch. These are long term plans though, but my hope is that having them in a single place, will make it easier to refactor once the time comes.

Wasmtime GitHub notifications bot (Sep 08 2023 at 23:18):

saulecabrera created PR review comment:

Given that this function is intended to be float-specific, I wonder if we should assert that src.class() == RegClass::Float?

Wasmtime GitHub notifications bot (Sep 09 2023 at 06:34):

elliottt created PR review comment:

That seems totally reasonable to me!

Wasmtime GitHub notifications bot (Sep 09 2023 at 06:34):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Sep 09 2023 at 06:37):

elliottt requested wasmtime-fuzz-reviewers for a review on PR #6982.

Wasmtime GitHub notifications bot (Sep 09 2023 at 06:37):

elliottt requested wasmtime-core-reviewers for a review on PR #6982.

Wasmtime GitHub notifications bot (Sep 09 2023 at 06:37):

elliottt updated PR #6982.

Last updated: Dec 23 2024 at 13:07 UTC