Initial PR for Wasm.Simd. Includes build/extract and binary arithmetic ops.

[Krovatkin]

This PR includes

• A list of SIMD opcodes
• Initial extensions to WasmBytecodeGenerator and WasmBinaryReader to support reading and generating bytecodes for extract, build and arithmetic binary operations.
• Tests for these operations
• A few fixes

The fixes include:

• https://github.com/Microsoft/ChakraCore/pull/2736/files#diff-b0ef4db6dba17fc268e8745b695badceL1117
• https://github.com/Microsoft/ChakraCore/pull/2736/files#diff-9d396b386e20933b7bfbec028a04e8c9R248
• https://github.com/Microsoft/ChakraCore/pull/2736/files#diff-314ffb76644084b820e66bdfe3f879d9R377

In this version, we decided to not group SIMD operations into one case arm since this way we could encapsulate all SIMD-related reading logic in ReadOpCode. We could always move all SIMD ops into a separate case arm later.

I temp disabled short-signature comparisons since I didn't know what the best way would be to extend it to support SIMD types.

[Krovatkin]

@Cellule @MikeHolman Please take a look whenever you have a moment! Thanks!

[arunetm-zz]

@LouisLaf review please. Thanks.

[Cellule]

I was wondering how are you generating the .wasm files ?
We very recently started using wabt directly in chakra with WebAssembly.wabt.convertWast2Wasm(wastSource, {spec: isSpecFormat}
I know right now it wouldn't work since the simd opcode are not part of the spec, but maybe we would want a fork of wabt to generate those binary files

[Krovatkin]

@Cellule
I was also using wabt to generate wasm files.
I made a few silly changes to wabt to make sure its opcode numerical space was wide enough to include all SIMD operations from SIMD.js and more.
Then, I got a few python scripts to generate wast files to test cover the operations included in this PR and simple beautifier to make sure the produced wast files are readable (it's surprisingly hard to find a beautifier for LISP/Scheme-based languages :-( )

https://github.com/WebAssembly/wabt/compare/master...Krovatkin:simd?expand=1

[arunetm-zz]

@Cellule We had extended wabt to generate the multibyte simd opcodes. Agree that it make sense to maintain a fork with those changes and build on it as needed. Should we create a new fork for this, or clean up krovatkin's fork for the time being?

[dilijev]

@arunetm dotnet/dotnet-ci#626 is merged and it looks like the CI is online.

[MikeHolman]

I understand the current PR is to get up and running with a prototype, but I think we should go a different direction with the wasm design.

I envision that we add a single new local type to wasm, e.g. m128. We then have opcodes for each type of operation, which takes an immediate value to tell which specific SIMD type we want to use.

Assuming we have: enum SimdTypes = { i32x4 = 0, i64x2, f32x4, f64x2, ... };, to do a i32x4 add, you would use m128.add 0, which pops 2 m128, interpreting and adding them as i32x4, and then pushes 1 m128 value as the result.

We can also have some SIMD type specific operations for creating a SIMD value using other local types popped from the stack, for example something like m128.from_i32x4, which pops 4 i32 from the stack and pushes 1 m128.

What do you think about this?

[Cellule]

Reviewed 11 of 39 files at r1, 4 of 8 files at r2.
Review status: 15 of 45 files reviewed at latest revision, 4 unresolved discussions.

---

lib/Backend/LowerMDSharedSimd128.cpp, line 3030 at r2 (raw file):

LowererMD::EnregisterBoolConst(IR::Instr* instr, IR::Opnd *constOpnd, IRType type)
{
    bool isSet = 0;

Here you can do bool isSet = constOpnd->GetImmediateValue(instr->m_func) != 0 it will take care of checking the type of const opnd.

---

lib/Common/ConfigFlagsList.h, line 864 at r2 (raw file):

FLAGNR(Boolean, WasmCheckVersion      , "Check the binary version for WebAssembly", DEFAULT_CONFIG_WasmCheckVersion)

#ifndef COMPILE_DISABLE_Wasm_Simd

The #define is required only for experimental features.

---

lib/Runtime/Language/InterpreterStackFrame.cpp, line 2328 at r2 (raw file):

    void InterpreterStackFrame::OP_InvalidWasmTypeConversion(...)
    {
        Assert(type != Wasm::WasmTypes::Void); //

Since type is a template argument, you should make this a CompileAssert, also check type < Wasm::WasmTypes::Limit

---

lib/Runtime/Language/InterpreterStackFrame.cpp, line 2329 at r2 (raw file):

    {
        Assert(type != Wasm::WasmTypes::Void); //
        static const char16* typeNames[] = {

I think we should add a function char16* WasmTypes::GetStrId(WasmType type); and put the implementation in WasmBinaryReader.cpp. That way we'll have that hardcoded table only once and avoid problems
I've been meaning to do it, but it wasn't totally worth it with only 4 types

Also, good to note, we don't allow static variables anymore

---

Comments from Reviewable

[Krovatkin]

@MikeHolman fyi @arunetm @Cellule

Could you please help us to understand the main benefits you see of this new design?

1. Removing argument type information
   The main reason why we introduced type checks for SIMD instructions is that both WebAssembly
   and SIMD.js seem to favour strong typing and we believe this behaviour will be extended to WASM.SIMD i.e. examples like this one should be considered invalid:

let a = SIMD.Int32x4(1,1,1,1);
let b = SIMD.Int32x4(2,2,2,2);
let c = SIMD.Bool32x4.xor(a,b);
print (c);

While there is more than one phase in which the type checks could be done, the bytecode generation seemed to be the most suitable.

• It already comes w/ an excellent and flexible framework for type checking.
• Interpreter (AsmJs) and compiler don't seem to do type checks. If we don't do the type checks at the bytecode generation phase, we might need to do it twice.

2. Replacing typed opcodes w/ untyped versions taking a type immediate.

• Are you suggesting to introduce untyped opcodes and type immediates at Wasm bytecode(proposal) level?
• If the bytecode generator handles SIMD operations in untyped way internally we might be needing two conversion trips from Wasm bytecode to Wasm-Chakra bytecode and another one from Wasm-Chakra bytecode to either IR or Chakra bytecode since both use typed operations.
  If we are extending it all the way through Interpreter and compiler would we just
  • In interpreter, use the wrapper for a particular SIMD operation to dispatch it to an existing handler for the corresponding typed operation or make changes to Parser?
  • In compiler, map untyped operation and immediate to a typed one?

[arunetm-zz]

@MikeHolman, Thanks for the feedback.
Wouldn't strong type system of wasm justify having multiple simd types in wasm-bytecode. As krovatkin pointed out, we will need the 2 byte encoding to accommodate the immediate in the encoding and the wasm future features might make more use of the second byte. Are there advantages in getting rid of the simd types in the encoding itself?
In case we plan to do this as an implementation detail, can you help to understand the main benefits?

@dilijev Thanks, it works. 👍

[MikeHolman]

Is there any benefit for code generators to have strong typing for simd types? My understanding is that the simd ops are generally just reinterpretations of bytes in xmm registers? Wasm doesn't have differentiated signed and unsigned types, but different ops interpret them differently. My proposal would follow that model. Possibly other architectures' implementations of simd aren't as polymorphic as x86/x64, but I'm not sure.

This just makes the point stronger that there should be a real design proposal. I think the other wasm committee members also would prefer 1 new local type for simd128 to 6+, but it is something that needs discussion.

[Cellule]

@arunetm @Krovatkin
For the topic of opcode encoding, I don't particularly like the 2 extended opcode prefix.
I see a few options,

• Encode the opcodes with LEB128. This option would break backward compatibility however so I guess it won't fly.
• Have only 1 prefix that says to read 2 bytes or even an LEB128 value to determine the opcode. We changed a while back in chakra for the extended opcode to mean "read 2 bytes" to avoid the need for a second extended opcode prefix.
• Avoid going past 1 byte opcode. To do this we can use 1 opcode that means SIMD then put the operator and type as immediate

As @MikeHolman said, I do believe we should have a formal proposal for simd in wasm.
What I am wondering is whether you are trying to make the design through implementation or prototype.
In either case, I think we should go about this differently.
If you are trying to present to us your design for simd, it shouldn't be by implementing it, because this means we will have to refactor a ton of times until we get it right.
If what you are trying to do is prototype it, I don't think the code should be in ChakraCore. This is what forks are for.

[Krovatkin]

@MikeHolman @Cellule

Is there any benefit for code generators to have strong typing for simd types?

I guess, the "strongest" argument for strong typing has and always will be to reduce the number of logical errors.
At this point we are trying to stick to SIMD.js implementation as much as possible. I guess, my biggest question here is whether the considerations for strong typing for SIMD.js are still valid for WASM.SIMD? If they aren't, we could relax type checks in the bytecode generation phase.

What I am wondering is whether you are trying to make the design through implementation

I think we might need an initial "Chakra" implementation rather than a prototype because prototypes are usually decoupled from real-world scenarios. Implementing a feature in Chakra gives us a feel of what approaches could work, how feasible and complex they could get when implemented (in general and specifically in Chakra), so we could argue for a particular case and back it up w/ some evidence saying that "the implementation A is a way too complex or impractical in general." It also highlights the parts in Chakra where the coupling gets too tight meaning that one small change could ripple through the entire runtime, so we could try to isolate/decouple those cases.
Wasm binary format for SIMD operations is a good example of the latter. We could try out different wasm binary formats for SIMD operations as long as ReadOpCode produces OpCodeAsmJs compatible opcodes, so the rest of the compiler is unaffected. Similarly, relaxing SIMD types should be relatively straightforward due to the way you guys implement signatures for each operation.

[Cellule]

@Krovatkin What I meant is for you to prototype with chakra, just not in the Microsoft repository, by using a GitHub fork. That way you are free to test however you like and once it is to your liking/got some design approval we can merge it back into the Microsoft repository.
What I am saying also is, we are not very comfortable in having design discussion, just like we are doing right now, on Pull Requests of the actual implementation. It makes the conversation confusing between actual code review and design concerns.

[arunetm-zz]

Agree that we need to have a formal proposal in place and have these encoding/design discussions as part of the wasm spec involving the community. For now, we are relying on the following simd wasm draft proposal based on a portable subset of the SIMD.js spec - https://github.com/stoklund/portable-simd

We are looking to have feedback on a Chakra implementation(code review and solution design) of the basic types and ops (which should not change significantly for wasm.simd) from the above draft. This can be handy during the actual proposal/design. This implementation is making some assumptions on the wasm.simd encoding which I think can be easily updated without much effort based on the final design.

Will revisit the way to achieve this and update this PR. Thanks!

[MikeHolman]

Type safety can be managed at a higher level than the wasm level. Right now every instruction in wasm helps to generate code. Adding 32 reinterpret casts, which are just for type safety seems excessive. A wasm producer can emit warnings for implicit conversions if it wants, though it's a bit unclear to me how you would really get safety in any case, e.g. at the c++ level there is not a type for each of these, is there? It feels like we are adding an abstraction layer in wasm that doesn't even exist at the source level. In general, we should be making things lower level when translating to wasm, not higher.

[Krovatkin]

@Cellule @MikeHolman relaxed type checking. Since the rest of the compiler doesn't have one single type for m128, I'm using float32x4 as a lingua franca for now.

fyi @arunetm

[MikeHolman]

I think that's fine for initial prototype.

[MikeHolman]

There are a few places in interpreter where we check IsSimdjsEnabled. I think that we will need to add condition to check if WasmSimd is enabled for some of these cases (they generally relate to arguments/return value).

[arunetm-zz]

Thanks, yes will fix the feature enabling checks as applicable.

[arunetm-zz]

@MikeHolman can you please sign off this PR if all looks well.

I should be able to merge this to the WASM.Simd feature branch once approved and signed.

[MikeHolman]

Sure, just want to give @Cellule gets a chance to sign off since he had some comments.

[Krovatkin]

@MikeHolman

There are a few places in interpreter where we check IsSimdjsEnabled. I think that we will need to add condition to check if WasmSimd is enabled for some of these cases (they generally relate to arguments/return value).

I'll check.
Some cases might not be applicable to wasm.simd just yet since I didn't hook up JavascriptSIMDObject to enable SIMD argument interoperability between JS and WASM.