Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(751)

Issue 1160443009: Add SIMD.Float32x4 functions. (Closed)

Created:
5 years, 6 months ago by bbudge
Modified:
5 years, 4 months ago
Reviewers:
rossberg
CC:
v8-dev, titzer
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Add all SIMD.float32x4 functions that can be implemented without the other SIMD types. constructor, splat constructor check extractLane, replaceLane abs, neg, sqrt, reciprocalApproximation, reciprocalSqrtApproximation add, sub, mul, div, min, max, minNum, maxNum swizzle, shuffle Includes a patch to the test harness for console.log output. LOG=N BUG=v8:4124

Patch Set 1 : Constructors, lane functions, add function. #

Patch Set 2 : #

Patch Set 3 : All functions that can be implemented right now. #

Patch Set 4 : Fix compile. #

Patch Set 5 : Fix compile. #

Total comments: 3

Patch Set 6 : Debugging. #

Patch Set 7 : Add test file. #

Patch Set 8 : #

Patch Set 9 : Tests working. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1032 lines, -41 lines) Patch
M BUILD.gn View 1 2 3 4 5 6 7 2 chunks +3 lines, -1 line 0 comments Download
M include/v8.h View 1 2 3 4 5 6 7 6 chunks +51 lines, -1 line 0 comments Download
M src/api.h View 1 2 3 4 5 6 7 3 chunks +4 lines, -0 lines 0 comments Download
M src/api.cc View 1 2 3 4 5 6 7 6 chunks +57 lines, -0 lines 0 comments Download
M src/bootstrapper.cc View 1 2 3 4 5 6 7 8 4 chunks +26 lines, -0 lines 0 comments Download
M src/code-stubs.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M src/code-stubs.cc View 1 2 3 4 5 6 7 8 3 chunks +8 lines, -4 lines 0 comments Download
M src/code-stubs-hydrogen.cc View 1 2 3 4 5 6 7 1 chunk +20 lines, -12 lines 0 comments Download
M src/contexts.h View 1 2 3 4 5 6 7 2 chunks +4 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M src/globals.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
A src/harmony-simd.js View 1 2 3 4 5 6 7 8 1 chunk +171 lines, -0 lines 0 comments Download
M src/heap-snapshot-generator.cc View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M src/heap/heap.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M src/hydrogen-instructions.cc View 1 2 3 4 5 6 7 2 chunks +4 lines, -1 line 0 comments Download
M src/ia32/code-stubs-ia32.cc View 1 2 3 4 5 6 7 8 1 chunk +3 lines, -0 lines 0 comments Download
M src/ia32/full-codegen-ia32.cc View 1 2 3 4 5 6 7 1 chunk +4 lines, -0 lines 0 comments Download
M src/ia32/lithium-codegen-ia32.cc View 1 2 3 4 5 6 7 8 2 chunks +11 lines, -0 lines 0 comments Download
M src/ic/handler-compiler.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M src/ic/ic-inl.h View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M src/macros.py View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 0 comments Download
M src/messages.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M src/messages.js View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M src/mirror-debugger.js View 1 2 3 4 5 6 7 7 chunks +32 lines, -0 lines 0 comments Download
M src/objects.h View 1 2 3 4 5 6 7 4 chunks +2 lines, -2 lines 0 comments Download
M src/objects.cc View 1 2 3 4 5 6 7 3 chunks +6 lines, -1 line 0 comments Download
M src/objects-inl.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -9 lines 0 comments Download
M src/runtime.js View 1 2 3 4 5 6 7 8 9 chunks +10 lines, -6 lines 0 comments Download
M src/runtime/runtime.h View 1 2 3 4 5 6 7 2 chunks +24 lines, -0 lines 0 comments Download
A src/runtime/runtime-simd.cc View 1 2 3 4 5 6 7 8 1 chunk +298 lines, -0 lines 0 comments Download
M src/type-info.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M src/types.cc View 1 2 3 4 5 6 7 2 chunks +4 lines, -0 lines 0 comments Download
A test/mjsunit/harmony/simd.js View 1 2 3 4 5 6 7 8 1 chunk +258 lines, -0 lines 0 comments Download
M test/mjsunit/messages.js View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M test/simdjs/harness-adapt.js View 1 chunk +5 lines, -0 lines 0 comments Download
M tools/gyp/v8.gyp View 1 2 3 4 5 6 7 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (8 generated)
bbudge
5 years, 6 months ago (2015-06-11 22:56:58 UTC) #5
rossberg
5 years, 6 months ago (2015-06-12 11:48:03 UTC) #6
Mostly looking good, except that it is missing plenty of tests. Some features
are obviously not working yet (e.g. equality, typeof, etc), but for the value
behaviour that this CL implements we need tests (in
test/mjsunit/harmony/simd.js), e.g.:

- variants of constructor calls
- prototype identities
- object wrapping
- ClassOf
- explicit & implicit conversions (especially to primitive types)

See e.g. the first half of test/mjsunit/es6/symbols.js for guidance.

I assume the actual SIMD methods are already tested in the SIMD suite.

https://codereview.chromium.org/1160443009/diff/140001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/1160443009/diff/140001/include/v8.h#newcode3836
include/v8.h:3836: class V8_EXPORT Float32x4 : public Object {
SIMDs aren't objects, so this needs to derive from Value.

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

https://codereview.chromium.org/1160443009/diff/140001/src/harmony-simd.js#ne...
src/harmony-simd.js:113: %FunctionSetPrototype(GlobalFloat32x4, new
GlobalObject());
Nit: s/new GlobalObject()/{}/

https://codereview.chromium.org/1160443009/diff/140001/src/harmony-simd.js#ne...
src/harmony-simd.js:119: %AddNamedProperty(GlobalFloat32x4, symbolToStringTag,
'float32x4', READ_ONLY | DONT_ENUM);
It seems inconsistent that the tag is lower-case, it is upper for all other
types (this is the name of the wrapper type).

Also nit: line length

Powered by Google App Engine
This is Rietveld 408576698