Add a MakeSharedObjects pass - #8953
Conversation
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.
|
Looks like there's a determinism bug with the type renaming when multiple types get merged. Will look into that tomorrow. |
| std::vector<Name> funcs; | ||
| std::unordered_map<Name, Index> funcIndices; | ||
|
|
||
| Type funcref = Type(HeapTypes::func, Nullable); |
There was a problem hiding this comment.
This pass could use a toplevel comment (what the pass does) and these properties as well (how does the pass achieve its goal using the table and so forth).
| walk(segment->offset); | ||
| } | ||
| for (auto* expr : segment->data) { | ||
| for (auto*& expr : segment->data) { |
There was a problem hiding this comment.
So replaceCurrent can replace the top-level expressions. Otherwise it would replace an on-stack copy of the expressions.
There was a problem hiding this comment.
I see. Then do we not also need this on line 176?
There was a problem hiding this comment.
No, the argument there is already a reference to the value that we would want replaceCurrent to replace.
| (exact == Exact && !heapType.isBasic() ? ExactMask : 0)) { | ||
| assert(!(heapType.getID() & | ||
| (TupleMask | NullMask | (heapType.isBasic() ? 0 : ExactMask)))); | ||
| assert(!heapType.isBasic() || exact == Inexact); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I ended up reverting this and checking whether the new type is basic before trying to make it exact.
| if (auto super = type.getDeclaredSuperType()) { | ||
| setSubType(i, map(*super)); | ||
| if (auto mapped = map(*super); !mapped.isBasic()) { | ||
| setSubType(i, map(*super)); |
There was a problem hiding this comment.
| setSubType(i, map(*super)); | |
| setSubType(i, mapped); |
Or is there some reason not to?
There was a problem hiding this comment.
Oh yeah, that's an oversight.
| if (auto desc = type.getDescriptorType()) { | ||
| setDescriptor(i, map(*desc)); | ||
| if (auto mapped = map(*desc); !mapped.isBasic()) { | ||
| setDescriptor(i, map(*desc)); |
There was a problem hiding this comment.
Maybe add a comment? I assume this can happen if both of a descriptor-described pair are mapped to a basic type?
2 participants
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.