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

Fix and enable GC on new x64 backend. #2410

Merged
merged 1 commit into from
Nov 13, 2020
Merged

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Nov 13, 2020

One critical bit of plumbing was missing: the StackMapSink passed to
compile_and_emit was not actually receiving stackmaps. This seemingly
very basic issue was not caught because the other major user of reftype
support, SpiderMonkey, extracts stackmaps with a lower-level API. The
SM integration was built this way to avoid an awkward API quirk when
passing stackmaps through a CodeSink that proxies them to a
StackMapSink: the CodeSink wants Values for each reference slot,
while the actual StackMapSink does not require these. This PR tweaks
the plumbing in a slightly different way to make wasmtime GC tests,
and presumably other consumers of stack-map info from the top-level
Cranelift interface, happy.

One critical bit of plumbing was missing: the `StackMapSink` passed to
`compile_and_emit` was not actually receiving stackmaps. This seemingly
very basic issue was not caught because the other major user of reftype
support, SpiderMonkey, extracts stackmaps with a lower-level API. The
SM integration was built this way to avoid an awkward API quirk when
passing stackmaps through a `CodeSink` that proxies them to a
`StackMapSink`: the `CodeSink` wants `Value`s for each reference slot,
while the actual `StackMapSink` does not require these. This PR tweaks
the plumbing in a slightly different way to make `wasmtime` GC tests,
and presumably other consumers of stack-map info from the top-level
Cranelift interface, happy.
@cfallin cfallin requested a review from fitzgen November 13, 2020 01:03
@cfallin
Copy link
Member Author

cfallin commented Nov 13, 2020

This addresses one of the items in #2079.

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Nov 13, 2020
@cfallin cfallin merged commit 7b9d870 into bytecodealliance:main Nov 13, 2020
@cfallin cfallin deleted the x64-gc branch January 6, 2021 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants