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 SPCL to use GeneratedDllImport where possible. #61640

Merged
merged 3 commits into from
Nov 17, 2021

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT added area-System.Runtime.InteropServices source-generator Indicates an issue with a source generator feature labels Nov 16, 2021
Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

Can you make sure to run the code-fix on the other target OSes of System.Private.CoreLib as well so we can catch the Linux and MacOS P/Invokes too? I think if you set the TargetOS environment variable before opening VS, you'll be able to override the target OS. Or you can change

<TargetOS Condition="'$(TargetOS)' == ''">$(_hostOS)</TargetOS>
to Linux, OSX, etc. to cycle through the different OSes in VS.

@stephentoub
Copy link
Member

where possible

What remains that's not possible? Do we have a plan to get to 100%, whether by improving the source generator or by changing the P/Invoke signatures or both?

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented Nov 16, 2021

What remains that's not possible? Do we have a plan to get to 100%, whether by improving the source generator or by changing the P/Invoke signatures or both?

@stephentoub The biggest change is the blittable signatures with Guid—we do have a plan. There are also a few scenarios involving marshalling via COM interop that will need to be updated using ComWrappers. So we do have a plan longer term and have added a TODO for each signature with what is needed.

@elinor-fung
Copy link
Member

This also doesn't include converting QCalls. Now that @jkoritzinsky has updated how the runtime handles QCalls, we should be able to do another pass over SPCL to convert those as well.

There are also a couple of other things like CriticalHandle, HandleRef, and StringBuilder that the generator doesn't support.

@stephentoub
Copy link
Member

stephentoub commented Nov 16, 2021

Thanks.

There are also a couple of other things like CriticalHandle, HandleRef, and StringBuilder that the generator doesn't support.

But we should be able to update any use of those to not need them, right (or decide to update GeneratedDllImport to support them)? I'm just trying to nudge us to a place where a) we've proven that GeneratedDllImport reasonably can be used exclusively instead of DllImport (using our vast numbers of DllImports in dotnet/runtime as a proxy for real-world usage) and b) we ship .NET 7 without having to generate any stubs at run-time.

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Nov 23, 2021

arm64 regressions (since fixed):

@EgorBo
Copy link
Member

EgorBo commented Dec 2, 2021

Improvements on windows-arm64: dotnet/perf-autofiling-issues#2555

@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.InteropServices source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants