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

[mono][wasm] Rework the handling of GC references in AOTed code. #59352

Merged
merged 1 commit into from
Sep 21, 2021

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Sep 20, 2021

Previously, variables holding GC refs were marked volatile so they
were loaded/stored to the C stack on every access. Since the stack
is conservatively scanned, all the objects pointed to by it are pinned,
so there is no need to load them on every access. Instead of
marking them as volatile, allocate a 'gc pinning' area on the stack,
and store ref variables to it after they are assigned. This improves
code size and performance.

Previously, variables holding GC refs were marked volatile so they
were loaded/stored to the C stack on every access. Since the stack
is conservatively scanned, all the objects pointed to by it are pinned,
so there is no need to load them on every access. Instead of
marking them as volatile, allocate a 'gc pinning' area on the stack,
and store ref variables to it after they are assigned. This improves
code size and performance.
@vargaz
Copy link
Contributor Author

vargaz commented Sep 20, 2021

@lewing

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

This approach makes sense, but I wonder if LLVM's optimizer will outsmart us.

Also do we want to do this on non-wasm in the future? might avoid some spilling and copy_stack_data?

src/mono/mono/mini/mini-llvm.c Show resolved Hide resolved
@lewing lewing merged commit 30226bf into dotnet:main Sep 21, 2021
@vargaz vargaz deleted the wasm-gc-new branch September 21, 2021 04:33
radical added a commit to radical/runtime that referenced this pull request Sep 21, 2021
@radical
Copy link
Member

radical commented Sep 21, 2021

Can this be backported to rc2 too? System.Text.Json tests failing: #51961 (comment)

@radical
Copy link
Member

radical commented Sep 21, 2021

/backport to release/6.0-rc2

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc2: https://github.com/dotnet/runtime/actions/runs/1256370822

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants