Skip to content

Function References Draft #168

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

iainsmith
Copy link
Contributor

@iainsmith iainsmith commented Jan 4, 2025

Function References Overview Link

👋 I started looking at adding support for the function-references extension. This is very much a work in progress, that I'd appreciate feedback on. I'm still building up context on the codebase, so I suspect I'm missing several fairly obvious issues. If you do check it out locally, feel free to push improvements directly to my branch, and I'll pull them in.

So far:

  • Updated WatParser to support (ref null? $variable) syntax
  • Configure spec tests to run the wast tests for function-references & GC
    • Make SpectestResult top level so that we can track partially implemented proposals.
  • Added an instruction for call_ref (that's not passing the call_ref.wast tests yet)

TODO:

  • Get call_ref implemented correctly (and stop leaving references on the stack)
  • Add remaining instructions
  • Get all the Function reference wast test passing
  • Check the benchmarks

Questions:

I punted on separating out a HeapType struct/enum for now, as I wasn't sure what shape it would land up in, and how big the knock on change would be. Do you have a shape in mind for a HeapType or shall I keep going with the extra cases for ReferenceType for now?

I'm hoping to have more time to work on this, but it will depend on other commitments.


I've got 8/34 of the initial call_ref.wast tests passing, while the rest are failing with Expected i32 on the stack top but got ref(WasmTypes.ReferenceType.funcRef) . Run testFunctionReferencesProposals if you want to reproduce it locally.

@iainsmith iainsmith marked this pull request as draft January 4, 2025 01:34
@kateinoigakukun
Copy link
Member

Thanks @iainsmith!

I punted on separating out a HeapType struct/enum for now, as I wasn't sure what shape it would land up in, and how big the knock on change would be. Do you have a shape in mind for a HeapType or shall I keep going with the extra cases for ReferenceType for now?

I would suggest following the structure used in the spec like:

/// Reference types
public struct ReferenceType: Equatable, Hashable {
    public var isNullable: Bool
    public var heapType: HeapType

    public static var funcRef: ReferenceType {
        ReferenceType(isNullable: true, heapType: .func)
    }

    public static var externRef: ReferenceType {
        ReferenceType(isNullable: true, heapType: .func)
    }

    public init(isNullable: Bool, heapType: HeapType) {
        self.isNullable = isNullable
        self.heapType = heapType
    }
}

public enum HeapType: Equatable, Hashable {
    /// A reference to any kind of function.
    case `func`
    /// An external host data.
    case extern
}

@iainsmith iainsmith force-pushed the iain/reference-proposal branch from 65c9324 to 9141418 Compare January 4, 2025 11:43
@kateinoigakukun
Copy link
Member

I added tail-call support in the main branch because function-ref proposal depends on the proposal.

It might also be a good example to see how to implement a new proposal in WasmKit.

@iainsmith iainsmith force-pushed the iain/reference-proposal branch from 9141418 to 494e878 Compare January 4, 2025 18:51
Comment on lines 169 to 170
// TODO: Enable smoke check for encoding
// _ = try wat.encode()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to merge this branch once the WAT/WasmParser implementation passes the Spectest suite. I don't have enough time to work on it this week, so I won't push any further commits, but feel free to ask any questions you may have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kateinoigakukun, will look at the commits you pushed and try to focus on the Wat/WasmParser.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants