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

NativeAOT-LLVM: Merge nativeaot #583

Closed
wants to merge 5 commits into from

Conversation

yowl
Copy link
Contributor

@yowl yowl commented Jan 23, 2021

Merge NativeAOT and address #577

Draft as not started the RuntimeTypeHandle.GetValueInternal changes and something has broken the build as getting

C:\Users\ScottWaye\.nuget\packages\microsoft.dotnet.apicompat\6.0.0-beta.21062.10\build\Microsoft.DotNet.ApiCompat.targets(82,5): error : MembersMustExist : Member 'public void System.Threading.Thread.UnsafeStart()' does not exist in the implementation but it does exist in the contract. [E:\GitHub\runtimelab\src\libraries\System.Threading.Thread\src\System.Threading.Thread.csproj]

yowl added 2 commits January 23, 2021 11:23
…NativeAOT-LLVM

# Conflicts:
#	eng/Subsets.props
#	src/coreclr/CMakeLists.txt
#	src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
#	src/coreclr/tools/aot/ILCompiler/ILCompiler.csproj
#	src/libraries/Native/Unix/System.Globalization.Native/CMakeLists.txt
@yowl yowl changed the title Merge nativeaot NativeAOT-LLVM: Merge nativeaot Jan 23, 2021
@yowl
Copy link
Contributor Author

yowl commented Jan 26, 2021

@joktas can you guide me a bit on this, please? As you can see it fails on the ApiCompat task.

C:\Users\ScottWaye\.nuget\packages\microsoft.dotnet.apicompat\6.0.0-beta.21062.10\build\Microsoft.DotNet.ApiCompat.targets(82,5): 
error : MembersMustExist : Member 'public void System.Threading.Thread.UnsafeStart()' does not exist in the implementation but it does exist in the contract. 
[E:\GitHub\runtimelab\src\libraries\System.Threading.Thread\src\System.Threading.Thread.csproj]

As I understand this makes sure that the built libraries conform with the API surface. It fails on System.Threading.Thread which is the one where the new UnsafeStart method is added. The ValidateApiCompatForSrc task runs with the rsp input

"E:\GitHub\runtimelab\artifacts\bin\System.Threading.Thread\ref\net6.0-Release\System.Threading.Thread.dll" 
--contract-depends "E:\GitHub\runtimelab\artifacts\bin\System.Threading.Thread\ref\net6.0-Release\,E:\GitHub\runtimelab\artifacts\bin\microsoft.netcore.app.ref\ref\net6.0\,E:\GitHub\runtimelab\artifacts\bin\coreclr\Browser.wasm.Debug\aotsdk\," 
--exclude-attributes "E:\GitHub\runtimelab\eng\DefaultGenApiDocIds.txt,E:\GitHub\runtimelab\eng\ApiCompatExcludeAttributes.txt" 
--enforce-optional-rules 
--impl-dirs "E:\GitHub\runtimelab\artifacts\obj\System.Threading.Thread\net6.0-Release\,E:\GitHub\runtimelab\artifacts\bin\coreclr\Browser.wasm.Debug\aotsdk\,E:\GitHub\runtimelab\artifacts\bin\microsoft.netcore.app.ref\ref\net6.0\,"

In the dll in the ref folder, this method is present:
image

So that's the contract generated from the source https://github.com/dotnet/runtimelab/blob/feature/NativeAOT-LLVM/src/libraries/System.Threading.Thread/ref/System.Threading.Thread.cs I think. The implementation type forwards all classes from a generated file artifacts\obj\System.Threading.Thread\net6.0-Release\\System.Threading.Thread.Forwards.cs e.g.

[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Threading.Thread))]

And the csc build includes reference:E:\GitHub\runtimelab\artifacts\bin\coreclr\Browser.wasm.Debug\aotsdk\System.Private.CoreLib.dll so I read that as it pulling the Thread class from System.Private.CoreLib.dll However I've got something wrong as if I look at System.Private.CoreLib.dll I see
image

Where is my logic wrong?

Thanks

@jkotas
Copy link
Member

jkotas commented Jan 26, 2021

You need to change UnsafeStart() in src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.CoreRT.Wasm.cs to public

@yowl
Copy link
Contributor Author

yowl commented Jan 26, 2021

Doh

@yowl
Copy link
Contributor Author

yowl commented Jan 28, 2021

Looks like I've squashed the merge commits again, I'll redo that.

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