Add Float64x2, Float64x2List, etc... with runtime and dart2js implementations

Add Float64x2, Float64x2List, etc... with runtime and dart2js implementations

R=sra@google.com, srdjan@google.com

Committed: https://code.google.com/p/dart/source/detail?r=32196

[srdjan, 2014-01-29 23:49:41]

LGTM for the VM changes.

https://codereview.chromium.org/148043003/diff/1/runtime/lib/simd128.cc
File runtime/lib/simd128.cc (right):

https://codereview.chromium.org/148043003/diff/1/runtime/lib/simd128.cc#newco...
runtime/lib/simd128.cc:66: float _y = v.y();
Cast explicitly

https://codereview.chromium.org/148043003/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/148043003/diff/1/runtime/vm/object.h#newcode5954
runtime/vm/object.h:5954: Heap::Space space = Heap::kNew);
Fix indent.

[sra1, 2014-01-30 02:08:08]

https://codereview.chromium.org/148043003/diff/1/sdk/lib/typed_data/dart2js/t...
File sdk/lib/typed_data/dart2js/typed_data_dart2js.dart (right):

https://codereview.chromium.org/148043003/diff/1/sdk/lib/typed_data/dart2js/t...
sdk/lib/typed_data/dart2js/typed_data_dart2js.dart:1069: final int
elementSizeInBytes = 16;
Shouldn't it be a getter rather than a stored field, e.g.

int get elementSizeInBytes => BYTES_PER_ELEMENT;

https://codereview.chromium.org/148043003/diff/1/sdk/lib/typed_data/dart2js/t...
sdk/lib/typed_data/dart2js/typed_data_dart2js.dart:1145: : _storage = new
Float64List.view(buffer, byteOffset, length);
The storage view should have length*2 elements since it has elementSizeInByte of
a half of the containing type.

Add a test for this first, then fix it :-)
You probably have to fix Int32x4List.view and friends too.

https://codereview.chromium.org/148043003/diff/1/tests/lib/typed_data/float64...
File tests/lib/typed_data/float64x2_functional_test.dart (right):

https://codereview.chromium.org/148043003/diff/1/tests/lib/typed_data/float64...
tests/lib/typed_data/float64x2_functional_test.dart:210: testTypedListView();
Add test for sublist.
Add test for Float64x2List.fromList(x) where x is a Float64x2List.

[Cutch, 2014-01-30 16:06:52]

https://codereview.chromium.org/148043003/diff/1/sdk/lib/typed_data/dart2js/t...
File sdk/lib/typed_data/dart2js/typed_data_dart2js.dart (right):

https://codereview.chromium.org/148043003/diff/1/sdk/lib/typed_data/dart2js/t...
sdk/lib/typed_data/dart2js/typed_data_dart2js.dart:1069: final int
elementSizeInBytes = 16;
On 2014/01/30 02:08:09, sra1 wrote:
> Shouldn't it be a getter rather than a stored field, e.g.
> 
> int get elementSizeInBytes => BYTES_PER_ELEMENT;

Done here and elsewhere.

https://codereview.chromium.org/148043003/diff/1/sdk/lib/typed_data/dart2js/t...
sdk/lib/typed_data/dart2js/typed_data_dart2js.dart:1145: : _storage = new
Float64List.view(buffer, byteOffset, length);
On 2014/01/30 02:08:09, sra1 wrote:
> The storage view should have length*2 elements since it has elementSizeInByte
of
> a half of the containing type.
> 
> Add a test for this first, then fix it :-)
> You probably have to fix Int32x4List.view and friends too.

Test added and fixed elsewhere.

https://codereview.chromium.org/148043003/diff/1/tests/lib/typed_data/float64...
File tests/lib/typed_data/float64x2_functional_test.dart (right):

https://codereview.chromium.org/148043003/diff/1/tests/lib/typed_data/float64...
tests/lib/typed_data/float64x2_functional_test.dart:210: testTypedListView();
On 2014/01/30 02:08:09, sra1 wrote:
> Add test for sublist.
> Add test for Float64x2List.fromList(x) where x is a Float64x2List.

Done.

[Cutch, 2014-01-30 18:27:26]

https://codereview.chromium.org/148043003/diff/1/runtime/lib/simd128.cc
File runtime/lib/simd128.cc (right):

https://codereview.chromium.org/148043003/diff/1/runtime/lib/simd128.cc#newco...
runtime/lib/simd128.cc:66: float _y = v.y();
On 2014/01/29 23:49:42, srdjan wrote:
> Cast explicitly

Done.

https://codereview.chromium.org/148043003/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/148043003/diff/1/runtime/vm/object.h#newcode5954
runtime/vm/object.h:5954: Heap::Space space = Heap::kNew);
On 2014/01/29 23:49:42, srdjan wrote:
> Fix indent.

Done.

[sra1, 2014-01-30 23:44:39]

lgtm

Thanks!

[Cutch, 2014-01-31 15:28:17]

Committed patchset #4 manually as r32196 (presubmit successful).

[Lasse Reichstein Nielsen, 2014-01-31 17:51:09]

Late comment drive-by.
I can look at doing some of the documentation myself on Monday.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
File sdk/lib/typed_data/typed_data.dart (right):

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:877: * all of whose elements are initially
zero.
zero on each entry
?

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:882: * Creates a [Float64x2List] with the
same size as the [elements] list
size -> length

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:900: * BYTES_PER_ELEMENT.
-> [BYTES_PER_ELEMENT]
(or `BYTES_PER_ELEMENT` if you don't want a link, but I think it should be a
link)

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:919: external factory
Float32x4.fromFloat64x2(Float64x2 v);
Needs documentation. It is not obvious what the resulting Float32x4 contains -
guessing (v.x, v.y, 0.0, 0.0)?

Should we have Float32x4.fromFloat64x2s(Float64x2 xy, Float64x2 zw)?

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1590: * Interface of Dart Float64x2 immutable
value type and operations.
First line is redundant with being an abstract class called Float64x2.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1598: external factory
Float64x2.fromFloat32x4(Float32x4 v);
Also needs documentation.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1600: /// Addition operator.
Specify that it adds the doubles lane-wise.
(A DartDoc comment that just says "addition operator" for operator+ is redundant
- it might as well not be documented).

Ditto below.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1611: /// Returns a copy of [this] each lane
being scaled by [s].
So this is equivalent to  this * new Float64x2.splat(s) ?
Might be worth saying.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1613: /// Returns the absolute value of this
[Float64x2].
... the lane-wise absolute value ...

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1616: /// Clamps [this] to be in the range
[lowerLimit]-[upperLimit].
lane-wise.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1625: /// Extract the sign bits from each
lane return them in the first 2 bits.
Specify order. I assume "x" is bit zero and "y" is bit one?

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1640: Float64x2 sqrt();
How about having:
  static Float64x2 sincos(double radian);
? I could see the use for that :)

[Cutch, 2014-01-31 19:36:13]

Doc fixes in this CL: https://codereview.chromium.org/144863003/

Lasse, please note that the typed_data classes are documented in two places (the
interface and the dart2js implementation) and I believe the doc generator
consumes the dart2js sources.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
File sdk/lib/typed_data/typed_data.dart (right):

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:877: * all of whose elements are initially
zero.
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> zero on each entry
> ?

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:882: * Creates a [Float64x2List] with the
same size as the [elements] list
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> size -> length

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:900: * BYTES_PER_ELEMENT.
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> -> [BYTES_PER_ELEMENT]
> (or `BYTES_PER_ELEMENT` if you don't want a link, but I think it should be a
> link)

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:919: external factory
Float32x4.fromFloat64x2(Float64x2 v);
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> Needs documentation. It is not obvious what the resulting Float32x4 contains -
> guessing (v.x, v.y, 0.0, 0.0)?
> 
> Should we have Float32x4.fromFloat64x2s(Float64x2 xy, Float64x2 zw)?

Done. I'm not sure if we should have Float32x4.fromFloat64x2s because there
aren't CPU instructions which do that quickly.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1590: * Interface of Dart Float64x2 immutable
value type and operations.
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> First line is redundant with being an abstract class called Float64x2.

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1598: external factory
Float64x2.fromFloat32x4(Float32x4 v);
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> Also needs documentation.

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1611: /// Returns a copy of [this] each lane
being scaled by [s].
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> So this is equivalent to  this * new Float64x2.splat(s) ?
> Might be worth saying.

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1625: /// Extract the sign bits from each
lane return them in the first 2 bits.
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> Specify order. I assume "x" is bit zero and "y" is bit one?

Done.

https://codereview.chromium.org/148043003/diff/60001/sdk/lib/typed_data/typed...
sdk/lib/typed_data/typed_data.dart:1640: Float64x2 sqrt();
On 2014/01/31 17:51:10, Lasse Reichstein Nielsen wrote:
> How about having:
>   static Float64x2 sincos(double radian);
> ? I could see the use for that :)

Will discuss with Srdjan and add that in a follow up CL.