Add a MakeSharedObjects pass by tlively · Pull Request #8953 · WebAssembly/binaryen

tlively · 2026-07-29T21:08:17Z

This pass makes all functions unshared and all structs and arrays shared. It avoids validation errors from storing unshared function references in shared structs and arrays by replacing all function references with i31 references holding indices into a function table. Indirect calls and casts are updated to extract their function reference from the table.

This new pass will make it easier to convert unshared Wasm GC programs to run on the shared heap for testing purposes. It will also lower shared functions emitted by toolchains to allow them to run on prototypes that do not support shared functions.

This pass makes all functions unshared and all structs and arrays shared. It avoids validation errors from storing unshared function references in shared structs and arrays by replacing all function references with i31 references holding indices into a function table. Indirect calls and casts are updated to extract their function reference from the table. This new pass will make it easier to convert unshared Wasm GC programs to run on the shared heap for testing purposes. It will also lower shared functions emitted by toolchains to allow them to run on prototypes that do not support shared functions.

tlively · 2026-07-29T21:55:06Z

Looks like there's a determinism bug with the type renaming when multiple types get merged. Will look into that tomorrow.

kripken · 2026-07-29T22:20:15Z

+           (exact == Exact && !heapType.isBasic() ? ExactMask : 0)) {
    assert(!(heapType.getID() &
             (TupleMask | NullMask | (heapType.isBasic() ? 0 : ExactMask))));
-    assert(!heapType.isBasic() || exact == Inexact);


Why relax this?

The when we map old function types to i31, the general type updating code tries to update the heap type but keep exactness. There would be other ways to solve this, but this seemed simple enough.

I ended up reverting this and checking whether the new type is basic before trying to make it exact.

kripken · 2026-07-29T22:21:12Z

    if (auto super = type.getDeclaredSuperType()) {
-      setSubType(i, map(*super));
+      if (auto mapped = map(*super); !mapped.isBasic()) {
+        setSubType(i, map(*super));


Suggested change

setSubType(i, map(*super));

setSubType(i, mapped);

Or is there some reason not to?

Oh yeah, that's an oversight.

kripken · 2026-07-29T22:23:03Z

    if (auto desc = type.getDescriptorType()) {
-      setDescriptor(i, map(*desc));
+      if (auto mapped = map(*desc); !mapped.isBasic()) {
+        setDescriptor(i, map(*desc));


Maybe add a comment? I assume this can happen if both of a descriptor-described pair are mapped to a basic type?

kripken

lgtm % comments and also I am only 25% through the tests

kripken · 2026-07-30T16:15:34Z

+        // Multiple old type indices are being merged into the same new type.
+        // Deterministically keep the lesser index.
+        newIt->second = std::min(newIt->second, it->second);
+      }


Maybe split this out from this PR?

Sure, I'll split it out once I've verified that the fix is correct by having the tests pass here.

kripken · 2026-07-30T16:17:38Z

+
+    GlobalTypeRewriter rewriter(*getModule(), getPassOptions().worldMode);
+    rewriter.mapTypes(oldToNew);
+    rewriter.mapTypeNamesAndIndices(oldToNew);


We discussed having a generic helper for this type of rewriting - which could be useful in e.g. StringLowering as well. Is that feasible?

We can definitely create a helper, but I'm not totally clear on where the boundaries of the shared logic should be. What do you have in mind, specifically?

Basically, you are replacing function types with i31, and StringLowering wants to replace string types with extern. Both can use an API similar to the current TypeMapper, but that preserves rec group shapes (rather than the default TypeMapper logic which creates a new singleton rec group). So this new API just replaces types "in place", without reshaping rec groups.

Do you mind if we do this refactoring as an immediate follow-up?

sgtm, no rush

kripken · 2026-07-30T16:18:55Z

-        map(t.getHeapType()), t.getNullability(), t.getExactness());
+      auto mapped = map(t.getHeapType());
+      auto null = t.getNullability();
+      auto exact = mapped.isBasic() ? Inexact : t.getExactness();


Suggested change

auto exact = mapped.isBasic() ? Inexact : t.getExactness();

// Basic types cannot be exact.

auto exact = mapped.isBasic() ? Inexact : t.getExactness();

kripken · 2026-07-30T16:20:31Z

+  ;; TODO: Handle exnref, contref.
+)
+
+(module


Suggested change

(module

;; Test recursive function references.

(module

I think?

kripken · 2026-07-30T16:24:14Z

+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $multiple-ref-func
+    ;; Mapping of functions to i31 values should be consistent.


Shouldn't a table be generated that maps these indices to functions?

We only add the table if it was ever actually used. I could add a call or cast to this module to make sure the table gets generated, if you want.

If later tests show the table then there is no need, but maybe add a comment.

kripken · 2026-07-30T16:57:13Z

+  )
+)
+
+(module


Suggested change

(module

;; Test function indexing in global locations

(module

I think?

kripken · 2026-07-30T20:41:12Z

+  ;; CHECK:      (elem $funcs (i32.const 0))
+
+  ;; CHECK:      (func $ref-test-func (type $1) (param $0 (ref null (shared i31))) (result i32)
+  ;; CHECK-NEXT:  (ref.test (ref (shared i31))


Wait, is this sound? This would also accept an i31 that wasn't a funcref in a past life.

Yes, thanks to the magic of separate type hierarchies. It's impossible for an i31 to reach a cast on the function hierarchy in the original module, so all the i31s that reach these transformed casts must represent function references.

kripken · 2026-07-30T21:08:17Z

+(module
+  (type $sig (func (param i32) (result i32)))
+
+  ;; Test that types in if, loop, result


wip comment?

kripken · 2026-07-30T21:08:45Z

+
+  ;; Test that types in if, loop, result
+
+  ;; Test that function types in globals, tables, element segments, and tags are updated.


Wasn't this tested above?

Kind of. There was a previous test checking that the instructions were updated, but this one is checking that the types are updated. The previous test covered that for the case of funcref, but this one covers it for the case of defined function types.

kripken

lgtm % comments

tlively requested a review from a team as a code owner July 29, 2026 21:08

tlively requested review from kripken and removed request for a team July 29, 2026 21:08

kripken reviewed Jul 29, 2026

View reviewed changes

Comment thread src/passes/MakeSharedObjects.cpp

kripken reviewed Jul 29, 2026

View reviewed changes

Comment thread src/wasm-traversal.h

kripken reviewed Jul 29, 2026

View reviewed changes

tlively added 3 commits July 29, 2026 20:18

top level comment

9cf91e8

wasm-type.h improvements

a62890a

determinism fix

1276a09

tlively requested a review from kripken July 30, 2026 04:44

update tests

540f162

kripken reviewed Jul 30, 2026

View reviewed changes

tlively added 3 commits July 30, 2026 11:44

try fixing determinism again

9785244

comment on exactness

87bc57f

test comments

6967075

tlively requested a review from kripken July 30, 2026 19:54

tlively mentioned this pull request Jul 30, 2026

Fix determinism bug when mapping type names #8956

Merged

kripken reviewed Jul 30, 2026

View reviewed changes

kripken approved these changes Jul 30, 2026

View reviewed changes

tlively added 2 commits July 30, 2026 15:30

Merge remote-tracking branch 'origin/main' into make-shared

9a1284a

fix test comment

59d2bd6

tlively enabled auto-merge (squash) July 30, 2026 22:34

tlively merged commit 26d46ed into WebAssembly:main Jul 30, 2026
16 checks passed

tlively deleted the make-shared branch July 30, 2026 23:02

	auto exact = mapped.isBasic() ? Inexact : t.getExactness();
	// Basic types cannot be exact.
	auto exact = mapped.isBasic() ? Inexact : t.getExactness();

+                )
+              )
+              (module


		;; Test that types in if, loop, result

		;; Test that function types in globals, tables, element segments, and tags are updated.

Uh oh!

Conversation

tlively commented Jul 29, 2026

Uh oh!

tlively commented Jul 29, 2026

Uh oh!

Uh oh!

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!

kripken left a comment

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!

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!

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!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants