Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement prototype v128.load{32,64}_zero instructions #3011

Merged
merged 2 commits into from
Aug 3, 2020

Conversation

tlively
Copy link
Member

@tlively tlively commented Jul 31, 2020

Specified in WebAssembly/simd#237. Since these
are just prototypes necessary for benchmarking, this PR does not add
support for these instructions to the fuzzer or the C or JS APIs. This
PR also renumbers the QFMA instructions that previously used the
opcodes for these new instructions. The renumbering matches the
renumbering in V8 and LLVM.

Specified in WebAssembly/simd#237. Since these
are just prototypes necessary for benchmarking, this PR does not add
support for these instructions to the fuzzer or the C or JS APIs. This
PR also renumbers the QFMA instructions that previously used the
opcodes for these new instructions. The renumbering matches the
renumbering in V8 and LLVM.
@tlively tlively requested a review from kripken July 31, 2020 20:34
@@ -896,8 +896,10 @@
)
)
(func $f32x4.qfma (param $0 v128) (param $1 v128) (param $2 v128) (result v128)
(f32x4.qfma
(drop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct change?

V128Load32Zero = 0xfc,
V128Load64Zero = 0xfd,

F32x4QFMA = 0xb4,
Copy link
Contributor

@MaxGraey MaxGraey Aug 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0xb4 already used for I32x4DotSVecI16x8:
https://github.com/WebAssembly/binaryen/blob/master/src/wasm-binary.h#L836

Is it correct?

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No extra questions from me aside from those already asked by others.

@tlively
Copy link
Member Author

tlively commented Aug 3, 2020

Yes, this is the correct renumbering, although the opcode collision is unintentional. I will shift the dot instruction opcode to use the number used by V8 as well. See https://github.com/WebAssembly/tool-conventions/blob/master/Linking.md#target-features-section for the numbering I am following.

@tlively tlively merged commit daa442b into WebAssembly:master Aug 3, 2020
@tlively tlively deleted the simd-load-zero branch August 3, 2020 20:48
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.

4 participants