[wasm] Fix OpcodeLength() calculation.

[wasm] Fix OpcodeLength() calculation.

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

Committed: https://crrev.com/90eb63418bbde165cd3b62dc71919292a8cce49f
Cr-Commit-Position: refs/heads/master@{#34730}

[Mircea Trofin, 2016-03-11 17:15:40]

mtrofin@chromium.org changed reviewers:
+ mtrofin@chromium.org

[Mircea Trofin, 2016-03-11 17:15:41]

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

https://codereview.chromium.org/1787733002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1472: void PrepareForLoop(const byte* pc, SsaEnv* env) {
Looks like the change below is aesthetic only. What do we need pc for?

https://codereview.chromium.org/1787733002/diff/1/test/unittests/wasm/wasm-ma...
File test/unittests/wasm/wasm-macro-gen-unittest.cc (right):

https://codereview.chromium.org/1787733002/diff/1/test/unittests/wasm/wasm-ma...
test/unittests/wasm/wasm-macro-gen-unittest.cc:33: EXPECT_SIZE(6,
WASM_I32V_5(-1119828934));
Do these numbers mean anything? Should they be a const indicating their
semantics?

[titzer, 2016-03-11 17:17:36]

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

https://codereview.chromium.org/1787733002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1472: void PrepareForLoop(const byte* pc, SsaEnv* env) {
On 2016/03/11 17:15:41, Mircea Trofin wrote:
> Looks like the change below is aesthetic only. What do we need pc for? 

Ah, an artifact of splitting this CL off. The next CL uses pc.

https://codereview.chromium.org/1787733002/diff/1/test/unittests/wasm/wasm-ma...
File test/unittests/wasm/wasm-macro-gen-unittest.cc (right):

https://codereview.chromium.org/1787733002/diff/1/test/unittests/wasm/wasm-ma...
test/unittests/wasm/wasm-macro-gen-unittest.cc:33: EXPECT_SIZE(6,
WASM_I32V_5(-1119828934));
On 2016/03/11 17:15:41, Mircea Trofin wrote:
> Do these numbers mean anything? Should they be a const indicating their
> semantics?

They're just random values with no semantic meaning. Factoring them out just
creates noise.

[Mircea Trofin, 2016-03-11 17:22:54]

lgtm

[titzer, 2016-03-11 17:59:01]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2016-03-11 17:59:12]

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

[commit-bot: I haz the power, 2016-03-11 18:01:57]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-11 18:02:27]

Description was changed from

==========
[wasm] Fix OpcodeLength() calculation.

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

to

==========
[wasm] Fix OpcodeLength() calculation.

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

Committed: https://crrev.com/90eb63418bbde165cd3b62dc71919292a8cce49f
Cr-Commit-Position: refs/heads/master@{#34730}
==========

[commit-bot: I haz the power, 2016-03-11 18:02:28]

Patchset 1 (id:??) landed as
https://crrev.com/90eb63418bbde165cd3b62dc71919292a8cce49f
Cr-Commit-Position: refs/heads/master@{#34730}

[binji, 2016-03-11 18:10:03]

lgtm

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

https://codereview.chromium.org/1787733002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:338: case kExprI32Const: {
oops, sorry about this. Didn't notice!

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

https://codereview.chromium.org/1787733002/diff/1/src/wasm/wasm-macro-gen.h#o...
src/wasm/wasm-macro-gen.h:113: DCHECK(length == 1 || !I32V_IN_RANGE(value,
length - 1));
yeah, probably better this way.