[wasm] Improve output of PrintAstForDebugging.

[wasm] Improve output of PrintAstForDebugging.

R=clemensh@chromium.org
BUG=

Committed: https://crrev.com/3181c87e04d06d5178b925c0a1fc04e0be632a95
Cr-Commit-Position: refs/heads/master@{#36086}

[Clemens Hammacher, 2016-05-06 14:11:05]

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

https://codereview.chromium.org/1956703002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1592: os << "   // arity=" << operand.arity << " depth="
<< operand.depth;
I don't see why you output the arity and depth here, where it is clearly visible
also in the output behind the corresponding expression (at least if it's
single-byte values).
Do you plan to extend the output of the decoded operands also to other things
like constants or local variables - or just to anything?

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

https://codereview.chromium.org/1956703002/diff/1/src/wasm/ast-decoder.h#newc...
src/wasm/ast-decoder.h:227: void PrintAstForDebugging(const byte* start, const
byte* end);
Is the (current) purpose to be called from gdb? Then we should add a comment to
tell this.

[titzer, 2016-05-06 16:00:57]

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

https://codereview.chromium.org/1956703002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1592: os << "   // arity=" << operand.arity << " depth="
<< operand.depth;
On 2016/05/06 14:11:05, Clemens Hammacher wrote:
> I don't see why you output the arity and depth here, where it is clearly
visible
> also in the output behind the corresponding expression (at least if it's
> single-byte values).
> Do you plan to extend the output of the decoded operands also to other things
> like constants or local variables - or just to anything?

Yeah, they are encoded as varint operand following the opcode. By printing them
we avoid any "in-head" decoding that is annoying and error prone.

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

https://codereview.chromium.org/1956703002/diff/1/src/wasm/ast-decoder.h#newc...
src/wasm/ast-decoder.h:227: void PrintAstForDebugging(const byte* start, const
byte* end);
On 2016/05/06 14:11:05, Clemens Hammacher wrote:
> Is the (current) purpose to be called from gdb? Then we should add a comment
to
> tell this.

Done.

[Clemens Hammacher, 2016-05-06 16:03:06]

lgtm

[titzer, 2016-05-06 16:06:58]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2016-05-06 16:07:06]

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

[commit-bot: I haz the power, 2016-05-06 16:27:30]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-05-06 16:28:56]

Description was changed from

==========
[wasm] Improve output of PrintAstForDebugging.

R=clemensh@chromium.org
BUG=
==========

to

==========
[wasm] Improve output of PrintAstForDebugging.

R=clemensh@chromium.org
BUG=

Committed: https://crrev.com/3181c87e04d06d5178b925c0a1fc04e0be632a95
Cr-Commit-Position: refs/heads/master@{#36086}
==========

[commit-bot: I haz the power, 2016-05-06 16:28:57]

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