Skip to content

Type allocations as exact #7589

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

Merged
merged 14 commits into from
May 15, 2025
Merged

Type allocations as exact #7589

merged 14 commits into from
May 15, 2025

Conversation

tlively
Copy link
Member

@tlively tlively commented May 14, 2025

Update RefFunc, StructNew, ArrayNew*, ContNew, and ContBind expression
to have exact reference types, as they will under the custom descriptors
proposal. Even without that proposal enabled, using exact types in the
IR can help better optimize casts and generally allows us to propagate
more precise type information.

Update the fuzzer to avoid assertion failures and invalid output now
that it can observe exact types via allocations. Update test output to
contain newly emitted exact types.

Update RefFunc, StructNew, ArrayNew*, ContNew, and ContBind expression
to have exact reference types, as they will under the custom descriptors
proposal. Even without that proposal enabled, using exact types in the
IR can help better optimize casts and generally allows us to propagate
more precise type information.

Update the fuzzer to avoid assertion failures and invalid output now
that it can observe exact types via allocations. Update test output to
contain newly emitted exact types.
@tlively tlively requested a review from kripken May 14, 2025 01:20
Copy link
Member Author

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Converting to draft until I can update all the tests that no longer test what they mean to. The actual code should be fine to review, though.

Comment on lines +424 to +425
# Add --fuzzing to allow legacy and standard EH to coexist
cmd = [shared.V8, '--wasm-staging', '--fuzzing', fuzz_file]
Copy link
Member Author

Choose a reason for hiding this comment

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

This is probably an unrelated side effect of having updated my local V8 version.

@tlively tlively marked this pull request as draft May 14, 2025 03:29
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

code lgtm

@tlively tlively marked this pull request as ready for review May 14, 2025 23:54
@tlively tlively requested a review from kripken May 14, 2025 23:54
(struct.new_default $"{i32}")
(struct.new_default $"{i32_i64}")
(i32.const 0)
)
Copy link
Member

Choose a reason for hiding this comment

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

perhaps add a comment here? i can't figure out why it was needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm!

@tlively tlively merged commit a150177 into main May 15, 2025
14 checks passed
@tlively tlively deleted the exact-allocations branch May 15, 2025 15:31
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