`br_table` binary format differs from AstSemantics and spec

[binji]

The br_table operator is specified in AstSemantics as taking one expression that yields a value for the exited construct. This matches the spec as implemented ml-proto.

But BinaryEncoding says that br_table takes just one expression as input (the key), and doesn't mention this additional expression for the break value. AFIAK all the browser implementations match this, or they wouldn't be able to run the AngryBots wasm file.

[rossberg]

I think BinaryEncoding needs updating. This should probably be part of Binary 11. I'll prepare a PR.

[lukewagner]

I don't think the AngryBots wasm file includes a br_table value; we haven't implemented that yet in SM.

[kripken]

I do see br_table in AngryBots. Most switches would turn into br_tables.

@kripken It's the 'value' at issue here. The current implementations do not support a value on br_table, and code that runs on them does not even encode a value, not even a nop. Personally I think the use case can be handled well by local variables, so the current br_table can be considered br_table_no_value and it's still possible to add a br_table_with_value(s) in future if that is the path taken but I see no compelling use case for it?

[kripken]

Oh sorry, I misunderstood. Thanks for the clarification. Yes, there are no branch return values in asm2wasm output currently, so none in AngryBots.

I am experimenting with block/branch return values in the block-return branch of binaryen currently. I don't have things fully working yet, but it looks promising, can remove a few % of set_locals.

[jfbastien]

Has this been resolved?

Looks stale or resolved to me.

[binji]

Yes, I think this is both resolved and stale. :)