feat!: Implement GC instructions by ospencer · Pull Request #210 · grain-lang/binaryen.ml

ospencer · 2025-03-11T01:28:32Z

Requires some upstream Binaryen.js changes I haven't upstreamed yet.

spotandjake

I just did a basic review of this it seems the TypeBuilder js api bassically ended up being the exact same minus one small difference.

I noticed some other functions we may want to implement.

My focus on this review was mostly to find out what needed updating so I didn't pay to close attention to the actual and .ml, .mli files.

src/expression.c

spotandjake · 2025-12-05T01:33:04Z

src/expression.js

+//Requires: caml_jsstring_of_string
+//Requires: caml_list_to_js_array, caml_js_from_bool
+function caml_binaryen_call_ref(wasm_mod, target, params, typ, is_return) {
+  return wasm_mod.call_ref(


I cannot find this function in the js bindings in v124

I think this needs to be added to the bindings, though I don't know the process on the binaryen side as I haven't touched it before:

self['call_ref'] = function(target, operands, type, isReturned) { return preserveStack(() => Module['_BinaryenCallRef'](module, target, i32sToStack(operands), operands.length, type, isReturn) ); };

spotandjake · 2025-12-05T01:33:57Z

src/expression.js

+function caml_binaryen_br_on(wasm_mod, op, name, ref, typ) {
+  switch (op) {
+    case Binaryen.BrOnNull:
+      return wasm_mod.br_on.null(caml_jsstring_of_string(name), ref, typ);


Shouldn't this be br_on_null https://github.com/WebAssembly/binaryen/blob/64ba23996a10e229d46e41eb37736a55af87f79a/src/js/binaryen.js-post.js#L2480 and not take typ?

spotandjake · 2025-12-05T01:38:44Z

src/expression.js

+//Provides: caml_binaryen_call_ref_get_target
+//Requires: Binaryen
+function caml_binaryen_call_ref_get_target(exp) {
+  return Binaryen.CallRef.getTarget(exp);


I couldn't find the Binaryen.CallRef stuff in the js bindings.

spotandjake · 2025-12-05T01:40:08Z

src/expression.js

+//Requires: caml_js_from_bool
+function caml_binaryen_struct_get(wasm_mod, index, ref, type, signed) {
+  if (caml_js_from_bool(signed)) {
+    return wasm_mod.struct.get_s(index, ref, type);


It looks like the get_s and get_u were converted to get accepting a signed bool. https://github.com/WebAssembly/binaryen/blob/64ba23996a10e229d46e41eb37736a55af87f79a/src/js/binaryen.js-post.js#L2506

spotandjake · 2025-12-05T01:51:04Z

src/expression.js

This doesn't seem to implement all of the function as of v124,

We don't seem to have the js versions of:

We don't seem to have either the c or js bindings for array.fill

We don't seem to have either the c or js bindings for array.new_elem

spotandjake · 2025-12-05T01:52:33Z

src/expression.c

This doesn't seem to implement all of the function as of v124,

We don't seem to have the js versions of:

We don't seem to have either the c or js bindings for BinaryenArrayFill

We don't seem to have either the c or js bindings for BinaryenArrayNewElem

spotandjake · 2025-12-05T01:57:39Z

src/type.ml

 external is_nullable : t -> bool = "caml_binaryen_type_is_nullable"
-external from_heap_type : Heap_type.t -> t = "caml_binaryen_type_from_heap_type"
+
+external from_heap_type : Heap_type.t -> bool -> t


Just a note that this is just a fix to our existing bindings.

spotandjake · 2025-12-05T02:09:52Z

src/type_builder.ml

+  mutable_ : bool;
+}
+
+type error =


Might be a good idea to link to the values here: https://github.com/WebAssembly/binaryen/blob/64ba23996a10e229d46e41eb37736a55af87f79a/src/binaryen-c.h#L3618

Just for future updates.

Might also be a good idea to document the update practice for this in #252

spotandjake · 2025-12-05T02:21:30Z

src/type_builder.js

+    var types = builder.buildAndDispose();
+    return [0, caml_js_to_array(types)];
+  } catch (e) {
+    return [1, [0, e.index, e.reason]];


https://github.com/WebAssembly/binaryen/blob/64ba23996a10e229d46e41eb37736a55af87f79a/src/js/binaryen.js-post.js#L2962

This looks to just throw a generic TypeError which I don't think has an index or reason.

The api looks to just ignore the error reason.

spotandjake · 2025-12-05T02:39:53Z

src/expression.js

+    case Binaryen.BrOnNull:
+      return wasm_mod.br_on.null(caml_jsstring_of_string(name), ref, typ);
+    case Binaryen.BrOnNonNull:
+      return wasm_mod.br_on.non_null(caml_jsstring_of_string(name), ref, typ);


Shouldn't this be br_on_non_null https://github.com/WebAssembly/binaryen/blob/6ec7b5f9c615d3b224c67ae221d6812c8f8e1a96/src/js/binaryen.js-post.js#L2484 and not take typ?

spotandjake · 2025-12-05T02:40:24Z

src/expression.js

+    case Binaryen.BrOnNonNull:
+      return wasm_mod.br_on.non_null(caml_jsstring_of_string(name), ref, typ);
+    case Binaryen.BrOnCast:
+      return wasm_mod.br_on.cast(caml_jsstring_of_string(name), ref, typ);


Shouldn't this be br_on_cast
https://github.com/WebAssembly/binaryen/blob/6ec7b5f9c615d3b224c67ae221d6812c8f8e1a96/src/js/binaryen.js-post.js#L2488

spotandjake · 2025-12-05T02:40:43Z

src/expression.js

+    case Binaryen.BrOnCast:
+      return wasm_mod.br_on.cast(caml_jsstring_of_string(name), ref, typ);
+    case Binaryen.BrOnCastFail:
+      return wasm_mod.br_on.cast_fail(caml_jsstring_of_string(name), ref, typ);


Shouldn't this be br_on_cast_fail
https://github.com/WebAssembly/binaryen/blob/6ec7b5f9c615d3b224c67ae221d6812c8f8e1a96/src/js/binaryen.js-post.js#L2492

spotandjake · 2025-12-06T23:46:47Z

Closes: #195, #182

ospencer self-assigned this Mar 11, 2025

ospencer force-pushed the oscar/122 branch 4 times, most recently from b8876c4 to 92aa57f Compare November 2, 2025 03:57

ospencer force-pushed the oscar/122 branch 2 times, most recently from 54618d1 to 6aa04d1 Compare November 11, 2025 22:42

Base automatically changed from oscar/122 to main November 12, 2025 01:07

feat!: Implement GC instructions

b195f5b

ospencer force-pushed the oscar/gc branch from eb74b0a to b195f5b Compare November 13, 2025 00:05

spotandjake reviewed Dec 5, 2025

View reviewed changes

spotandjake added the Api Issues or pull requests relating to implementing or modifying a binaryen api. label Dec 6, 2025

Conversation

ospencer commented Mar 11, 2025

Uh oh!

spotandjake left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spotandjake Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spotandjake Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spotandjake Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spotandjake Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spotandjake commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

spotandjake Dec 5, 2025 •

edited

Loading

spotandjake Dec 5, 2025 •

edited

Loading

spotandjake Dec 5, 2025 •

edited

Loading

spotandjake Dec 5, 2025 •

edited

Loading

spotandjake commented Dec 6, 2025 •

edited

Loading