1524530 - Support elem_expr initializer in passive elem segments + encode element type

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Lars T Hansen [:lth]]

The proposal has evolved and we must keep up:

See https://github.com/WebAssembly/bulk-memory-operations/blob/master/proposals/bulk-memory-operations/Overview.md#element-segments. When a passive segment of anyfunc references a function, the reference is not encoded as a function index but as a (ref.func <funcname>) operation, and it is also possible for the expression to be a ref.null. Furthermore in this case the encoding includes an element type, which at the moment must be funcref/anyfunc but can also be anyref once we have reftypes.

This situation represents an awkward dependency of the bulkmem proposal on the reftypes proposal, but we can probably ship bulkmem by only allowing anyfunc as the type and by presupposing (and thus forcing reftypes to accept it) the encoding of the ref.func initializer expression.

Comment 1

[Lars T Hansen [:lth]]

This also means coming up with something sane for the wat format, eg,

(elem passive anyfunc $f $g $h)                    ;; no need to spell out ref.func if type == anyfunc
(elem passive anyfunc $f $g $h ref.null $i $j)     ;; funcref is nullable
(elem passive anyfunc (ref.func $f) (ref.func $g)) ;; spell it out if you like
(elem passive anyref ref.null (ref.func $f))       ;; anyfunc upcasts to anyref
(elem passive anyref ref.null $f)                  ;; should we infer ref.func here?  not obviously a winner

Note, in our current in-flight text syntax, "anyfunc" is implied and can't even be present, but since this comes after "passive" we can do whatever we want without breaking much.

Of course funcref is an alias for anyfunc everywhere, probably even without the reftypes proposal.

ref.null and ref.func should probably only be recognized as (text) encodings with the reftypes proposal present, though one could dispute that, based on some of the prose in the bulk memory spec. It's possible that ref.null is so generic and so pre-existing that it should be supported for bulk memory.

Comment 2

[Lars T Hansen [:lth]]

For now it looks like the spec interpreter goes with only this:

(elem passive $f $g $h) ;; these are all functions

which is what we already support, except for the encoding. Thus in the future we'd default to anyfunc and would opt into any other type.

https://github.com/WebAssembly/bulk-memory-operations/pull/54/files#diff-a782e449d3e7b44f39b6bc740d5e508fR572

Comment 3

[Lars T Hansen [:lth]]

Attachment: bug1524530-passive-segments.patch

This is "done" and we should just review it, but we might discuss whether we should actually land it given the interactions with the reftypes proposal. I'm guessing there's no real harm in landing it though.

Comment 4

[Julian Seward [:jseward]]

Comment on attachment 9041442 [details] [diff] [review]
bug1524530-passive-segments.patch

Review of attachment 9041442 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/wasm/passive-segs-nonboundary.js
@@ +310,5 @@
> +               body.push(1);           // 1 element segment
> +               body.push(1);           // Flag: Passive
> +               body.push(AnyFuncCode + (mangle == "type" ? 1 : 0)); // always anyfunc
> +               body.push(1);           // Element count
> +               body.push(0xD2 + (mangle == "ref.func" ? 1 : 0)); // always ref.func

Can you lift the 0xD2 out and give it a name, PlaceholderRefFunc, so it's more obvious it refers to the same thing that the C++ refers to?

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e10ebb4af183
Fix encoding/decoding of passive element segments.  r=jseward

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e10ebb4af183