I macro-ized SIMD types in C++ and JS to reduce repetition.
objects.h defines SIMD128_TYPES in a V8 style, and uses it to define the object
types. I moved some methods off Float32x4, which belonged on the Object::
methods any way.
heap and bootstrapper now use SIMD128_TYPES.



https://codereview.chromium.org/1250733005/diff/210001/src/harmony-simd.js
File src/harmony-simd.js (right):

https://codereview.chromium.org/1250733005/diff/210001/src/harmony-simd.js#newcode35
src/harmony-simd.js:35: return %CreateFloat32x4(TO_NUMBER_INLINE(c0),
TO_NUMBER_INLINE(c1), TO_NUMBER_INLINE(c2), TO_NUMBER_INLINE(c3));
On 2015/07/29 14:37:55, rossberg wrote:
Nit: line length

Done.

https://codereview.chromium.org/1250733005/diff/210001/src/harmony-simd.js#newcode90
src/harmony-simd.js:90:
//-------------------------------------------------------------------
On 2015/07/29 14:37:55, rossberg wrote:
Would it be possible to avoid all the repetition by using a macro?
(See e.g.
typedarray.js)

Macros in JS? I didn't know! Done.

https://codereview.chromium.org/1250733005/diff/210001/src/heap/heap.cc
File src/heap/heap.cc (right):

https://codereview.chromium.org/1250733005/diff/210001/src/heap/heap.cc#newcode3119
src/heap/heap.cc:3119: AllocationResult Heap::AllocateFloat32x4(float
lanes[4],
On 2015/07/29 14:37:55, rossberg wrote:
Macrofication might help here, too.

Definitely. Done.

https://codereview.chromium.org/1250733005/diff/210001/src/macros.py
File src/macros.py (right):

https://codereview.chromium.org/1250733005/diff/210001/src/macros.py#newcode136
src/macros.py:136: macro IS_SIMD_VALUE(arg)         = (IS_FLOAT32X4(arg)
|| IS_INT32X4(arg) || IS_BOOL32X4(arg) || IS_INT16X8(arg) ||
IS_BOOL16X8(arg) || IS_INT8X16(arg) || IS_BOOL8X16(arg));
On 2015/07/29 14:37:55, rossberg wrote:
Hm, this seems rather expensive. It is used on some potentially
performance-relevant paths, so there should probably be a
%_IsSimdValue
intrinsic to make it efficient. I'm fine with doing that in a
follow-up CL, but
better be prepared that landing the CL as is might affect some
benchmarks.

I don't like those perf bugs, so I'll use your suggestion now. Done.

https://codereview.chromium.org/1250733005/diff/210001/src/macros.py#newcode137
src/macros.py:137: macro IS_SIMD_VALUE_WRAPPER(arg) =
(IS_FLOAT32X4_WRAPPER(arg) || IS_INT32X4_WRAPPER(arg) ||
IS_BOOL32X4_WRAPPER(arg) || IS_INT16X8_WRAPPER(arg) ||
IS_BOOL16X8_WRAPPER(arg) || IS_INT8X16_WRAPPER(arg) ||
IS_BOOL8X16_WRAPPER(arg));
On 2015/07/29 14:37:55, rossberg wrote:
Is this actually used?

Nope. Deleted.

https://codereview.chromium.org/1250733005/diff/210001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1250733005/diff/210001/src/objects.cc#newcode102
src/objects.cc:102: return MaybeHandle<JSReceiver>();
On 2015/07/29 14:37:55, rossberg wrote:
Nit: the return here is redundant.

Done.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js
File src/runtime.js (right):

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode106
src/runtime.js:106: if (IS_SYMBOL(y) || IS_SIMD_VALUE(y)) return 1;  //
not equal
I moved these primitive checks which always return "not equal" to the
end of their blocks, since they shouldn't be encountered in real or
benchmark code.  WDYT?

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode116
src/runtime.js:116: if (IS_SYMBOL(y) || IS_SIMD_VALUE(y)) return 1;  //
not equal
Made this %IsSimdValue call and moved it to the end of this block with
the IS_SYMBOL case.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode136
src/runtime.js:136: } else if (IS_FLOAT32X4(x)) {
On 2015/07/29 14:37:55, rossberg wrote:
Guard these by a (cheaper) IS_SIMD_VALUE test.

Done. Using %IsSimdValue intrinsic. Also, moved all the type checking
into a SimdEquals intrinsic.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode170
src/runtime.js:170: if (IS_SYMBOL(y) || IS_SIMD_VALUE(y)) return 1;  //
not equal
Moved this to after the IS_BOOLEAN case.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode189
src/runtime.js:189: if (IS_FLOAT32X4(this) && IS_FLOAT32X4(x))
On 2015/07/29 14:37:55, rossberg wrote:
Same here.

Done.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode884
src/runtime.js:884: if (IS_BOOL8X16(x)) return %NewBool8x16Wrapper(x);
Did the same thing here too, with the %IsSimdValue and a %SimdToObject
intrinsic.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode932
src/runtime.js:932: if (IS_FLOAT32X4(x)) return %Float32x4SameValue(x,
y);
On 2015/07/29 14:37:55, rossberg wrote:
And here.

Done.

https://codereview.chromium.org/1250733005/diff/210001/src/runtime.js#newcode949
src/runtime.js:949: if (IS_FLOAT32X4(x)) return
%Float32x4SameValueZero(x, y);
On 2015/07/29 14:37:56, rossberg wrote:
And here.

Done.

https://codereview.chromium.org/1250733005/diff/210001/test/cctest/test-simd.cc
File test/cctest/test-simd.cc (right):

https://codereview.chromium.org/1250733005/diff/210001/test/cctest/test-simd.cc#newcode14
test/cctest/test-simd.cc:14: TEST(SameValue) {
On 2015/07/29 14:37:56, rossberg wrote:
Any reason not to test the other types, too?

I just forgot about these. Done.

https://codereview.chromium.org/1250733005/diff/210001/test/mjsunit/harmony/simd.js
File test/mjsunit/harmony/simd.js (right):

https://codereview.chromium.org/1250733005/diff/210001/test/mjsunit/harmony/simd.js#newcode277
test/mjsunit/harmony/simd.js:277: // SIMD values should not be equal to
any other kind of object.
On 2015/07/29 14:37:56, rossberg wrote:
Can you add tests involving two different SIMD types?

Good idea. Done.

https://codereview.chromium.org/1250733005/diff/210001/test/mjsunit/harmony/simd.js#newcode324
test/mjsunit/harmony/simd.js:324: function TestSameValue(type, lanes) {
On 2015/07/29 14:37:56, rossberg wrote:
Same here.

Done.

https://codereview.chromium.org/1250733005/diff/210001/test/mjsunit/harmony/simd.js#newcode371
test/mjsunit/harmony/simd.js:371: var a = createInstance(type), b =
createInstance(type);
On 2015/07/29 14:37:56, rossberg wrote:
Again it would be good to check the matrix of SIMD types, and also
include a
couple of non-SIMD values on either side.

Done.

https://codereview.chromium.org/1250733005/diff/210001/test/mjsunit/harmony/simd.js#newcode423
test/mjsunit/harmony/simd.js:423: test(set, instance);
On 2015/07/29 14:37:56, rossberg wrote:
This now only tests for a zeroed value, which seems a bit limited.

I changed createInstance to create slightly more interesting values.

https://codereview.chromium.org/1250733005/diff/210001/test/mjsunit/harmony/simd.js#newcode451
test/mjsunit/harmony/simd.js:451: test(map, instance, {});
On 2015/07/29 14:37:56, rossberg wrote:
Similarly here.

Done.

https://codereview.chromium.org/1250733005/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to