1510216 - TypedObjects: Add wasm anyref type

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Lars T Hansen [:lth]]

For the initial work on wasm struct types I mapped 'anyref' to TYPE_OBJECT in the TO system.  This worked well because our boxing strategy for JS values to anyref was basically ToObject, and we never did any unboxing.  This is changing (bug 1505768).  We want some kind of direct support for "anyref" now, where anyref will represent a pointer-sized datum that will eventually be tagged and which can represent both native wasm values and host references in some kind of boxed form.  A "native wasm value" is of course just a TO instance.

There seem to be two avenues here:

- change the meaning of the ANY type in the TO system to mean anyref
- introduce a new ANYREF (or WASM_ANYREF) type

I'm tending to think that the latter is really what we want, so that ANY can remain available for general JS values in general TO instances, and WASM_ANYREF will have a specific meaning and can use a different boxing format.

Given the amount of C++ and JS involved here, this looks like a significant job, but it probably can't be put off much longer.  To simplify the initial job we can do this as for Stage 0 in bug 1505768: values that are not Object or null are boxed as distinguishable Objects.  That way, we can implement proper tagging support in a later round.

Comment 1

[Lars T Hansen [:lth]]

Really, type barriers etc for TYPE_WASM_ANYREF are as for TYPE_ANY but the actual loads and stores are as for TYPE_OBJECT, with a boxing step on store and an unboxing step after load.

Comment 2

[Lars T Hansen [:lth]]

Attachment: bug1510216-wasm-anyref-type-for-typed-objects.patch

This works (for our current representation, with TODO comments for the more complex representation) and is not completely frightening.  It passes the test cases I already had, but more should be written.

Comment 3

[Lars T Hansen [:lth]]

Attachment: bug1510216-wasm-anyref-type-for-typed-objects-v2.patch

See commit comment for most technical details.  Some overarching things to talk about here are:

- I've named the TO type "WasmAnyRef" / TYPE_WASM_ANYREF rather than straight
  up "AnyRef" / TYPE_ANYREF, for the sake of clarity.  I'm open to other
  opinions; "WasmAnyRef" is pretty ugly.

- The new type appears on the TypedObject namespace, this falls out when we
  treat the type the same way as existing types.  This seems like an OK thing
  to me but again, opinions welcome.

- I think I've been able to properly disable optimizations in CacheIR and
  IonBuilder, but extra attention here is probably good.  Disabling optimization
  is just a simplification, we would want to discuss which optimizations
  might make sense in the future, to avoid the C++ call overhead.

Comment 4

[Lars T Hansen [:lth]]

There will be a minor update here to fix a GC issue.

Comment 5

[Lars T Hansen [:lth]]

Attachment: bug1510216-wasm-anyref-type-for-typed-objects-v3.patch

Some minor bug fixes (gc, rooting).

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 9028989 [details] [diff] [review]
bug1510216-wasm-anyref-type-for-typed-objects-v3.patch

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

::: js/src/builtin/TypedObject.h
@@ +286,4 @@
>  enum class ReferenceType {
>    TYPE_ANY = JS_REFERENCETYPEREPR_ANY,
>    TYPE_OBJECT = JS_REFERENCETYPEREPR_OBJECT,
> +  TYPE_WASM_ANYREF = JS_REFERENCETYPEREPR_WASM_ANYREF,

I see you're asking this question in comment 0, but, iiuc, in the limit, wasm anyref struct fields correspond 1:1 with typed object 'any' fields (i.e., in the correspondence described in https://github.com/WebAssembly/gc/blob/66e2edf04da646aeded4011a94174dd31f2339d9/proposals/gc/MVP-JS.md).  Is that right and would TYPE_WASM_ANYREF be more like a temporary transitional type?

::: js/src/wasm/WasmValidate.cpp
@@ -1382,5 @@
> -        // non-Object values in Objects, it is safe; however,
> -        // boxing/unboxing operations on the JS side will be incorrect.
> -        // Once we use a tagged representation, using TYPE_OBJECT will be
> -        // unsound.
> -        ASSERT_ANYREF_IS_JSOBJECT;

This was a cool way to mark dependencies on a temporary assumption btw.

Comment 7

[Lars T Hansen [:lth]]

(In reply to Luke Wagner [:luke] from comment #6)
> Comment on attachment 9028989 [details] [diff] [review]
> bug1510216-wasm-anyref-type-for-typed-objects-v3.patch
> 
> Review of attachment 9028989 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/builtin/TypedObject.h
> @@ +286,4 @@
> >  enum class ReferenceType {
> >    TYPE_ANY = JS_REFERENCETYPEREPR_ANY,
> >    TYPE_OBJECT = JS_REFERENCETYPEREPR_OBJECT,
> > +  TYPE_WASM_ANYREF = JS_REFERENCETYPEREPR_WASM_ANYREF,
> 
> I see you're asking this question in comment 0, but, iiuc, in the limit,
> wasm anyref struct fields correspond 1:1 with typed object 'any' fields
> (i.e., in the correspondence described in
> https://github.com/WebAssembly/gc/blob/
> 66e2edf04da646aeded4011a94174dd31f2339d9/proposals/gc/MVP-JS.md).  Is that
> right and would TYPE_WASM_ANYREF be more like a temporary transitional type?

At the higher level, /every/ change I make in the TypedObject code at this time is a temporary transitional thing to one extent or another...  I'm just trying to keep things in working order, without damaging TO too much, but with the expectation that the entire TO layer will be rewritten, also to the point where it affects wasm.

It's very hard to know if the TO 'any' type corresponds to anyref because that document has nothing to say about 'any' other than that there is such a type.  I would likely feel more comfortable if there were a WebAssembly.anyref type in the same sense as there is a WebAssembly.eqref type.  I don't think we should pointlessly duplicate functionality, and if TC39 is happy making 'any' something that is compatible with our 'anyref' type in terms of layout and semantics and needs then that's great, but at the moment it's hard to know for sure.

For example, we almost certainly want our 'anyref' type to have default value null.  I suspect TC39 is more inclined to go for an 'undefined' default value for 'any', if they have a choice.  Also, we will necessarily end up boxing some values because we need 'anyref' to be compatible with 'ref' and hence be a pointer-sized thing, while 'any', if not constrained to be compatible, could avoid boxing by being a Value format, and thus a better fit performance-wise for pure JS TO use cases.  So it may be to everyone's advantage to keep the types separate, even if they are similar.

Comment 8

[Luke Wagner [:luke]]

Thanks, those are interesting points and I agree we don't need to figure this out now; just keeping TO working and minimizing unnecessary effort.

One downside if 'any' wasn't defined to simply be the same thing as `anyref` is that wasm would have no way to represent that field type and thus it would be impossible for wasm to point to objects with 'any' fields with anything more specific than an eqref.

For the null/undefined issue: iirc, wasm always requires explicit initializer values for tables/structs/arrays; it's only locals, which are purely wasm-internal, which would have the naturally-null default value.  It's only the JS API for typed objects and Table.prototype.grow() that has default values and thus we can simply do The Right JS Thing in the JS API without affecting core wasm.  Thus, iiuc, this doesn't need to be a problem.

Comment 9

[Lars T Hansen [:lth]]

(In reply to Luke Wagner [:luke] from comment #8)

> For the null/undefined issue: iirc, wasm always requires explicit
> initializer values for tables/structs/arrays;

Actually, AR has defined two struct.new variants, one that takes values for all fields and one that uses default values (not sure about arrays).  Our current prototype does not have the latter variant though.

Comment 10

[Luke Wagner [:luke]]

Ah gotcha.  I suppose it's still possible to have the default values be different, considering the defaults as sugar for the explicit and wasm/JS using different defaults...

Comment 11

[Till Schneidereit [:till]]

(In reply to Luke Wagner [:luke] from comment #10)
> Ah gotcha.  I suppose it's still possible to have the default values be
> different, considering the defaults as sugar for the explicit and wasm/JS
> using different defaults...

FWIW, I'd prefer null as the default value for `any`, and am cautiously optimistic about getting that through TC39. That doesn't address the other points from comment 7, though, so we'll see where we are when we get to fully defining the semantics of `any`.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/3e8268f13176
Add WasmAnyRef type to the TypedObject system. r=luke

Comment 13

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/3e8268f13176