1124291 - SIMD (interpreter): implement int8x16 and int16x8

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[:ProgramFOX]

int8x16 and int16x8 support has been added to the polyfill:
https://github.com/johnmccutchan/ecmascript_simd/commit/00ec45c155fb9a44fd4c381cc000424e25f2765a

Comment 1

[:ProgramFOX]

Attachment: Implemented int16x8 and int8x16

Proposed patch that implements int16x8 and int8x16.

Comment 2

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

Patch containing test cases for int16x8 and int8x16

Comment 3

[:ProgramFOX]

Attachment: Implemented int16x8 and int8x16

The implementation patch had to be rebased, this is the new one. (The test cases patch didn't need to be rebased, it can still be applied)

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8564600 [details] [diff] [review]
Implemented int16x8 and int8x16

Review of attachment 8564600 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the long review, I've got quite a lot on my plate these last few weeks.
Nice work of generalizing operations in SIMD.cpp! Requesting a second round of review, because there are some missing operations and this patch should be split, but this looks pretty good.

::: js/public/Conversions.h
@@ +30,5 @@
>  
>  /* DO NOT CALL THIS.  Use JS::ToNumber. */
>  extern JS_PUBLIC_API(bool)
>  ToNumberSlow(JSContext *cx, JS::Value v, double *dp);
>  

Could you split the changes made in this file into another smaller patch, with the changes as in jsnum.cpp, and ask :Waldo for review please? I don't have reviewing authority on public/

@@ +175,5 @@
>      }
>      return js::ToUint32Slow(cx, v, out);
>  }
>  
> +/* ES6 draft 20141224, 7.1.7. */

this is 7.1.8 in the latest draft, see also http://people.mozilla.org/~jorendorff/es6-draft.html

::: js/src/builtin/SIMD.cpp
@@ +155,5 @@
> +    LANE_ACCESSOR(Int16x8, 3);
> +    LANE_ACCESSOR(Int16x8, 4);
> +    LANE_ACCESSOR(Int16x8, 5);
> +    LANE_ACCESSOR(Int16x8, 6);
> +    LANE_ACCESSOR(Int16x8, 7);

Can you instead 1) define a macro EIGHT_LANES_ACCESSOR that reuses FOUR_LANES_ACCESSOR for the first 4 lanes, 2) uses this macro for Int16x8, 3) reuse this macro for Int8x16 and define the remaining 8 lanes by hand?

@@ +446,5 @@
>        case SimdTypeDescr::TYPE_FLOAT64:
>          return "1";
> +      case SimdTypeDescr::TYPE_INT8:
> +        return "15";
> +      case SimdTypeDescr::TYPE_INT16:

Can you order cases by type length, please?

@@ +753,5 @@
>      static inline T apply(T l, T r) { return IsNaN(l) ? r : (IsNaN(r) ? l : math_max_impl(l, r)); }
>  };
>  template<typename T>
>  struct LessThan {
> +    static inline int32_t apply(T l, T r) { return l < r ? -1 : 0x0; }

Here and below, can you drop the 0x in front 0, now, please?

@@ +1110,4 @@
>          return ErrorBadArgs(cx);
> +    for (unsigned i = 0; i < In::lanes; i++) {
> +        if (!args[i].isBoolean())
> +            return ErrorBadArgs(cx);

Actually, it seems this requirement is too strong and we could relax it. args[i] could be any type (there is no special requirement in the spec polyfill, which just applies (!x - 1) to determine what we should store, so any value which is !-able could fit in). Do you mind opening a followup for this, please, and CC me? (and you can also fix it, if you want to!)

::: js/src/builtin/SIMD.h
@@ +138,5 @@
> +  V(fromInt16x8Bits, (FuncConvertBits<Int16x8, Int8x16>), 1, 0)                     \
> +  V(fromInt32x4Bits, (FuncConvertBits<Int32x4, Int8x16>), 1, 0)                     \
> +  V(neg, (UnaryFunc<Int8x16, Neg, Int8x16>), 1, 0)                                  \
> +  V(not, (UnaryFunc<Int8x16, Not, Int8x16>), 1, 0)                                  \
> +  V(splat, (FuncSplat<Int8x16>), 0, 0)

nit: splat has 1 argument, not 0

Also, this list could contain check as well.

@@ +179,5 @@
> +  V(fromInt8x16Bits, (FuncConvertBits<Int8x16, Int16x8>), 1, 0)                     \
> +  V(fromInt32x4Bits, (FuncConvertBits<Int32x4, Int16x8>), 1, 0)                     \
> +  V(neg, (UnaryFunc<Int16x8, Neg, Int16x8>), 1, 0)                                  \
> +  V(not, (UnaryFunc<Int16x8, Not, Int16x8>), 1, 0)                                  \
> +  V(splat, (FuncSplat<Int16x8>), 0, 0)

same remarks as for Int8x16

@@ +479,4 @@
>  extern bool                                                          \
>  simd_int32x4_##Name(JSContext *cx, unsigned argc, Value *vp);
> +INT32X4_FUNCTION_LIST(DECLARE_SIMD_INT32X4_FUNCTION)
> +#undef DECLARE_SIMD_INT32X4_FUNCTION

nice catch

::: js/src/builtin/TypedObject.js
@@ +567,5 @@
>    var protoString = SimdProtoString(type);
>    var length = SimdTypeToLength(type);
>    if (length == 4)
>      return protoString+"("+this.x+", "+this.y+", "+this.z+", "+this.w+")";
>    else if (length == 2)

Can you please reorder this so that length are ordered from smaller lengths to bigger lengths?

::: js/src/jit/MCallOptimize.cpp
@@ +2701,5 @@
>          break;
>        case SimdTypeDescr::TYPE_FLOAT64:
>          return InliningStatus_NotInlined; // :TODO: NYI (Bug 1124205)
> +      case SimdTypeDescr::TYPE_INT8:
> +        return InliningStatus_NotInlined; // :TODO: NYI

Can you please file bugs for inlining these in Ion?

::: js/src/jsnum.cpp
@@ +1561,5 @@
> + * Convert a value to an int8_t, according to the WebIDL rules for byte
> + * conversion. Return converted value in *out on success, false on failure.
> + */
> +JS_PUBLIC_API(bool)
> +js::ToInt8Slow(JSContext *cx, const HandleValue v, int8_t *out)

As said in Conversions.h, please put changes to this file in the same split patch as Conversions.h and ask :Waldo to review.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8554188 [details] [diff] [review]
Added test cases for int16x8 and int8x16

Review of attachment 8554188 [details] [diff] [review]:
-----------------------------------------------------------------

Now it sounds like a good time to group operations, because the number of new files is growing exponentially.  Do you mind taking care of bug 1063946 before this, in different small patches, please? We could group:
- getters / setters tests
- conversions tests
- bit conversions tests
- handle / reify tests
- bool tests

Comment 6

[:ProgramFOX]

Attachment: Added ToInt8 and ToInt16

This patch adds ToInt8 and ToInt16.

Comment 7

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Updated patch that implements int8x16 and int16x8.

> Now it sounds like a good time to group operations, because the number of new files is growing exponentially. Do you mind taking care of bug 1063946 before this, in different small patches, please?

I certainly agree, but wasn't there already someone who wanted to work on it?

Comment 8

[:ProgramFOX]

A try push, with the tests in their current state:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f4946c06d067

Comment 9

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to ProgramFOX from comment #7)
> > Now it sounds like a good time to group operations, because the number of new files is growing exponentially. Do you mind taking care of bug 1063946 before this, in different small patches, please?
> 
> I certainly agree, but wasn't there already someone who wanted to work on it?

It's been a week now and the user there is inactive. I'll post in the bug.

Comment 10

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8567145 [details] [diff] [review]
Implemented int8x16 and int16x8

Review of attachment 8567145 [details] [diff] [review]:
-----------------------------------------------------------------

Almost there! I'll r+ it after a last look, plus the two follow-up bugs filed, please.

::: js/src/builtin/SIMD.cpp
@@ +1088,4 @@
>          return ErrorBadArgs(cx);
> +    for (unsigned i = 0; i < In::lanes; i++) {
> +        if (!args[i].isBoolean())
> +            return ErrorBadArgs(cx);

Please file follow up as asked in the previous review and CC me on it :)

@@ +1092,5 @@
>      }
>  
> +    InElem result[In::lanes];
> +    for (unsigned i = 0; i < In::lanes; i++)
> +        result[i] = args[i].toBoolean() ? -1 : 0x0;

nit: drop the 0x prefix here as well

::: js/src/builtin/SIMD.h
@@ +167,5 @@
> +  V(select, (Select<Int8x16, Int8x16>), 3, 0)                                       \
> +  V(store, (Store<Int8x16, 16>), 3, 0)                                              \
> +
> +#define INT8X16_QUARTERNARY_FUNCTION_LIST(V)                                        \
> +  V(bool, Bool<Int8x16>, 4, 0)

Hmm, bool is actually sixteen-ary (?), i.e. it takes 16 arguments. Please fix the macro name ("INT8X16_BOOL_FUNCTION") and the number of args in the macro (it's 4 at the moment).

@@ +209,5 @@
> +  V(select, (Select<Int16x8, Int16x8>), 3, 0)                                       \
> +  V(store, (Store<Int16x8, 16>), 3, 0)                                              \
> +
> +#define INT16X8_QUARTERNARY_FUNCTION_LIST(V)                                        \
> +  V(bool, Bool<Int16x8>, 4, 0)

ditto

::: js/src/jit/MCallOptimize.cpp
@@ +2829,5 @@
>          return InliningStatus_NotInlined; // :TODO: NYI (Bug 1124205)
> +      case SimdTypeDescr::TYPE_INT8:
> +        return InliningStatus_NotInlined; // :TODO: NYI
> +      case SimdTypeDescr::TYPE_INT16:
> +        return InliningStatus_NotInlined; // :TODO: NYI

Please file a follow up bug and mark its bug number here. We don't put TODOs without bug numbers in the code.

Comment 11

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Updated patch.

IonMonkey bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1136226
Type requirement bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1136221

Comment 12

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8568645 [details] [diff] [review]
Implemented int8x16 and int16x8

Review of attachment 8568645 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks a lot!

::: js/src/builtin/SIMD.h
@@ +151,5 @@
> +  V(greaterThan, (CompareFunc<Int8x16, GreaterThan, Int8x16>), 2, 0)                \
> +  V(greaterThanOrEqual, (CompareFunc<Int8x16, GreaterThanOrEqual, Int8x16>), 2, 0)  \
> +  V(lessThan, (CompareFunc<Int8x16, LessThan, Int8x16>), 2, 0)                      \
> +  V(lessThanOrEqual, (CompareFunc<Int8x16, LessThanOrEqual, Int8x16>), 2, 0)        \
> +  V(load,    (Load<Int8x16, 4>), 2, 0)                                              \

nit: unnecessary spaces

@@ +193,5 @@
> +  V(greaterThan, (CompareFunc<Int16x8, GreaterThan, Int16x8>), 2, 0)                \
> +  V(greaterThanOrEqual, (CompareFunc<Int16x8, GreaterThanOrEqual, Int16x8>), 2, 0)  \
> +  V(lessThan, (CompareFunc<Int16x8, LessThan, Int16x8>), 2, 0)                      \
> +  V(lessThanOrEqual, (CompareFunc<Int16x8, LessThanOrEqual, Int16x8>), 2, 0)        \
> +  V(load,    (Load<Int16x8, 4>), 2, 0)                                              \

nit: unnecessary spaces

Comment 13

[:ProgramFOX]

Attachment: Added ToInt8 and ToInt16

The patch adding ToInt8 and ToInt16, with r=Waldo appended to the commit message.

Comment 14

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Updated patch. I had to rebase it, and I also had to add some more `case`s to some switch statements. Because I don't think this is a very minor change, I r?'ed again.

Comment 15

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8574309 [details] [diff] [review]
Implemented int8x16 and int16x8

Review of attachment 8574309 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! Assuming nothing changed in SIMD.cpp (I glanced at it quickly), r=me

Comment 16

[:ProgramFOX]

(In reply to Benjamin Bouvier [:bbouvier] from comment #15)
> Comment on attachment 8574309 [details] [diff] [review]
> Implemented int8x16 and int16x8
> 
> Review of attachment 8574309 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks! Assuming nothing changed in SIMD.cpp (I glanced at it quickly), r=me

I had to to a small rebase in SIMD.cpp: rebasing the Bool function after bug 1136221, and I also had to add a `const` keyword to the `template JSObject *js::CreateSimd` lines of Int8x16 and Int16x8 (which was necessary after bug 1112165).

Comment 17

[Florian Scholz (Open Web Docs)]

Looks like the patches here are r+. Can we actually land this now?

Comment 18

[:ProgramFOX]

(In reply to Florian Scholz [:fscholz] (MDN) from comment #17)
> Looks like the patches here are r+. Can we actually land this now?

Not yet; there have been changes to the SIMD files while waiting for the blocking bug to be fixed, so the patches first have to be rebased, and tests for it have to be written.

Comment 19

[:ProgramFOX]

Attachment: Added ToInt8 and ToInt16

The rebased patch that adds ToInt8 and ToInt16.

Comment 20

[:ProgramFOX]

Attachment: Implemented Int8x16 and Int16x8

The rebased patch for int8x16 and int16x8. Rebased test cases are coming soon; I've been able to re-use the original test cases I made to test this patch, and I'll soon merge these test cases with the bigger test case files.

Comment 21

[:ProgramFOX]

So I just received a mail that my review request is overdue. Do I have to do something with that?

Comment 22

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to :ProgramFOX from comment #21)
> So I just received a mail that my review request is overdue. Do I have to do
> something with that?

Sorry about that. I'll get to it quickly, probably at the start of next week.

Comment 23

[:ProgramFOX]

No problem! I'll try to get the test cases ready this week.

Comment 24

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

The int16x8 and int8x16 patch, rebased.

Comment 25

[:ProgramFOX]

I'm working on the test cases. Now, for swizzle and shuffle, the amount of possible operations is so large for int8x16 and int16x8 that the test cases take very long to run - and will probably timeout on TBPL. Should I only test a part of the possible cases?

Comment 26

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to :ProgramFOX from comment #25)
> I'm working on the test cases. Now, for swizzle and shuffle, the amount of
> possible operations is so large for int8x16 and int16x8 that the test cases
> take very long to run - and will probably timeout on TBPL. Should I only
> test a part of the possible cases?

Yes, testing only a subset of the possible cases sounds fine. I'd make sure to test various, interesting patterns (all the same values, all different, double pairs (e.g. 1-2-1-2), etc.).

Comment 27

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

The test cases for int16x8 and int8x16.

Comment 28

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8611374 [details] [diff] [review]
Implemented int8x16 and int16x8

Review of attachment 8611374 [details] [diff] [review]:
-----------------------------------------------------------------

Wow, so much stuff has changed since the previous version. I actually found a few things that need to be fixed (in particular the maximum mask for the shifts), plus a few nice-to-haves. Keep up the good work!

::: js/src/builtin/SIMD.cpp
@@ +710,5 @@
>      static T apply(T l, T r) { return IsNaN(l) ? r : (IsNaN(r) ? l : math_max_impl(l, r)); }
>  };
>  template<typename T>
>  struct LessThan {
> +    static int32_t apply(T l, T r) { return l < r ? -1 : 0x0; }

Here and everywhere below, please remove the 0x prefix

@@ +955,1 @@
>          result[i] = Op::apply(val[i], bits);

Don't forget to change the ShiftLeft / both ShiftRight, as their behavior is closely tight to int32 so far. In particular, the maximum value for the mask needs to be changed with respect to V.

@@ +1138,5 @@
>  
>      return StoreResult<In>(cx, args, result);
>  }
>  
> +template<typename V, typename Coercion>

Can you rename Coercion into MaskType as well, please?

::: js/src/builtin/TypedObject.js
@@ +157,5 @@
>      var y = Load_float64(typedObj, offset + 8);
>      return GetFloat64x2TypeDescr()(x, y);
>  
> +  case JS_SIMDTYPEREPR_INT8:
> +    var s0 = Load_int8(typedObj, offset + 0);

Out of curiosity, can you try using an array and a loop, here? maybe pushing to arrays is weird because we're in self-hosted code, but otherwise, having a loop here would be cleaner.

@@ +173,5 @@
> +    var s12 = Load_int8(typedObj, offset + 12);
> +    var s13 = Load_int8(typedObj, offset + 13);
> +    var s14 = Load_int8(typedObj, offset + 14);
> +    var s15 = Load_int8(typedObj, offset + 15);
> +    return GetInt8x16TypeDescr()(s0, s1, s2, s3, s4, s5, s6, s7,

and maybe you can use rest parameters here: if lanes is the name of the array described earlier:

return GetInt8x16TypeDescr()(...lanes);

@@ +177,5 @@
> +    return GetInt8x16TypeDescr()(s0, s1, s2, s3, s4, s5, s6, s7,
> +                                 s8, s9, s10, s11, s12, s13, s14, s15);
> +
> +  case JS_SIMDTYPEREPR_INT16:
> +    var s0 = Load_int16(typedObj, offset + 0);

ditto

@@ +362,5 @@
>        Store_float64(typedObj, offset + 0, Load_float64(fromValue, 0));
>        Store_float64(typedObj, offset + 8, Load_float64(fromValue, 8));
>        break;
> +    case JS_SIMDTYPEREPR_INT8:
> +      Store_int8(typedObj, offset + 0, Load_int8(fromValue, 0));

ditto

@@ +380,5 @@
> +      Store_int8(typedObj, offset + 14, Load_int8(fromValue, 14));
> +      Store_int8(typedObj, offset + 15, Load_int8(fromValue, 15));
> +      break;
> +    case JS_SIMDTYPEREPR_INT16:
> +      Store_int16(typedObj, offset + 0, Load_int16(fromValue, 0));

ditto

@@ +566,5 @@
>    var type = DESCR_TYPE(descr);
>    var protoString = SimdProtoString(type);
>    var length = SimdTypeToLength(type);
> +  if (length == 2)
> +    return protoString+"("+this.x+", "+this.y+")";

Please use template strings, here and everywhere below:

return `${protoString}(${this.x}, ${this.y})`;

::: js/src/jit/IonBuilder.cpp
@@ +10330,5 @@
>        case SimdTypeDescr::Float32x4: return MIRType_Float32x4;
> +      case SimdTypeDescr::Float64x2:
> +      case SimdTypeDescr::Int8x16:
> +      case SimdTypeDescr::Int16x8:
> +        return MIRType_Undefined;

Can you keep alignment with the 'return' please, and order from small to big? (16x8, 8x16, 64x2)

::: js/src/jit/MCallOptimize.cpp
@@ +3116,5 @@
>  {
>      // Generic constructor of SIMD valuesX4.
>      MIRType simdType = SimdTypeDescrToMIRType(descr->type());
>  
> +    // TODO Happens for Float64x2 (Bug 1124205) and Int8x16/Int16x8 (Bug 1136226)

Thanks for the note!

Comment 29

[:ProgramFOX]

> Out of curiosity, can you try using an array and a loop, here?
I've tried it, it's not possible, as this error says:
> Exception: Failed to test XUL condition '!this.hasOwnProperty("SIMD")'; output was '', stderr was 'Assertion failure: emitterMode != BytecodeEmitter::SelfHosting (.next() iteration is prohibited in self-hosted code because it can run user-modifiable iteration code), at /home/programfox/mozilla/mozilla-central/js/src/frontend/BytecodeEmitter.cpp:3548\n'

Comment 30

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Updated patch, with the remarks fixed.

Comment 31

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

Updated test cases. I had to change the shifts.js one after changing the code of ShiftLeft/ShiftRight.

Comment 32

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8614750 [details] [diff] [review]
Implemented int8x16 and int16x8

Review of attachment 8614750 [details] [diff] [review]:
-----------------------------------------------------------------

Getting there! Will happily review your test updates with the new version of this patch :)

::: js/src/builtin/SIMD.cpp
@@ +710,5 @@
>      static T apply(T l, T r) { return IsNaN(l) ? r : (IsNaN(r) ? l : math_max_impl(l, r)); }
>  };
>  template<typename T>
>  struct LessThan {
> +    static int32_t apply(T l, T r) { return l < r ? -1 : 0; }

With this, we would return int32_t even for comparisons between two int8_t, which will get truncated as an int8_t / int16_t later. Instead of adding a new template parameter (which would be possible solution), I'd prefer to have all the comparison functors return a bool (so in this case e.g. return l < r; }, and then have the CompareFunc convert it into -1 or 0. It's also nicer to have the boolean conversion (x ? -1 : 0) in one place.

@@ +768,2 @@
>  struct ShiftLeft {
> +    static T apply(T v, int32_t bits, int32_t maxBits) {

In this functor and the two ShiftRight, you can use sizeof(T) instead of the maxBits argument.

@@ +934,5 @@
>  
>      return StoreResult<V>(cx, args, result);
>  }
>  
> +template<template<typename T> class Op, typename V>

nit: can you put V as the first template parameter, please?

@@ +954,5 @@
>      if (!ToInt32(cx, args[1], &bits))
>          return false;
>  
> +    for (unsigned i = 0; i < V::lanes; i++)
> +        result[i] = Op<Elem>::apply(val[i], bits, 16 / V::lanes * 8);

(the last argument can be removed here)

::: js/src/builtin/SIMD.h
@@ +163,5 @@
> +  V(store, (Store<Int8x16, 16>), 3)                                                   \
> +  V(withX, (FuncWith<Int8x16, WithX>), 2)                                             \
> +  V(withY, (FuncWith<Int8x16, WithY>), 2)                                             \
> +  V(withZ, (FuncWith<Int8x16, WithZ>), 2)                                             \
> +  V(withW, (FuncWith<Int8x16, WithW>), 2)                                             \

Can you remove the with* methods, please, here and for int16x8? ExtractLane is coming soon anyways, and you would actually need to add a lot of functors for int8x16 and int16x8...

@@ +164,5 @@
> +  V(withX, (FuncWith<Int8x16, WithX>), 2)                                             \
> +  V(withY, (FuncWith<Int8x16, WithY>), 2)                                             \
> +  V(withZ, (FuncWith<Int8x16, WithZ>), 2)                                             \
> +  V(withW, (FuncWith<Int8x16, WithW>), 2)                                             \
> +  V(xor, (BinaryFunc<Int8x16, Xor, Int8x16>), 2)

A few methods are missing, but at this point, considering the size of the patch, I think it's better to land and implement missing methods as follow-ups.

@@ +171,5 @@
> +  V(bitselect, (BitSelect<Int8x16, Int8x16>), 3)                                      \
> +  V(select, (Select<Int8x16, Int8x16>), 3)
> +
> +#define INT8X16_QUARTERNARY_FUNCTION_LIST(V)                                          \
> +  V(bool, Bool<Int8x16>, 4)

should receive 16 arguments, not 4

@@ +174,5 @@
> +#define INT8X16_QUARTERNARY_FUNCTION_LIST(V)                                          \
> +  V(bool, Bool<Int8x16>, 4)
> +
> +#define INT8X16_SHUFFLE_FUNCTION_LIST(V)                                              \
> +  V(swizzle, Swizzle<Int8x16>, 2)                                                     \

swizzle should receive 1 + 16 == 17 arguments, not 2 (realize these values aren't correct for float32x4 and int32x4 as well; feel free to fix them)

@@ +175,5 @@
> +  V(bool, Bool<Int8x16>, 4)
> +
> +#define INT8X16_SHUFFLE_FUNCTION_LIST(V)                                              \
> +  V(swizzle, Swizzle<Int8x16>, 2)                                                     \
> +  V(shuffle, Shuffle<Int8x16>, 3)

shuffle should receive 2 + 16 == 18 arguments, not 3

@@ +214,5 @@
> +  V(store, (Store<Int16x8, 8>), 3)                                                    \
> +  V(withX, (FuncWith<Int16x8, WithX>), 2)                                             \
> +  V(withY, (FuncWith<Int16x8, WithY>), 2)                                             \
> +  V(withZ, (FuncWith<Int16x8, WithZ>), 2)                                             \
> +  V(withW, (FuncWith<Int16x8, WithW>), 2)                                             \

ditto

@@ +222,5 @@
> +  V(bitselect, (BitSelect<Int16x8, Int16x8>), 3)                                      \
> +  V(select, (Select<Int16x8, Int16x8>), 3)
> +
> +#define INT16X8_QUARTERNARY_FUNCTION_LIST(V)                                          \
> +  V(bool, Bool<Int16x8>, 4)

should receive 8 arguments, not 4

@@ +226,5 @@
> +  V(bool, Bool<Int16x8>, 4)
> +
> +#define INT16X8_SHUFFLE_FUNCTION_LIST(V)                                              \
> +  V(swizzle, Swizzle<Int16x8>, 2)                                                     \
> +  V(shuffle, Shuffle<Int16x8>, 3)

number of arguments is wrong here too

Comment 33

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Updated patch with the remarks fixed. I also had to make an additional change to a switch statement in MCallOptimize.cpp.

Comment 34

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8621549 [details] [diff] [review]
Implemented int8x16 and int16x8

Review of attachment 8621549 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, looks perfect! (two comments, but feel free to either ignore them or fix them and carry forward the r+)

::: js/src/builtin/SIMD.h
@@ +159,5 @@
> +  V(sub, (BinaryFunc<Int8x16, Sub, Int8x16>), 2)                                      \
> +  V(shiftLeftByScalar, (BinaryScalar<Int8x16, ShiftLeft>), 2)                         \
> +  V(shiftRightArithmeticByScalar, (BinaryScalar<Int8x16, ShiftRightArithmetic>), 2)   \
> +  V(shiftRightLogicalByScalar, (BinaryScalar<Int8x16, ShiftRightLogical>), 2)         \
> +  V(store, (Store<Int8x16, 16>), 3)                                                   \

I am very nit-picking here, but store does take 3 arguments, which makes it a ternary function (and it should belong to the next list). Feel free to ignore this anyways, it seems that I will have to change a few things in this file anyways :)

@@ +166,5 @@
> +#define INT8X16_TERNARY_FUNCTION_LIST(V)                                              \
> +  V(bitselect, (BitSelect<Int8x16, Int8x16>), 3)                                      \
> +  V(select, (Select<Int8x16, Int8x16>), 3)
> +
> +#define INT8X16_QUARTERNARY_FUNCTION_LIST(V)                                          \

(nit picking again: this doesn't sound like 16 arguments make "bool" a quarternary function, but, well, not a big deal)

Comment 35

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Done. I renamed the quaternary list and moved the store functions (and also did this for float32x4, float64x2 and int32x4).

Comment 36

[Benjamin Bouvier [:bbouvier] (inactive)]

Hi ProgramFOX, are you planning to rework the test patch, or should I give it a look now? Thanks!

Comment 37

[:ProgramFOX]

The test cases as on comment 31 are the test cases that are reworked to the new test file structure. However, they might need rebasing just like the main patch, as I just noticed. Working on that now.

Comment 38

[:ProgramFOX]

Attachment: implemented-int16x8-8x16-11.diff

Comment 39

[:ProgramFOX]

Comment on attachment 8627213 [details] [diff] [review]
implemented-int16x8-8x16-11.diff

Rebased patch for int8x16 and int16x8. Had to rebase SIMD.h and the Shift* structs at SIMD.cpp. I also added replaceLane for int8x16 and int16x8.

(sorry for posting this as two comments; I submitted too fast on the first one)

Comment 40

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

Updated test cases. They did not need to be rebased, but I added the test cases for replaceLane for int8x16 and int16x8.

Comment 41

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8627213 [details] [diff] [review]
implemented-int16x8-8x16-11.diff

Review of attachment 8627213 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 42

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8627214 [details] [diff] [review]
Added test cases for int16x8 and int8x16

Review of attachment 8627214 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the test cases! A few comments below. Please make sure to address them, ensure that your test cases run fine on your machine and then send these two patches to try! I'd like to have another look at the tests.

::: js/src/tests/ecma_7/SIMD/binary-operations.js
@@ +145,5 @@
> +
> +  var vals = [
> +    [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16],
> +     [10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120, 130, 140, 150, 160]],
> +    [[INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, 0, -1, -2, -3, -4, -5, -6, -7, -8, -9],

As you're reusing the same values for all tests, how about hoisting this array outside of this function, rename it something like "i8x16vals", and reuse it in all the functions?

Also, for the second couple to be more interesting, can you make it:
[[INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, 8, 9, 10, 11, 12, 13, 14, 15],
 [1, 1, -1, -1, INT8_MAX, INT8_MAX, INT8_MIN, INT8_MIN, 8, 9, 10, 11, 12, 13, 14, 15]]

Ditto for the int16x8 arrays and values in this array?

::: js/src/tests/ecma_7/SIMD/comparisons.js
@@ +165,5 @@
> +      int8x16(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16),
> +      int8x16(-1, -2, -3, -4, -5, -6, -7, -8, -9, -10, -11, -12, -13, -14, -15, -16),
> +      int8x16(-1, 2, -3, 4, -5, 6, -7, 8, -9, 10, -11, 12, -13, 14, -15, 16),
> +      int8x16(1, -2, 3, -4, 5, -6, 7, -8, 9, -10, 11, -12, 13, -14, 15, -16),
> +      int8x16(INT8_MAX, INT8_MAX, INT8_MIN, INT8_MIN, INT8_MIN - 1, INT8_MAX + 1, -7, -8, -9, -10, -11, -12, -13, -14, -15, -16),

Hmm, INT8_MIN - 1 will overflow, so it will be equal to INT8_MAX. Can you change this to INT8_MIN + 1? And change INT8_MAX + 1 to INT8_MAX - 1 (for the same reason)? Also, can you change the equivalent values in int16x8val?

::: js/src/tests/ecma_7/SIMD/int16x8bool.js
@@ +1,5 @@
> +// |reftest| skip-if(!this.hasOwnProperty("SIMD"))
> +var BUGNUMBER = 1124291;
> +var int16x8 = SIMD.int16x8;
> +
> +var summary = 'int16x8 bool';

no BUGNUMBER and summary, please

::: js/src/tests/ecma_7/SIMD/int8x16bool.js
@@ +1,2 @@
> +// |reftest| skip-if(!this.hasOwnProperty("SIMD"))
> +var BUGNUMBER = 1124291;

Please remove BUGNUMBER and summary in the newly introduced test cases.

::: js/src/tests/ecma_7/SIMD/load.js
@@ +79,5 @@
>              assertFunc(v, slice(index, 1));
>          },
>  
>          load2: function(index) {
> +            if (lanes < 4 || lanes >= 8)

if (lanes !== 4)

@@ +86,5 @@
>              assertFunc(v, slice(index, 2));
>          },
>  
>         load3: function(index) {
> +           if (lanes < 4 || lanes >= 8)

ditto

@@ +155,5 @@
>          C.load3(3);
>          C.load3(lastValidArgLoad3);
>  
> +        if (lanes < 8) {
> +            assertThrowsInstanceOf(() => SIMD[kind].load(ta, lastValidArgLoad + 1), RangeError);

the first line should be out of this condition, as all types support load()

::: js/src/tests/ecma_7/SIMD/replaceLane.js
@@ +42,5 @@
> +    return [arr[0], arr[1], arr[2], x, arr[4], arr[5], arr[6], arr[7],
> +            arr[8], arr[9], arr[10], arr[11], arr[12], arr[13], arr[14], arr[15]];
> +}
> +
> +function replaceLane4(arr, x) {

I think replaceLaneN(laneIndex, arr, value) can be implemented this way:

function replaceLaneN(laneIndex, arr, value) {
  var copy = arr.slice();
  assert(laneIndex <= arr.length);
  copy[laneIndex] = value;
  return copy
}

And thanks to currying, you can spare lots of chars here:
var replaceLane0 = replaceLaneN.bind(null, 0);
var replaceLane1 = replaceLaneN.bind(null, 1);
// etc.

The good thing is that it always works, no matter what the vector length is. Feel free to replace all the replaceLaneN functions, or leave it for later as you prefer.

Actually, some of these functions are used only once (e.g. replaceLane10), so feel free to just use replaceLaneN(10, arr, val) at these places!

::: js/src/tests/ecma_7/SIMD/select-bitselect.js
@@ +12,4 @@
>  var int32x4 = SIMD.int32x4;
>  
> +function getMask(i, maskLength) {
> +    if (maskLength == 4)

var args = [];
for (var j = 0; j < maskLength; j++) args.push(!!((i >> j) & 1));
if (maskLength == 4)
  return int32x4.bool(...args);
// etc

::: js/src/tests/ecma_7/SIMD/shell.js
@@ +151,3 @@
>      else
>          throw new TypeError("Unknown SIMD kind.");
> +}        

nit: trailing whitespace

@@ +182,5 @@
>      for (var i = 0; i < observed.length; i++)
>          assertEq(observed[i], expected[i]);
>  }
>  
> +function testBinaryCompare(v, w, simdFunc, func, outType) {

I wouldn't worry about the outType (and remove it from the list of arguments, plus the associated assertion). Bool vector types will fix this in the future.

::: js/src/tests/ecma_7/SIMD/shifts.js
@@ +49,5 @@
> +            int8x16(-1, 2, -3, 4, -5, 6, -7, 8, -9, 10, -11, 12, -13, 14, -15, 16),
> +            int8x16(INT8_MAX, INT8_MIN, INT8_MAX - 1, INT8_MIN + 1)
> +       ])
> +  {
> +      for (var bits = -2; bits < 36; bits++) {

36 is much high for this case: what about 12 instead?

@@ +64,5 @@
> +            int16x8(-1, 2, -3, 4, -5, 6, -7, 8),
> +            int16x8(INT16_MAX, INT16_MIN, INT16_MAX - 1, INT16_MIN + 1)
> +       ])
> +  {
> +      for (var bits = -2; bits < 36; bits++) {

ditto: what about 20?

::: js/src/tests/ecma_7/SIMD/signmask.js
@@ +21,5 @@
> +function test_int8x16() {
> +  var v, w;
> +  for ([v, w] of [[int8x16(-1, 20, 30, 4, -5, 6, 70, -80, 9, 100, -11, 12, 13, -14, 15, -16), 0b1010010010010001],
> +                  [int8x16(10, 2, 30.2, -4, -5.2, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16), 0b11000],
> +                  [int8x16(0, 0x80000000, 0x7fffffff, -0, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16), 0b0100]])

i think the expected result should be 0b1000, can you confirm? Also, can you change the input: 0x80000000 to INT8_MIN, 0x7fffffff to INT8_MAX? (the other values don't make sense on 8 bits and will get truncated in a not-obvious and immediate way) Of course, the expected result will need to change as well :)

@@ +34,5 @@
> +function test_int16x8() {
> +  var v, w;
> +  for ([v, w] of [[int16x8(-1, 20, 30, 4, -5, 6, 70, -80), 0b10010001],
> +                  [int16x8(10, 2, 30.2, -4, -5.2, 6, 7, 8), 0b11000],
> +                  [int16x8(0, 0x80000000, 0x7fffffff, -0, 5, 6, 7, 8), 0b0100]])

ditto

::: js/src/tests/ecma_7/SIMD/swizzle-shuffle.js
@@ +79,5 @@
>          for (var i = 0; i < Math.pow(2, 2); i++) {
>            [x, y] = [x & 1, (y >> 1) & 1];
>            assertEqVec(type.swizzle(v, x, y), swizzle2(simdToArray(v), x, y));
>          }
> +    } else if (lanes == 8) {

please order the if/else by number of lanes.

@@ +85,5 @@
> +        var vals = [[1, 2, 1, 2, 1, 2, 1, 2], [1, 1, 1, 1, 1, 1, 1, 1], [0, 1, 2, 3, 4, 5, 6, 7],
> +                    [7, 6, 5, 4, 3, 2, 1, 0], [5, 3, 2, 6, 1, 7, 4, 0]];
> +        for (var t of vals) {
> +          [s0, s1, s2, s3, s4, s5, s6, s7] = t;
> +          assertEqVec(type.swizzle(v, s0, s1, s2, s3, s4, s5, s6, s7), swizzle8(simdToArray(v), s0, s1, s2, s3, s4, s5, s6, s7));

Feel free to use the spread operator:

assertEqVec(type.swizzle(v, ...t), swizzle8(simdToArray(v), ...t));

@@ +99,5 @@
> +                    [5, 14, 3, 2, 6, 9, 1, 10, 7, 11, 4, 0, 13, 15, 8, 12]];
> +        for (var t of vals) {
> +          [s0, s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15] = t;
> +          assertEqVec(type.swizzle(v, s0, s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15),
> +                      swizzle16(simdToArray(v), s0, s1, s2, s3, s4, s5, s6, s7, s8, s9, s10, s11, s12, s13, s14, s15));

ditto

@@ +128,5 @@
>  
>          // In bounds is [0, 1]
>          assertThrowsInstanceOf(() => type.swizzle(v, 0, -1), TypeError);
>          assertThrowsInstanceOf(() => type.swizzle(v, 0, 2), TypeError);
> +    } else if (lanes == 8) {

(please order the if/else by number of lanes)

@@ +259,3 @@
>          lhs = type(1, 2);
>          rhs = type(3, 4);
> +    } else if (lanes == 8) {

(order here too)

@@ +289,5 @@
>              [x, y] = [i & 3, (i >> 3) & 3];
>              assertEqVec(type.shuffle(lhs, rhs, x, y),
>                          shuffle2(simdToArray(lhs), simdToArray(rhs), x, y));
>          }
> +    } else if (lanes == 8) {

(order here too)

@@ +341,5 @@
>  
>          // In bounds is [0, 3]
>          assertThrowsInstanceOf(() => type.shuffle(lhs, rhs, 0, -1), TypeError);
>          assertThrowsInstanceOf(() => type.shuffle(lhs, rhs, 0, 4), TypeError);
> +    } else if (lanes == 8) {

(order here too)

::: js/src/tests/ecma_7/SIMD/unary-operations.js
@@ +110,5 @@
>  
> +function testInt8x16neg() {
> +  var vals = [
> +    [[1, 2, 3, 4, 5, 6, 7, 8, -1, -2, -3, -4, -5, INT8_MAX, INT8_MIN, 0],
> +     [-1, -2, -3, -4, -5, -6, -7, -8, 1, 2, 3, 4, 5, -INT8_MAX << 24 >> 24, -INT8_MIN << 24 >> 24, 0]]

I think -INT8_MAX === INT8_MIN, right? And -INT8_MIN === INT8_MAX?

@@ +130,5 @@
> +
> +function testInt16x8neg() {
> +  var vals = [
> +    [[1, 2, 3, -1, -2, 0, INT16_MIN, INT16_MAX],
> +     [-1, -2, -3, 1, 2, 0, -INT16_MIN << 16 >> 16, -INT16_MAX << 16 >> 16]]

ditto?

Comment 43

[:ProgramFOX]

Attachment: Added ToInt8 and ToInt16

Rebased patch for ToInt8 and ToInt16. Didn't require changes to the ToInt8 and ToInt16 methods itself, but they couldn't be auto-applied anymore.

Comment 44

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

New patch, with the remarks fixed. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d311eeea6319

> assert(laneIndex <= arr.length);

The assert method didn't exist, so I used assertEq with `true` as second argument.

> I wouldn't worry about the outType (and remove it from the list of arguments, plus the associated assertion). Bool vector types will fix this in the future.

I believe it's necessary because the outType of a float64x2 comparison is int32x4, so it has to be there.

> I think -INT8_MAX === INT8_MIN, right? And -INT8_MIN === INT8_MAX?

My testing told me that -INT8_MIN === INT8_MIN and -INT8_MAX === -INT8_MAX (no special case for MAX), so I changed the values accordingly.

Comment 45

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8627213 [details] [diff] [review]
implemented-int16x8-8x16-11.diff

Review of attachment 8627213 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the new tests patch! Can you please:
- fix the one comment here, that triggered some assertions in the builds
- do another round of try, but also select jit-test (SIMD is tested in the jit-tests suite as well). That would be "jittests" on the trychooser website [0]
I'll review the other patch today.

[0] http://trychooser.pub.build.mozilla.org/

::: js/src/builtin/TypedObject.js
@@ +573,5 @@
> +  else if (length == 8)
> +    return `${protoString}(${this.s0},${this.s1},${this.s2},${this.s3},${this.s4},${this.s5},${this.s6},${this.s7})`;
> +  else if (length == 16)
> +    return `${protoString}(${this.s0},${this.s1},${this.s2},${this.s3},${this.s4},${this.s5},${this.s6},${this.s7},
> +                           ${this.s8},${this.s9},${this.s10},${this.s11},${this.s12},${this.s13},${this.s14},${this.s15})`;

That wasn't obvious, but the spacing changed here. I'd like to keep spaces for readability, so can you add them back, please? (r+ on that change)

Comment 46

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8628988 [details] [diff] [review]
Added test cases for int16x8 and int8x16

Review of attachment 8628988 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thank you! Can you try this patch a last time, please? I think int16x8bool should be failing, because of the BUGNUMBER / summary issue (unless it's defined somewhere else...).
Please fix the remaining issues (no need to ask again for r+), and when try is back and all green, you can set the "checkin-needed" keyword and have these two patches landed! (feel free to do so during the weekend if you're up to!)
Congratulations on all this work, these were pretty big changes!

::: js/src/tests/ecma_7/SIMD/binary-operations.js
@@ +140,5 @@
>  
> +var i8x16vals = [
> +  [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16],
> +   [10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 110, 120, 130, 140, 150, 160]],
> +  [[INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, INT8_MAX, INT8_MIN, 0, -1, -2, -3, -4, -5, -6, -7, -8, -9],

can you change the 2 next values after the last INT8_MIN to INT8_MAX and INT8_MIN, please?

::: js/src/tests/ecma_7/SIMD/int16x8bool.js
@@ +6,5 @@
> + * https://creativecommons.org/publicdomain/zero/1.0/
> + */
> +
> +function test() {
> +  print(BUGNUMBER + ": " + summary);

don't forget to remove this line, otherwise this will fail. I think the tests should have failed. Can you make sure to run another round of try, please?

::: js/src/tests/ecma_7/SIMD/load.js
@@ +155,5 @@
>          C.load3(3);
>          C.load3(lastValidArgLoad3);
>  
> +        assertThrowsInstanceOf(() => SIMD[kind].load(ta, lastValidArgLoad + 1), RangeError);
> +        if (lanes < 8) {

i'd prefer <= 4 here, but that's fairly about personal taste :)

::: js/src/tests/ecma_7/SIMD/swizzle-shuffle.js
@@ +73,5 @@
> +        for (var i = 0; i < Math.pow(2, 2); i++) {
> +          [x, y] = [x & 1, (y >> 1) & 1];
> +          assertEqVec(type.swizzle(v, x, y), swizzle2(simdToArray(v), x, y));
> +        }
> +    } 

nit: trailing whitespace

@@ +112,5 @@
> +
> +        // In bounds is [0, 1]
> +        assertThrowsInstanceOf(() => type.swizzle(v, 0, -1), TypeError);
> +        assertThrowsInstanceOf(() => type.swizzle(v, 0, 2), TypeError);
> +    } 

nit: trailing whitespace

Comment 47

[Benjamin Bouvier [:bbouvier] (inactive)]

For what it's worth, BUGNUMBER is actually defined in tests/shell.js (thanks jandem!), so no need for a new try run.

Comment 48

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Updated patch, with the spacing in TypedObject.js fixed.

Comment 49

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

Updated test cases.

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1e86c8154826

Comment 50

[:ProgramFOX]

Attachment: Implemented int8x16 and int16x8

Doing some testing on the shell revealed that the .toSource for int8x16 was broken because of the newline I added to the source-string in TypedObject.js. I removed it here to fix the issue.

New try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa62c299064c

Comment 51

[:ProgramFOX]

The try push returned green (after retriggering Jit2), adding checkin-needed.

Comment 52

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6c24695543a9
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1b878ce28f4

Comment 53

[Carsten Book [:Tomcat]]

sorry had to back this out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=11476998&repo=mozilla-inbound

does this need all 3 patches from this bug (the last try run had only the last 2 that why i ask)

Comment 54

[Benjamin Bouvier [:bbouvier] (inactive)]

(In reply to Carsten Book [:Tomcat] from comment #53)
> sorry had to back this out for bustage like
> https://treeherder.mozilla.org/logviewer.html#?job_id=11476998&repo=mozilla-
> inbound
> 
> does this need all 3 patches from this bug (the last try run had only the
> last 2 that why i ask)

Yes please. Sorry for not having added this information somewhere in a comment.

Comment 55

[:ProgramFOX]

Also I just noticed that I forgot the r=bbouvier on the test cases patch; I'll add it now before it gets checked in again.

Comment 56

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bd6656254d6
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef9373b8da69

Comment 57

[:ProgramFOX]

Attachment: Added test cases for int16x8 and int8x16

Same test cases, but now with r=bbouvier appended to the commit message. Sorry for noticing this just now!

Comment 58

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/51c6f51151a7
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bd880877d2d
https://hg.mozilla.org/integration/mozilla-inbound/rev/1566eb1070e8

Comment 59

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/51c6f51151a7
https://hg.mozilla.org/mozilla-central/rev/7bd880877d2d
https://hg.mozilla.org/mozilla-central/rev/1566eb1070e8

Comment 60

[Florian Scholz (Open Web Docs)]

Documentation:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Int8x16
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Int16x8

Also mentioned in the relevant method pages under 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SIMD

(Constructors are capitalized as per bug 1173722)