[wasm] Adding a convolution matrix filter test to highlight the performance advantages of JITing

[wasm] Adding a convolution matrix filter test to highlight the performance advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap because the module instance wasn't correctly synthesized. This wasn't discovered in the previous test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as the length of the opcode wasn't computed correctly.
BUG=5044
Committed: https://crrev.com/ccfd224ec3d1d9fd3b7e577f1db91bdf1e30d1f0
Cr-Commit-Position: refs/heads/master@{#37957}

[ritesht, 2016-07-21 19:00:31]

Description was changed from

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

BUG=5044
==========

to

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

BUG=5044
==========

[ritesht, 2016-07-21 19:00:31]

ritesht@google.com changed reviewers:
+ bradnelson@chromium.org, jpp@chromium.org, mtrofin@chromium.org

[bradnelson, 2016-07-21 19:05:20]

Update the commit message to reflect that this fixed two bugs:
* JITed functions couldn't access the heap (since we weren't setting it up
properly)
* Decoding of jited functions was off, resulting in one of the operands being
also interpreted as an opcode after the jitting instruction.

https://codereview.chromium.org/2168183002/diff/1/test/mjsunit/wasm/jit-singl...
File test/mjsunit/wasm/jit-single-function.js (left):

https://codereview.chromium.org/2168183002/diff/1/test/mjsunit/wasm/jit-singl...
test/mjsunit/wasm/jit-single-function.js:16: 
No real change in file, drop it.

https://codereview.chromium.org/2168183002/diff/1/test/mjsunit/wasm/module-me...
File test/mjsunit/wasm/module-memory.js (left):

https://codereview.chromium.org/2168183002/diff/1/test/mjsunit/wasm/module-me...
test/mjsunit/wasm/module-memory.js:11: 
No real change in file, drop it.

[ritesht, 2016-07-21 19:12:45]

Description was changed from

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

BUG=5044
==========

to

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1)JITed functions were not allowed access to the heap    because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2)Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

[bradnelson, 2016-07-21 22:33:53]

Description was changed from

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1)JITed functions were not allowed access to the heap    because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2)Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

to

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap   because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

[bradnelson, 2016-07-21 22:34:08]

Description was changed from

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap   because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

to

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

[bradnelson, 2016-07-21 22:34:32]

The CQ bit was checked by bradnelson@chromium.org

[bradnelson, 2016-07-21 22:34:33]

lgtm

[commit-bot: I haz the power, 2016-07-21 22:34:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 22:37:02]

Description was changed from

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

to

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

[commit-bot: I haz the power, 2016-07-21 22:37:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-21 22:39:50]

Description was changed from

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044
==========

to

==========
[wasm] Adding a convolution matrix filter test to highlight the performance
advantages of JITing

This cl also fixes two bugs in the previous code:
    1) JITed functions were not allowed access to the heap because the module
instance wasn't correctly synthesized. This wasn't discovered in the previous
test.
    2) Decoding of functions with the JITSingleFunction opcode was off by 1 as
the length of the opcode wasn't computed correctly.
BUG=5044

Committed: https://crrev.com/ccfd224ec3d1d9fd3b7e577f1db91bdf1e30d1f0
Cr-Commit-Position: refs/heads/master@{#37957}
==========

[commit-bot: I haz the power, 2016-07-21 22:39:51]

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

[Mircea Trofin, 2016-07-22 00:15:58]

Please see comments.

An overall comment: could you clarify (maybe in subsequent checkin) 
what you're comparing JITing with, and also, what exactly does 
"performance" refer to (e.g.: time spent JIT-ing? time spent executing 
JIT-ed code? both? etc)

Maybe I missed something, but the test seems to be just testing functionality,
not performance. Was the intent to just add a functional 
test?

If your purpose is to perform runtime measurements, we should do that
in a benchmark.

https://codereview.chromium.org/2168183002/diff/20001/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (left):

https://codereview.chromium.org/2168183002/diff/20001/src/wasm/ast-decoder.cc...
src/wasm/ast-decoder.cc:20: 
why remove this space?

https://codereview.chromium.org/2168183002/diff/20001/test/mjsunit/wasm/filte...
File test/mjsunit/wasm/filter-jit.js (right):

https://codereview.chromium.org/2168183002/diff/20001/test/mjsunit/wasm/filte...
test/mjsunit/wasm/filter-jit.js:2: // Use of this source code is governed by a
BSD-style license that can be
Copyright 2016, not 2015

https://codereview.chromium.org/2168183002/diff/20001/test/mjsunit/wasm/filte...
test/mjsunit/wasm/filter-jit.js:31: kExprSetLocal, 06,
I'm concerned this will be tricky to maintain. A few suggestions:
- factor the instruction stream into a few building block functions, like
"dereference array"; or

- build the wasm part from C or from wast.