Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update doc for which integer types are supported #6233

Merged
merged 2 commits into from
Apr 18, 2023

Conversation

avanhatt
Copy link
Member

This keeps coming up on Zulip, so proposing some new text.

This is a documentation-only change.

@avanhatt avanhatt requested a review from a team as a code owner April 18, 2023 19:04
@avanhatt avanhatt requested review from abrown and removed request for a team April 18, 2023 19:04
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Thanks!

@cfallin cfallin enabled auto-merge April 18, 2023 19:09
@jameysharp jameysharp disabled auto-merge April 18, 2023 19:18
The function `valid_for_target` within the [fuzzgen function generator][fungen]
contains information about which instructions support which types.

[fungen]: https://github.com/elliottt/wasmtime/blob/545749b279974e87bfceada10f5f301f4503e4ce/cranelift/fuzzgen/src/function_generator.rs#L310
Copy link
Contributor

Choose a reason for hiding this comment

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

This text is great, but I'm not sure we should use this link. At the least, I think we should link to mainline Wasmtime rather than Trevor's fork.

Also maybe we should link to whatever is on main, rather than a specific commit. Then we can't reliably link to the specific function, but just giving the function name is probably good enough.

So I think linking to https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/fuzzgen/src/function_generator.rs would be best here.

Copy link
Contributor

@jameysharp jameysharp left a comment

Choose a reason for hiding this comment

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

Thank you!

@jameysharp jameysharp enabled auto-merge April 18, 2023 19:43
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:docs labels Apr 18, 2023
@jameysharp jameysharp added this pull request to the merge queue Apr 18, 2023
Merged via the queue into bytecodealliance:main with commit a486aa3 Apr 18, 2023
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Apr 28, 2023
* Update doc for which integer types are supported

* Update ir.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:docs cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants