Skip to content

wasmtime: Implement table.fill#1973

Merged
fitzgen merged 1 commit into
bytecodealliance:mainfrom
fitzgen:table-fill
Jul 6, 2020
Merged

wasmtime: Implement table.fill#1973
fitzgen merged 1 commit into
bytecodealliance:mainfrom
fitzgen:table-fill

Conversation

@fitzgen
Copy link
Copy Markdown
Member

@fitzgen fitzgen commented Jul 3, 2020

Part of #929

@fitzgen fitzgen requested a review from alexcrichton July 3, 2020 00:00
Copy link
Copy Markdown
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Could you add a fill method to the wasmtime crate too? Also how come there are two builtin functions indices but only one actual function?

@fitzgen
Copy link
Copy Markdown
Member Author

fitzgen commented Jul 6, 2020

Could you add a fill method to the wasmtime crate too?

I'd prefer to make a separate PR for that, and I already have a separate checkbox for bubbling these things out to the API in the tracking issue so it shouldn't get lost or forgotten about.

Also how come there are two builtin functions indices but only one actual function?

This is the same as our table.grow implementation: they methods have the same Rust signature and essentially do the same thing regardless which reference type they are working with but Cranelift needs to have different signatures for the different variants so that it can produce stack maps for values flowing into the externref variants. We don't have much to gain by making different copies of the functions because they are already out-of-line, relatively expensive calls doing expensive operations, and we can have less code (in terms of binary size and maintenance) by having one function.

@fitzgen fitzgen merged commit 80ff22f into bytecodealliance:main Jul 6, 2020
@fitzgen fitzgen deleted the table-fill branch July 6, 2020 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants