Winch: Add SIMD load and extend and load and splat instructions for x64 #9950
Conversation
jeffcharles
requested review from
abrown and
pchickey
and removed request for
a team
|
I can take this review. |
Subscribe to Label Action
This issue or pull request has been labeled: "winch"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| self.asm.uload(src, dst, size); | ||
| match kind { | ||
| LoadKind::None => self.asm.uload(src, dst, size), | ||
| LoadKind::Splat => todo!(), |
There was a problem hiding this comment.
Could you return an error here instead of panicking? I've done a refactoring across the compiler to ensure that panics are minimal, particularly for unimplemented/unsupported features. This ensures that we can can successfully run spec tests even when there's partial support for Wasm proposals.
Perhaps we can introduce a new Unimplemeted* enum entry here https://github.com/bytecodealliance/wasmtime/blob/main/winch/codegen/src/codegen/error.rs#L7, something around the lines of UnimplementedLoadKind
| let op = match ext { | ||
| VectorExtendKind::V128Extend8x8S => SseOpcode::Pmovsxbw, | ||
| VectorExtendKind::V128Extend8x8U => SseOpcode::Pmovzxbw, | ||
| VectorExtendKind::V128Extend16x4S => SseOpcode::Pmovsxwd, | ||
| VectorExtendKind::V128Extend16x4U => SseOpcode::Pmovzxwd, | ||
| VectorExtendKind::V128Extend32x2S => SseOpcode::Pmovsxdq, | ||
| VectorExtendKind::V128Extend32x2U => SseOpcode::Pmovzxdq, | ||
| }; |
There was a problem hiding this comment.
Given that we're starting with avx2 as our baseline, I wonder if we should emit the vpmov* variants here.
| /// Kinds of behavior supported by Wasm loads. | ||
| pub(crate) enum LoadKind { | ||
| /// Do not extend or splat. | ||
| None, |
There was a problem hiding this comment.
Nit: can we remove this enum entry and keep Option<LoadKind> instead? Or where you envisioning this approach to be easier to consume in the callee as opposed to using an Option?
Part of #8093. Adds support on x64 with AVX2 for:
v128.load8x8_sv128.load8x8_uv128.load16x4_sv128.load16x4_uv128.load32x2_sv128.load32x2_uv128.load8_splatv128.load16_splatv128.load32_splatv128.load64_splatI've changed
wasm_loadon the macroassemblers to take aLoadKindinstead ofOption<ExtendKind>to model the additional vector operations (vector extends and splats), but I'm open to changing that to a different abstraction.