Skip to content

Fix i16x8.relaxed_dot_i8x16_i7x16_s pairwise add - #2252

Open
brendandahl wants to merge 1 commit into
WebAssembly:mainfrom
brendandahl:relaxed-dot-update
Open

brendandahl wants to merge 1 commit into
WebAssembly:mainfrom
brendandahl:relaxed-dot-update

Conversation

@brendandahl

Copy link
Copy Markdown
Contributor

Allow i16x8.relaxed_dot_i8x16_i7x16_s to use either wrapping or saturating pairwise addition via $R_idot instead of hardcoding saturating addition. ARM NEON uses smull/smull2/addp (signed multiply with wrapping i16 add), whereas x86-64 uses vpmaddubsw (signed/unsigned multiply with saturating i16 add).

Add a spec test covering both lowerings.

Allow i16x8.relaxed_dot_i8x16_i7x16_s to use either wrapping or
saturating pairwise addition via $R_idot instead of hardcoding
saturating addition. ARM NEON uses smull/smull2/addp (signed
multiply with wrapping i16 add), whereas x86-64 uses vpmaddubsw
(signed/unsigned multiply with saturating i16 add).

Add a spec test covering both lowerings.
@brendandahl

Copy link
Copy Markdown
Contributor Author

The spec should now match the original pseudo code and matches what is actually implemented by v8, jsc, and spidermonkey.

There are also some issues with i32x4.relaxed_dot_i8x16_i7x16_add_s and the interpreter needs some work for fixing some of the relaxed conversion implementations, but I'll do those in a separate PR.

@akirilov-arm

Copy link
Copy Markdown

FWIW I believe that the specification could be implemented as is (with saturating addition) at a similar cost to what the Wasm engines are doing currently using instructions introduced by the second version of the Scalable Vector Extension (SVE2) to the Arm architecture:

SMULLB Ztmp.H, Zin1.B, Zin2.B
SMULLT Zout.H, Zin1.B, Zin2.B
SQADD  Vout.8H, Vout.8H, Vtmp.8H

IMHO it is still worth changing the specification, since with the update proposed here the implementation could be simplified to:

MOVI Vout.2D, #0
SDOT Zout.H, Zin1.B, Zin2.B

if the FEAT_SVE2p3 architectural feature is supported by the target platform.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants