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

Issue 1991143002: Convert SIMD wasm ops to runtime function calls (Closed)

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

Description

Convert SIMD wasm ops to runtime function calls - Add Simd128 type to Wasm AST types - Decode SIMD prefix, wasm opcodes correctly - Add a pass that converts SIMD machine ops to runtime calls - Sample opcodes Int32x4Splat, Int32x4ExtractLane and test LOG=N BUG=v8:4124 R=bradnelson@chromium.org, bbudge@chromium.org, titzer@chromium.org Committed: https://crrev.com/73df92fc2fdbbfadc17e8ab4e58ec56ae2b3d91a Committed: https://crrev.com/18543ff1da34a7b12f583375b53320fcf5013931 Cr-Original-Commit-Position: refs/heads/master@{#37789} Cr-Commit-Position: refs/heads/master@{#37807}

Patch Set 1 #

Total comments: 6

Patch Set 2 : Bill's review #

Total comments: 18

Patch Set 3 : Rebase, handle simd prefix correctly #

Patch Set 4 : Split test, trigger sequential compile for simd tests #

Patch Set 5 : Fix test #

Patch Set 6 : rebase to fix broken tests #

Patch Set 7 : Review changes #

Total comments: 9

Patch Set 8 : Bill's review #

Patch Set 9 : Cleanup #

Total comments: 12

Patch Set 10 : Review changes #

Patch Set 11 : Fix bot build #

Patch Set 12 : Use NodeVector instead of std::vector #

Total comments: 4

Patch Set 13 : Add --enable-wasm-simd flag #

Patch Set 14 : Rebase #

Total comments: 6

Patch Set 15 : Ben's review #

Patch Set 16 : Rebase #

Patch Set 17 : rebase #

Patch Set 18 : Fix bot fails #

Unified diffs Side-by-side diffs Delta from patch set Stats (+453 lines, -5 lines) Patch
M BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -0 lines 0 comments Download
A src/compiler/simd-lowering.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +46 lines, -0 lines 0 comments Download
A src/compiler/simd-lowering.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +116 lines, -0 lines 0 comments Download
M src/compiler/wasm-compiler.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 8 chunks +15 lines, -0 lines 0 comments Download
M src/compiler/wasm-compiler.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 6 chunks +120 lines, -0 lines 0 comments Download
M src/flag-definitions.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -0 lines 0 comments Download
M src/v8.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -0 lines 0 comments Download
M src/wasm/ast-decoder.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +20 lines, -0 lines 0 comments Download
M src/wasm/wasm-macro-gen.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +7 lines, -0 lines 0 comments Download
M src/wasm/wasm-opcodes.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +5 lines, -0 lines 0 comments Download
M src/wasm/wasm-opcodes.cc View 1 2 3 4 5 6 7 8 9 1 chunk +23 lines, -3 lines 0 comments Download
M test/cctest/cctest.gyp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +2 lines, -1 line 0 comments Download
A test/cctest/wasm/test-run-wasm-simd.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +60 lines, -0 lines 0 comments Download
M test/cctest/wasm/test-signatures.h View 5 chunks +7 lines, -1 line 0 comments Download
A test/mjsunit/wasm/test-wasm-simd-ops.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +23 lines, -0 lines 0 comments Download
M test/mjsunit/wasm/wasm-constants.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 46 (20 generated)
gdeepti1
4 years, 7 months ago (2016-05-19 08:58:53 UTC) #2
bbudge
https://codereview.chromium.org/1991143002/diff/1/src/compiler/simd-lowering.h File src/compiler/simd-lowering.h (right): https://codereview.chromium.org/1991143002/diff/1/src/compiler/simd-lowering.h#newcode22 src/compiler/simd-lowering.h:22: explicit SimdLowering(JSGraph* jsgraph, WasmGraphBuilder* builder, explicit not necessary https://codereview.chromium.org/1991143002/diff/1/src/compiler/simd-lowering.h#newcode25 ...
4 years, 7 months ago (2016-05-19 09:14:28 UTC) #3
gdeepti
https://codereview.chromium.org/1991143002/diff/1/src/compiler/simd-lowering.h File src/compiler/simd-lowering.h (right): https://codereview.chromium.org/1991143002/diff/1/src/compiler/simd-lowering.h#newcode22 src/compiler/simd-lowering.h:22: explicit SimdLowering(JSGraph* jsgraph, WasmGraphBuilder* builder, On 2016/05/19 09:14:28, bbudge ...
4 years, 7 months ago (2016-05-19 09:34:09 UTC) #6
titzer
Can we split the opcode-related stuff into a separate CL? For opcodes, I think we'll ...
4 years, 7 months ago (2016-05-19 09:43:27 UTC) #7
bbudge
https://codereview.chromium.org/1991143002/diff/20001/src/compiler/simd-lowering.cc File src/compiler/simd-lowering.cc (right): https://codereview.chromium.org/1991143002/diff/20001/src/compiler/simd-lowering.cc#newcode67 src/compiler/simd-lowering.cc:67: kNone, kNone, kNone, kNone, kNone, kNone, kNone, kNone, \ ...
4 years, 7 months ago (2016-05-19 09:53:14 UTC) #8
titzer
https://codereview.chromium.org/1991143002/diff/20001/src/compiler/simd-lowering.cc File src/compiler/simd-lowering.cc (right): https://codereview.chromium.org/1991143002/diff/20001/src/compiler/simd-lowering.cc#newcode28 src/compiler/simd-lowering.cc:28: Conversion signature[] = { You can use "static const" ...
4 years, 7 months ago (2016-05-19 09:58:38 UTC) #9
gdeepti
PTAL Wasm opcodes landed, there's a little bit of opcode related code mostly just to ...
4 years, 5 months ago (2016-07-01 22:24:48 UTC) #10
bbudge
https://codereview.chromium.org/1991143002/diff/120001/src/compiler/wasm-compiler.h File src/compiler/wasm-compiler.h (right): https://codereview.chromium.org/1991143002/diff/120001/src/compiler/wasm-compiler.h#newcode167 src/compiler/wasm-compiler.h:167: Node* BuildChangeTaggedToFloat64(Node* value); It's unfortunate we have to perturb ...
4 years, 5 months ago (2016-07-06 20:18:08 UTC) #11
gdeepti
https://codereview.chromium.org/1991143002/diff/120001/src/compiler/wasm-compiler.h File src/compiler/wasm-compiler.h (right): https://codereview.chromium.org/1991143002/diff/120001/src/compiler/wasm-compiler.h#newcode167 src/compiler/wasm-compiler.h:167: Node* BuildChangeTaggedToFloat64(Node* value); On 2016/07/06 20:18:08, bbudge wrote: > ...
4 years, 5 months ago (2016-07-07 04:05:34 UTC) #12
gdeepti
https://codereview.chromium.org/1991143002/diff/120001/src/compiler/wasm-compiler.h File src/compiler/wasm-compiler.h (right): https://codereview.chromium.org/1991143002/diff/120001/src/compiler/wasm-compiler.h#newcode167 src/compiler/wasm-compiler.h:167: Node* BuildChangeTaggedToFloat64(Node* value); On 2016/07/07 04:05:34, gdeepti wrote: > ...
4 years, 5 months ago (2016-07-08 00:09:25 UTC) #13
titzer
https://codereview.chromium.org/1991143002/diff/160001/src/compiler/wasm-compiler.cc File src/compiler/wasm-compiler.cc (right): https://codereview.chromium.org/1991143002/diff/160001/src/compiler/wasm-compiler.cc#newcode616 src/compiler/wasm-compiler.cc:616: set_has_simd_ops(true); Can you add a helper method called simd() ...
4 years, 5 months ago (2016-07-08 14:16:03 UTC) #14
bbudge
https://codereview.chromium.org/1991143002/diff/160001/src/compiler/wasm-compiler.h File src/compiler/wasm-compiler.h (right): https://codereview.chromium.org/1991143002/diff/160001/src/compiler/wasm-compiler.h#newcode361 src/compiler/wasm-compiler.h:361: void set_has_simd_ops(bool has_simd_ops) { has_simd_ops_ = has_simd_ops; } Forgot ...
4 years, 5 months ago (2016-07-08 17:17:29 UTC) #15
gdeepti
https://codereview.chromium.org/1991143002/diff/160001/src/compiler/wasm-compiler.cc File src/compiler/wasm-compiler.cc (right): https://codereview.chromium.org/1991143002/diff/160001/src/compiler/wasm-compiler.cc#newcode616 src/compiler/wasm-compiler.cc:616: set_has_simd_ops(true); On 2016/07/08 14:16:03, titzer wrote: > Can you ...
4 years, 5 months ago (2016-07-11 09:50:34 UTC) #16
gdeepti
PTAL
4 years, 5 months ago (2016-07-12 16:29:24 UTC) #17
bradnelson
I think this is close if you put it behind a flag. But we do ...
4 years, 5 months ago (2016-07-13 18:10:42 UTC) #21
gdeepti
https://codereview.chromium.org/1991143002/diff/220001/src/wasm/ast-decoder.cc File src/wasm/ast-decoder.cc (right): https://codereview.chromium.org/1991143002/diff/220001/src/wasm/ast-decoder.cc#newcode1003 src/wasm/ast-decoder.cc:1003: DecodeSimdOpcode(opcode); On 2016/07/13 18:10:42, bradnelson wrote: > Add an ...
4 years, 5 months ago (2016-07-13 18:58:35 UTC) #22
titzer
lgtm modulo remaining comments https://codereview.chromium.org/1991143002/diff/260001/src/compiler/simd-lowering.cc File src/compiler/simd-lowering.cc (right): https://codereview.chromium.org/1991143002/diff/260001/src/compiler/simd-lowering.cc#newcode23 src/compiler/simd-lowering.cc:23: ConversionSignature::Builder conversions(zone_, kReturnCount, kSimd32x4); Is ...
4 years, 5 months ago (2016-07-13 19:45:10 UTC) #23
gdeepti
https://codereview.chromium.org/1991143002/diff/260001/src/compiler/simd-lowering.cc File src/compiler/simd-lowering.cc (right): https://codereview.chromium.org/1991143002/diff/260001/src/compiler/simd-lowering.cc#newcode23 src/compiler/simd-lowering.cc:23: ConversionSignature::Builder conversions(zone_, kReturnCount, kSimd32x4); On 2016/07/13 19:45:09, titzer wrote: ...
4 years, 5 months ago (2016-07-15 08:26:40 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1991143002/320001
4 years, 5 months ago (2016-07-15 08:27:08 UTC) #27
commit-bot: I haz the power
Committed patchset #17 (id:320001)
4 years, 5 months ago (2016-07-15 08:29:12 UTC) #29
commit-bot: I haz the power
Patchset 17 (id:??) landed as https://crrev.com/73df92fc2fdbbfadc17e8ab4e58ec56ae2b3d91a Cr-Commit-Position: refs/heads/master@{#37789}
4 years, 5 months ago (2016-07-15 08:30:34 UTC) #31
Michael Achenbach
A revert of this CL (patchset #17 id:320001) has been created in https://codereview.chromium.org/2154723002/ by machenbach@chromium.org. ...
4 years, 5 months ago (2016-07-15 08:59:22 UTC) #32
gdeepti
On 2016/07/15 08:59:22, Michael Achenbach wrote: > A revert of this CL (patchset #17 id:320001) ...
4 years, 5 months ago (2016-07-16 03:47:12 UTC) #39
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/1991143002/340001
4 years, 5 months ago (2016-07-16 03:47:33 UTC) #42
commit-bot: I haz the power
Committed patchset #18 (id:340001)
4 years, 5 months ago (2016-07-16 03:49:53 UTC) #44
commit-bot: I haz the power
4 years, 5 months ago (2016-07-16 03:51:01 UTC) #46
Message was sent while issue was closed.
Patchset 18 (id:??) landed as
https://crrev.com/18543ff1da34a7b12f583375b53320fcf5013931
Cr-Commit-Position: refs/heads/master@{#37807}

Powered by Google App Engine
This is Rietveld 408576698