wasmtime / PR #11487 Add initial empty rec groups · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11487 Add initial empty rec groups

Wasmtime GitHub notifications bot (Aug 21 2025 at 05:04):

khagankhan opened PR #11487 from khagankhan:rec-empty to bytecodealliance:main:

This PR introduces initial support for empty (rec ...) groups:

Adds generation of empty recursive groups within a configurable range (clamping is applied).

Establishes foundational support for future GC work (e.g., struct definitions, related implementations).

Updates existing tests and adds new unit tests.

Introduce a new Op that attempts to instantiate a struct from a group. It is designed to be self-dropping due to type incompatibility.

Note:

The line:
StructNew(k: |ops| ops.total_struct_types() => u32) : 0 => 0,
should effectively prevent struct.new from being generated when no struct types exist. Depending on the configuration and macro logic, it will either:

Not be generated at all,

Trigger an assertion failure during macro expansion,

Or work as expected by skipping generation.

I will confirm and refine this behavior in a follow-up PR.

Wasmtime GitHub notifications bot (Aug 21 2025 at 05:04):

khagankhan requested wasmtime-fuzz-reviewers for a review on PR #11487.

Wasmtime GitHub notifications bot (Aug 21 2025 at 05:04):

khagankhan requested fitzgen for a review on PR #11487.

Wasmtime GitHub notifications bot (Aug 21 2025 at 07:45):

github-actions[bot] commented on PR #11487:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:14):

fitzgen submitted PR review:

Thanks! A few thoughts below.

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:14):

fitzgen created PR review comment:

Because this is going to be serialized and deserialized, and the fuzzer will occasionally mutate those raw bytes, that means that we will get arbitrary values for next_group and rec_groups and we can't rely on any relationship between the two fields (such as, rec_groups does not contain a group with id == next_group).

It is probably better to get rid of the next_group field and just insert random values, resolving conflicts at insertion time as necessary (or even just ignoring conflicts and letting that "merge" rec groups).

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:14):

fitzgen created PR review comment:

Nitpick: even though we are starting with struct types we plan on defining all kinds of types here eventually, so lets future-proof this a little bit and name it simply Types.

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:14):

fitzgen created PR review comment:

I think we should delay emitting empty structs in rec groups, and just emit empty rec groups in this PR. Because longer-term, we aren't going to want to special-case empty structs in rec groups like this.

Then, in the next PR, we can extend Types something like this:
struct Types {
    // ...previous stuff from above...

    // The actual type definitions.
    type_defs: BTreeMap<TypeId, SubType>,
}

struct SubType {
    // Eventually, this will have finality and an optional super
    // type; for now it will only have this stuff.

    // The rec group that this type should be defined within.
    rec_group: RecGroupId,

    // The actual type definition.
    composite_type: CompositeType,
}

enum CompositeType {
    Struct(StructType),

    // And eventually, this will have array types too.
}

struct StructType {
    // Empty for now, but eventually will have fields.
}
When doing fixups/emitting a Wasm binary, we will create a map from rec group id to set of type ids for the types that are defined within it, since we need that to encode the rec group and its members. And then further in the future, we will have to do an SCC-based thing[^0] to merge rec groups together if there are cycles, since references to types across rec groups need to be acyclic (that is, type references can only be cyclic within the same rec group).

[^0]: FWIW, we have an existing implementation of Tarjan's SCC algorithm in tree for our inliner. We could consider reusing that directly, or we could just copy and tweak some code. Will need to think more about this when we get to it.

Anyways, all that should give us a really good skeleton to start filling out everything else on top of.

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:14):

fitzgen created PR review comment:

And then we can add this in that next PR as well.

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:14):

fitzgen created PR review comment:

Kinda unexpected to have the "empty" ops contain a non-zero number of rec groups?

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:54):

khagankhan submitted PR review.

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:54):

khagankhan created PR review comment:

Oh... Copy-paste error

Wasmtime GitHub notifications bot (Aug 21 2025 at 22:55):

khagankhan updated PR #11487.

Wasmtime GitHub notifications bot (Aug 28 2025 at 03:24):

khagankhan updated PR #11487.

Wasmtime GitHub notifications bot (Aug 28 2025 at 19:02):

khagankhan commented on PR #11487:

@fitzgen I guess it is ready for another round of review

Wasmtime GitHub notifications bot (Aug 28 2025 at 19:02):

khagankhan edited a comment on PR #11487:

Thanks @fitzgen ! I guess it is ready for another round of review

Wasmtime GitHub notifications bot (Aug 28 2025 at 19:09):

fitzgen submitted PR review:

Looks great!

Wasmtime GitHub notifications bot (Aug 28 2025 at 19:10):

khagankhan commented on PR #11487:

Thank you! Time to make new ones!!!!

Wasmtime GitHub notifications bot (Aug 28 2025 at 19:49):

fitzgen merged PR #11487.

Last updated: Dec 06 2025 at 07:03 UTC