Add new Float32x4 type for SIMD.js.

Add new Float32x4 type for SIMD.js.

LOG=N
BUG=v8:4124
Committed: https://crrev.com/f9dd3446da82fbbf20c344ceb6a87be2c11f5759
Cr-Commit-Position: refs/heads/master@{#28797}

[bbudge, 2015-06-03 00:57:02]

Patchset #1 (id:1) has been deleted

[bbudge, 2015-06-03 00:57:11]

Patchset #1 (id:20001) has been deleted

[bbudge, 2015-06-03 01:12:11]

bbudge@chromium.org changed reviewers:
+ hpayer@chromium.org, titzer@chromium.org

[bbudge, 2015-06-03 01:12:12]

I could keep playing with this but I think I need some V8 expertise now.

[Hannes Payer (out of office), 2015-06-03 06:16:52]

hi bill, what the question?

[Hannes Payer (out of office), 2015-06-03 06:17:13]

On 2015/06/03 06:16:52, Hannes Payer wrote:
> hi bill, what the question?

what's

[Hannes Payer (out of office), 2015-06-03 06:20:33]

On 2015/06/03 06:17:13, Hannes Payer wrote:
> On 2015/06/03 06:16:52, Hannes Payer wrote:
> > hi bill, what the question?
> 
> what's

the heap/gc part looks good

[bbudge, 2015-06-03 12:12:43]

On 2015/06/03 06:16:52, Hannes Payer wrote:
> hi bill, what the question?

I guess my questions are, have I left any part out, and is there anything that
is not needed?

I derived from HeapObject and mimicked HeapNumber where it made sense, until the
code compiled and I could run tests.

Is there more that's needed?

I added heap snapshot stuff (mainly an enum value). Is this needed?

Maybe it's ready for review? I sent it to you and Ben but feel free to reassign
as appropriate.

[titzer, 2015-06-03 13:27:15]

https://codereview.chromium.org/1153373003/diff/140001/src/factory.h
File src/factory.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/factory.h#newcode355
src/factory.h:355: Handle<Float32x4> NewFloat32x4(double w, double x, double y,
double z,
Should we use float here to force the rounding into the caller?

https://codereview.chromium.org/1153373003/diff/140001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/heap/heap.h#newcod...
src/heap/heap.h:1610: AllocateFloat32x4(double w, double x, double y, double z,
Float

https://codereview.chromium.org/1153373003/diff/140001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/objects-inl.h#newc...
src/objects-inl.h:1587: DCHECK(lane < 4 && lane >= 0);
Looks like we are only supporting little endian for now. I suppose that's OK.

https://codereview.chromium.org/1153373003/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1153373003/diff/140001/src/objects.cc#newcode100
src/objects.cc:100: if (IsFloat32x4()) return
Float32x4::cast(this)->Float32x4BooleanValue();
Maybe it's better just to return true for now here, since the rest of the system
will treat all objects as true. There'll be a lot of work to make the
value-objectness work correctly in V8, so let's just be consistent for now.

https://codereview.chromium.org/1153373003/diff/140001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/objects.h#newcode1621
src/objects.h:1621: inline double get_lane(int lane) const;
I think we should float here too.

https://codereview.chromium.org/1153373003/diff/140001/src/objects.h#newcode1634
src/objects.h:1634: #if defined(V8_TARGET_LITTLE_ENDIAN)
Can we leave these out of the header to force users to go through the get_lane()
and set_lane() methods?

https://codereview.chromium.org/1153373003/diff/140001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/1153373003/diff/140001/test/cctest/test-heap....
test/cctest/test-heap.cc:187: value = factory->NewFloat32x4(1, 2, 3, 4);
Can we try some more floating point values, and also -0.0 and NaN values?

You can write your own method with lots of example values and exhaustive reading
and writing of the individual lanes.

[bbudge, 2015-06-03 20:31:49]

PTAL

https://codereview.chromium.org/1153373003/diff/140001/src/factory.h
File src/factory.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/factory.h#newcode355
src/factory.h:355: Handle<Float32x4> NewFloat32x4(double w, double x, double y,
double z,
On 2015/06/03 13:27:15, titzer wrote:
> Should we use float here to force the rounding into the caller?

Yes, much better.

https://codereview.chromium.org/1153373003/diff/140001/src/heap/heap.h
File src/heap/heap.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/heap/heap.h#newcod...
src/heap/heap.h:1610: AllocateFloat32x4(double w, double x, double y, double z,
On 2015/06/03 13:27:15, titzer wrote:
> Float

Done.

https://codereview.chromium.org/1153373003/diff/140001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/objects-inl.h#newc...
src/objects-inl.h:1587: DCHECK(lane < 4 && lane >= 0);
On 2015/06/03 13:27:15, titzer wrote:
> Looks like we are only supporting little endian for now. I suppose that's OK.

I guess there aren't any big-endian media instruction sets? However, it's easy
enough to support here. Done.

https://codereview.chromium.org/1153373003/diff/140001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1153373003/diff/140001/src/objects.cc#newcode100
src/objects.cc:100: if (IsFloat32x4()) return
Float32x4::cast(this)->Float32x4BooleanValue();
On 2015/06/03 13:27:15, titzer wrote:
> Maybe it's better just to return true for now here, since the rest of the
system
> will treat all objects as true. There'll be a lot of work to make the
> value-objectness work correctly in V8, so let's just be consistent for now.

Done. (I'll remove the Float32x4BooleanValue() function too.)

https://codereview.chromium.org/1153373003/diff/140001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/1153373003/diff/140001/src/objects.h#newcode1621
src/objects.h:1621: inline double get_lane(int lane) const;
On 2015/06/03 13:27:15, titzer wrote:
> I think we should float here too.

Done.

https://codereview.chromium.org/1153373003/diff/140001/src/objects.h#newcode1634
src/objects.h:1634: #if defined(V8_TARGET_LITTLE_ENDIAN)
On 2015/06/03 13:27:15, titzer wrote:
> Can we leave these out of the header to force users to go through the
get_lane()
> and set_lane() methods?

Done (I just deleted them, since they're not used anywhere and I'm not sure
they're needed.)

https://codereview.chromium.org/1153373003/diff/140001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/1153373003/diff/140001/test/cctest/test-heap....
test/cctest/test-heap.cc:187: value = factory->NewFloat32x4(1, 2, 3, 4);
On 2015/06/03 13:27:15, titzer wrote:
> Can we try some more floating point values, and also -0.0 and NaN values?
> 
> You can write your own method with lots of example values and exhaustive
reading
> and writing of the individual lanes.

Done.

[titzer, 2015-06-03 20:34:41]

LGTM with one last comment on test.

https://codereview.chromium.org/1153373003/diff/160001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/1153373003/diff/160001/test/cctest/test-heap....
test/cctest/test-heap.cc:248: CHECK(std::isnan(float32x4->get_lane(2)));
Can we also check that writing to one lane doesn't overwrite the other lanes?

[bbudge, 2015-06-03 21:32:52]

https://codereview.chromium.org/1153373003/diff/160001/test/cctest/test-heap.cc
File test/cctest/test-heap.cc (right):

https://codereview.chromium.org/1153373003/diff/160001/test/cctest/test-heap....
test/cctest/test-heap.cc:248: CHECK(std::isnan(float32x4->get_lane(2)));
On 2015/06/03 20:34:40, titzer wrote:
> Can we also check that writing to one lane doesn't overwrite the other lanes?

Done. I added a templated function to do the lane tests, since this will be
needed by all types and is regular. I tried to do it without adding anything
weird to the Float32x4 definition.

[bbudge, 2015-06-03 23:54:20]

The CQ bit was checked by bbudge@chromium.org

[bbudge, 2015-06-03 23:54:21]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1153373003/#ps220001
(title: "Fix int type mismatches.")

[commit-bot: I haz the power, 2015-06-03 23:54:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1153373003/220001

[commit-bot: I haz the power, 2015-06-03 23:56:27]

Committed patchset #10 (id:220001)

[commit-bot: I haz the power, 2015-06-03 23:56:47]

Patchset 10 (id:??) landed as
https://crrev.com/f9dd3446da82fbbf20c344ceb6a87be2c11f5759
Cr-Commit-Position: refs/heads/master@{#28797}