Regression when generating `ref.func`?

[dcodeIO]

Our preliminary reference types support so far created ref.func expressions with a funcref type, like so

BinaryenRefFunc(theModule, theName, BinaryenTypeFuncref());

which used to work (both in codegen and execution), but now (during optimizing?) seems to hit the assertion at

binaryen/src/literal.h

Lines 81 to 84 in 5a938f4

	explicit Literal(Name func, HeapType type)
	: func(func), type(type, NonNullable) {
	assert(type.isSignature());
	}

Is this expected, i.e. that funcref is !type.isSignature() (and now requires a concrete GC type, even if only reftypes is enabled)?

[tlively]

We did update Literal so that it required a specific signature type since every way to create a function reference literal in WebAssembly has that type information available. I guess we should make the same change to ref.func and add a binaryen-specific validation rule that have a specific type. That wouldn't keep your code working, but it would catch the problem earlier. We recently did something similar for ref.null by requiring it to have a bottom heap type as well.

[dcodeIO]

So, is the idea to always use concrete GC types, and if GC is not enabled, Binaryen would instead output funcref? Or does this imply that reference types without GC are no longer possible?

[tlively]

Right, binaryen will output funcref if GC is not enabled, even though the internal representation is a specific GC type.

(edit: assuming there are no other bugs!)

[dcodeIO]

Something interesting is happening when building the concrete GC types on the side in preparation for using them with ref.func as of this issue. Probably due to using a type builder, which requires setting a type system, there are large diffs in the text format like

- (type $none_=>_i64 (func (result i64)))
- (type $none_=>_none (func))
- (type $i32_i32_i32_i32_=>_none (func (param i32 i32 i32 i32)))
+ (type $none_=>_i64 (func_subtype (result i64) func))
+ (type $none_=>_none (func_subtype func))
+ (type $i32_i32_i32_i32_=>_none (func_subtype (param i32 i32 i32 i32) func))

- (func $bigint-integration/getInternalValue (result i64)
+ (func $bigint-integration/getInternalValue (type $none_=>_i64) (result i64)
   global.get $bigint-integration/internalValue
  )
- (func $~start
+ (func $~start (type $none_=>_none)

just from generating the types, which is unexpected since the types aren't used and GC is disabled (hoping that just reftypes would be emitted). Might be that this is limited to the text format or something, but so far seems that just generating the necessary internal representation has undesired side effects?

[tlively]

Interesting. Yeah, this change should only be visible in the text format, but it's still curious. What type system setting are you using?

[dcodeIO]

Right now the code sets the type system to nominal before using a type builder. I guess that's where func_subtype comes from?

[tlively]

Yeah, that's right. If you were previously using equirecursive types (still the default, although they shouldn't be), then that explains the difference.

[dcodeIO]

Would it make sense perhaps to check, when generating the text format, what the supported feature set is, and emit text format according to the feature set, similar to what is done for the binary format? Say, in this case, where GC is not enabled, keep emitting func, similar to how the binary format emits funcref when GC is disabled? Seems useful in case someone has a need to roundtrip the text format into another tool or so, which otherwise would only work with the binary format.

[dcodeIO]

Speaking of which, doesn't this change lead to roundtripping problems, in that the binary format cannot be roundtripped to Binaryen itself? For instance, a ref.func instructions now requires a concrete GC type, but when emitting binary format with GC disabled, Binaryen would emit funcref, losing information about the concrete GC type, so reading the binary back would use funcref, which is invalid?

[tlively]

The text format we emit isn't actually the standard GC text format, so it needs to be overhauled at some point anyway, probably as part of switching to the new text text parser. I consider tweaking it before that a low priority, but would happily take patches if that's something that's something you think is worth doing. The change you're proposing certainly makes sense.

ref.func has a function index immediate argument rather than a type immediate, so it's always possible to get a specific signature for it and I don't think there are any round trip problems. I believe the only place type immediates are used before the GC proposal is on ref.null instructions, which always have bottom types in Binaryen IR anyway.

[dcodeIO]

Ah, right, missed that the type can still be reconstructed in case of a ref.func. Indeed a non-issue then. Regarding the current text format, that's more a nice to have right now as far as I can tell, as I am not aware of anyone strictly relying on the text format. Thanks for the info about the new parser, can probably wait :)

[tlively]

Our support for WasmGC is now stable and follows the completed standard.