[wasm] Rework encoding of local declarations.

[wasm] Rework encoding of local declarations.

Local declarations were previously encoded as an optional set of
4 uint16 values as part of the function declaration. This CL
implements the current design of moving these declarations to
a list of pairs of (type, count) that is part of the body.

R=bradnelson@chromium.org,binji@chromium.org
BUG=

Committed: https://crrev.com/835c5e6beb792b01b1a75a895dc2d72c61f9d1f0
Cr-Commit-Position: refs/heads/master@{#34564}

[bradnelson, 2016-03-02 21:51:15]

lgtm

https://codereview.chromium.org/1763433002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:450: local_type_vec_.insert(local_type_vec_.end(),
Shame how big this gonna be, but oh well.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/encoder.cc
File src/wasm/encoder.cc (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/encoder.cc#newcode289
src/wasm/encoder.cc:289: EmitUint16(header, static_cast<uint16_t>(size));
I assume this will change to a leb in a later change?

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h
File src/wasm/wasm-macro-gen.h (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:285: #define U32V_1(x) static_cast<byte>((x)&MASK_7)
inconsistent spacing around operator

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:290: static_cast<byte>((((x) >> 7) & MASK_7) | 0x80),
\
yeah not having those parens was muy mal.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:362: size_t SizeofUint32v(size_t val) const {
This will move out too I assume?

[titzer, 2016-03-02 22:43:41]

https://codereview.chromium.org/1763433002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:450: local_type_vec_.insert(local_type_vec_.end(),
On 2016/03/02 21:51:15, bradnelson wrote:
> Shame how big this gonna be, but oh well.

It's only for testing, so, no big deal.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/encoder.cc
File src/wasm/encoder.cc (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/encoder.cc#newcode289
src/wasm/encoder.cc:289: EmitUint16(header, static_cast<uint16_t>(size));
On 2016/03/02 21:51:15, bradnelson wrote:
> I assume this will change to a leb in a later change?

Yeah, we'll just update everything to be LEB in one go.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h
File src/wasm/wasm-macro-gen.h (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:285: #define U32V_1(x) static_cast<byte>((x)&MASK_7)
On 2016/03/02 21:51:15, bradnelson wrote:
> inconsistent spacing around operator

Somehow that's what git cl format has done. Fixed it and then it fixed it back.
:-/

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:290: static_cast<byte>((((x) >> 7) & MASK_7) | 0x80),
\
On 2016/03/02 21:51:15, bradnelson wrote:
> yeah not having those parens was muy mal.

Acknowledged.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:362: size_t SizeofUint32v(size_t val) const {
On 2016/03/02 21:51:15, bradnelson wrote:
> This will move out too I assume?

Yeah, it needs to find a nice common home.

[binji, 2016-03-02 22:51:38]

lgtm

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h
File src/wasm/wasm-macro-gen.h (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:321: byte* buffer = new byte[5 + 6 *
local_decls.size() + size];
name the magic numbers?

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:343: if (count == 0) return
static_cast<uint32_t>(local_decls.size());
include sig->parameter_count() if sig is non-null?

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:362: size_t SizeofUint32v(size_t val) const {
should be uint32_t val, right?

https://codereview.chromium.org/1763433002/diff/1/test/unittests/wasm/ast-dec...
File test/unittests/wasm/ast-decoder-unittest.cc (right):

https://codereview.chromium.org/1763433002/diff/1/test/unittests/wasm/ast-dec...
test/unittests/wasm/ast-decoder-unittest.cc:1374: // (block[2] (loop[2] (if (get
p) break[N]) (set p 1)) (return p)) (11)
(get p) => 0

https://codereview.chromium.org/1763433002/diff/1/test/unittests/wasm/ast-dec...
test/unittests/wasm/ast-decoder-unittest.cc:1388: // (block[1] (loop[1]
(block[1] (if (get p) break[N])
here too

[titzer, 2016-03-02 23:03:28]

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h
File src/wasm/wasm-macro-gen.h (right):

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:321: byte* buffer = new byte[5 + 6 *
local_decls.size() + size];
On 2016/03/02 22:51:38, binji wrote:
> name the magic numbers?

Fixed to use Size(), which should be more robust.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:343: if (count == 0) return
static_cast<uint32_t>(local_decls.size());
On 2016/03/02 22:51:38, binji wrote:
> include sig->parameter_count() if sig is non-null?

Done.

https://codereview.chromium.org/1763433002/diff/1/src/wasm/wasm-macro-gen.h#n...
src/wasm/wasm-macro-gen.h:362: size_t SizeofUint32v(size_t val) const {
On 2016/03/02 22:51:38, binji wrote:
> should be uint32_t val, right?

Done.

https://codereview.chromium.org/1763433002/diff/1/test/unittests/wasm/ast-dec...
File test/unittests/wasm/ast-decoder-unittest.cc (right):

https://codereview.chromium.org/1763433002/diff/1/test/unittests/wasm/ast-dec...
test/unittests/wasm/ast-decoder-unittest.cc:1374: // (block[2] (loop[2] (if (get
p) break[N]) (set p 1)) (return p)) (11)
On 2016/03/02 22:51:38, binji wrote:
> (get p) => 0

Done.

https://codereview.chromium.org/1763433002/diff/1/test/unittests/wasm/ast-dec...
test/unittests/wasm/ast-decoder-unittest.cc:1388: // (block[1] (loop[1]
(block[1] (if (get p) break[N])
On 2016/03/02 22:51:38, binji wrote:
> here too

Done.

[titzer, 2016-03-07 20:52:03]

The CQ bit was checked by titzer@chromium.org

[titzer, 2016-03-07 20:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
bradnelson@chromium.org, binji@chromium.org
Link to the patchset: https://codereview.chromium.org/1763433002/#ps180001
(title: " ")

[commit-bot: I haz the power, 2016-03-07 20:52:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1763433002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1763433002/180001

[commit-bot: I haz the power, 2016-03-07 21:04:14]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-03-07 21:05:23]

Description was changed from

==========
[wasm] Rework encoding of local declarations.

Local declarations were previously encoded as an optional set of
4 uint16 values as part of the function declaration. This CL
implements the current design of moving these declarations to
a list of pairs of (type, count) that is part of the body.

R=bradnelson@chromium.org,binji@chromium.org
BUG=
==========

to

==========
[wasm] Rework encoding of local declarations.

Local declarations were previously encoded as an optional set of
4 uint16 values as part of the function declaration. This CL
implements the current design of moving these declarations to
a list of pairs of (type, count) that is part of the body.

R=bradnelson@chromium.org,binji@chromium.org
BUG=

Committed: https://crrev.com/835c5e6beb792b01b1a75a895dc2d72c61f9d1f0
Cr-Commit-Position: refs/heads/master@{#34564}
==========

[commit-bot: I haz the power, 2016-03-07 21:05:24]

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