[wasm] Implement official wasm text format

[wasm] Implement official wasm text format

When disassembling functions for the inspector, we used an internal
text representation before. This CL implements the official text
format like it is understood by the spec interpreter.

Example output:
func $main (param i32) (result i32)
block i32
  get_local 0
  i32.const 2
  i32.lt_u
  if
    i32.const -2
    return
  end
  get_local 0
  call_indirect 0
end

R=rossberg@chromium.org, titzer@chromium.org
BUG=chromium:659715
Committed: https://crrev.com/172f501233498e4dac793f415e12a213aa0b56b4
Cr-Commit-Position: refs/heads/master@{#41172}

[Clemens Hammacher, 2016-11-21 13:21:23]

The CQ bit was checked by clemensh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 13:21:26]

Dry run: 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-11-21 13:25:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 13:25:36]

Dry run: Try jobs failed on following builders:
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[Clemens Hammacher, 2016-11-21 13:49:12]

The CQ bit was checked by clemensh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 13:49:21]

Dry run: 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-11-21 13:59:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 13:59:29]

Dry run: Try jobs failed on following builders:
  v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/18105)

[Clemens Hammacher, 2016-11-21 14:04:05]

The CQ bit was checked by clemensh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 14:04:08]

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

[Clemens Hammacher, 2016-11-21 14:44:09]

OK, it's not really s-expressions. How do you call it?

Also: Sorry for the nested macros :)
They are only local to one function though, and save a lot of duplication and
possible errors.

[commit-bot: I haz the power, 2016-11-21 14:45:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 14:45:05]

Dry run: This issue passed the CQ dry run.

[titzer, 2016-11-21 14:57:26]

lgtm with nits

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc
File src/wasm/s-expr.cc (right):

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:37: CASE_INT_OP(Eqz, "eqz")
Nice! This is well macro'd code :-)

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.h
File src/wasm/s-expr.h (right):

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.h#newco...
src/wasm/s-expr.h:17: class AccountingAllocator;
Is this declaration necessary?

[rossberg, 2016-11-21 15:03:51]

Mostly nits

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc
File src/wasm/s-expr.cc (right):

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:56: CASE_SIGN_OP(INT, Shr, "shr")
Nit: there should be CASE_INT_SIGN_OP for symmetry. :)

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:185: int num_whitespaces = control_depth < 32 ? 2 *
control_depth : 64;
Maybe turn the 32 into a constant.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:212: os << "br" << (opcode == kExprBr ? "" : "_if") << " " <<
operand.depth;
Why not use GetOpName here?

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:251: os << (opcode == kExprGetLocal ? 'g' : 's') <<
"et_global "
GetOpName?

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:268: FOREACH_SIMPLE_OPCODE(CASE_OPCODE)
Nit: indent less?

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:279: FOREACH_LOAD_MEM_OPCODE(CASE_OPCODE)
Nit: indent less?

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:287: // Better print anything than failing.
Better fail than introducing silent bugs

[Clemens Hammacher, 2016-11-21 16:29:22]

The CQ bit was checked by clemensh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 16:29:30]

Dry run: 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-11-21 16:59:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 16:59:52]

Dry run: This issue passed the CQ dry run.

[Clemens Hammacher, 2016-11-21 17:48:40]

I now renamed files and methods from "s-expression" to "wasm-text".

PTAL.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc
File src/wasm/s-expr.cc (right):

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:56: CASE_SIGN_OP(INT, Shr, "shr")
On 2016/11/21 at 15:03:51, rossberg wrote:
> Nit: there should be CASE_INT_SIGN_OP for symmetry. :)

I use CASE_SIGN_OP for both I64 and INT. So I would have to introduce both
CASE_INT_SIGN_OP and CASE_I64_SIGN_OP.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:185: int num_whitespaces = control_depth < 32 ? 2 *
control_depth : 64;
On 2016/11/21 at 15:03:50, rossberg wrote:
> Maybe turn the 32 into a constant.

Done.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:212: os << "br" << (opcode == kExprBr ? "" : "_if") << " " <<
operand.depth;
On 2016/11/21 at 15:03:51, rossberg wrote:
> Why not use GetOpName here?

Yeah, it's just two alternatives, so I thought inline is more compact. But I
changed it to use GetOpName now.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:251: os << (opcode == kExprGetLocal ? 'g' : 's') <<
"et_global "
On 2016/11/21 at 15:03:51, rossberg wrote:
> GetOpName?

Same.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:268: FOREACH_SIMPLE_OPCODE(CASE_OPCODE)
On 2016/11/21 at 15:03:51, rossberg wrote:
> Nit: indent less?

This is clang-format messing it up. I tried to place "// clang-format off" and
"// clang-format on" comments, but it even misplaces those :/
The alternative would be to disable clang-formatting for the whole function, but
I'd rather have a few indentations wrong than inconsistent code style in the
long run.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:279: FOREACH_LOAD_MEM_OPCODE(CASE_OPCODE)
On 2016/11/21 at 15:03:51, rossberg wrote:
> Nit: indent less?

Same.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:287: // Better print anything than failing.
On 2016/11/21 at 15:03:51, rossberg wrote:
> Better fail than introducing silent bugs

Hm, this requires us to also handle the opcodes which only occur in asm-wasm
code, or are still experimantal. Otherwise it fails with the
--trace-wasm-text-{start,end} flags, as it just outputs all the functions.
I now added simple opcode printing for those cases (asm.js, simd, atomic), and
fail if we see anything unexpected.

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.h
File src/wasm/s-expr.h (right):

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.h#newco...
src/wasm/s-expr.h:17: class AccountingAllocator;
On 2016/11/21 at 14:57:26, titzer wrote:
> Is this declaration necessary?

Nope, removed it.

[Clemens Hammacher, 2016-11-21 17:49:02]

The CQ bit was checked by clemensh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-21 17:49:08]

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

[Clemens Hammacher, 2016-11-21 17:52:48]

Description was changed from

==========
[wasm] Implement s-expression printing

When disassembling functions for the inspector, we used an internal
text representation before. This CL implements s-expression printing
like it is understood by the spec interpreter.

Example output:
func $main (param i32) (result i32)
block i32
  get_local 0
  i32.const 2
  i32.lt_u
  if
    i32.const -2
    return
  end
  get_local 0
  call_indirect 0
end

R=rossberg@chromium.org, titzer@chromium.org
BUG=chromium:659715
==========

to

==========
[wasm] Implement official wasm text format

When disassembling functions for the inspector, we used an internal
text representation before. This CL implements the official text
format like it is understood by the spec interpreter.

Example output:
func $main (param i32) (result i32)
block i32
  get_local 0
  i32.const 2
  i32.lt_u
  if
    i32.const -2
    return
  end
  get_local 0
  call_indirect 0
end

R=rossberg@chromium.org, titzer@chromium.org
BUG=chromium:659715
==========

[commit-bot: I haz the power, 2016-11-21 18:14:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-21 18:14:21]

Dry run: This issue passed the CQ dry run.

[rossberg, 2016-11-22 11:51:58]

lgtm

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc
File src/wasm/s-expr.cc (right):

https://codereview.chromium.org/2520943002/diff/40001/src/wasm/s-expr.cc#newc...
src/wasm/s-expr.cc:268: FOREACH_SIMPLE_OPCODE(CASE_OPCODE)
On 2016/11/21 17:48:40, Clemens Hammacher wrote:
> On 2016/11/21 at 15:03:51, rossberg wrote:
> > Nit: indent less?
> 
> This is clang-format messing it up. I tried to place "// clang-format off" and
> "// clang-format on" comments, but it even misplaces those :/
> The alternative would be to disable clang-formatting for the whole function,
but
> I'd rather have a few indentations wrong than inconsistent code style in the
> long run.

clang-format, making your code horrible since 2015.

[Clemens Hammacher, 2016-11-22 11:57:29]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2016-11-22 11:57:29]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/2520943002/#ps80001
(title: "Address comments")

[commit-bot: I haz the power, 2016-11-22 11:57:34]

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-11-22 11:59:28]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479815849068570,
"parent_rev": "035de07b1287f82f2b9ab5eb902c9c1b2bb87ad9", "commit_rev":
"ea0b73c1099f258add0fe4ffe77e6ca21dc24149"}

[commit-bot: I haz the power, 2016-11-22 11:59:32]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-11-22 12:00:01]

Description was changed from

==========
[wasm] Implement official wasm text format

When disassembling functions for the inspector, we used an internal
text representation before. This CL implements the official text
format like it is understood by the spec interpreter.

Example output:
func $main (param i32) (result i32)
block i32
  get_local 0
  i32.const 2
  i32.lt_u
  if
    i32.const -2
    return
  end
  get_local 0
  call_indirect 0
end

R=rossberg@chromium.org, titzer@chromium.org
BUG=chromium:659715
==========

to

==========
[wasm] Implement official wasm text format

When disassembling functions for the inspector, we used an internal
text representation before. This CL implements the official text
format like it is understood by the spec interpreter.

Example output:
func $main (param i32) (result i32)
block i32
  get_local 0
  i32.const 2
  i32.lt_u
  if
    i32.const -2
    return
  end
  get_local 0
  call_indirect 0
end

R=rossberg@chromium.org, titzer@chromium.org
BUG=chromium:659715

Committed: https://crrev.com/172f501233498e4dac793f415e12a213aa0b56b4
Cr-Commit-Position: refs/heads/master@{#41172}
==========

[commit-bot: I haz the power, 2016-11-22 12:00:02]

Patchset 5 (id:??) landed as
https://crrev.com/172f501233498e4dac793f415e12a213aa0b56b4
Cr-Commit-Position: refs/heads/master@{#41172}