Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Remove System.Runtime.Intrinsics for .NET Core 2.1#27567

Merged
eerhardt merged 1 commit into
dotnet:masterfrom
eerhardt:IntrinsicsExperimental
Mar 1, 2018
Merged

Remove System.Runtime.Intrinsics for .NET Core 2.1#27567
eerhardt merged 1 commit into
dotnet:masterfrom
eerhardt:IntrinsicsExperimental

Conversation

@eerhardt
Copy link
Copy Markdown
Member

  • Remove System.Runtime.Intrinsics from Microsoft.NETCore.App.
  • Rename to System.Runtime.Intrinsics.Experimental
  • Create a standalone nupkg.

See https://github.com/dotnet/corefx/issues/27486 for more information.

- Remove System.Runtime.Intrinsics from Microsoft.NETCore.App.
- Rename to System.Runtime.Intrinsics.Experimental
- Create a standalone nupkg.
Copy link
Copy Markdown
Contributor

@sdmaclea sdmaclea left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@fiigii fiigii left a comment

Choose a reason for hiding this comment

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

Thanks!

@sdmaclea
Copy link
Copy Markdown
Contributor

sdmaclea commented Mar 1, 2018

@tannergooding Please cc me on test changes to add ref.

@ahsonkhan
Copy link
Copy Markdown

@eerhardt, good to merge?

@tannergooding
Copy link
Copy Markdown
Member

@sdmaclea, could you clarify your request?

@eerhardt
Copy link
Copy Markdown
Member Author

eerhardt commented Mar 1, 2018

good to merge?

I'd like for this to get at least a cursory look from @ericstj or @weshaggard first to ensure the pkg stuff looks right.

@sdmaclea
Copy link
Copy Markdown
Contributor

sdmaclea commented Mar 1, 2018

@tannergooding I am assuming the tests in coreclr will need to be updated to add a ref to the new external package. I also assume the ne wpackage must be uploaded to nuget before coreclr can reference it. So when you create the change in coreclr to pick this up, I'd like to be cc'd, so that I know it is ready.

@eerhardt eerhardt merged commit 7e5f966 into dotnet:master Mar 1, 2018
@eerhardt eerhardt deleted the IntrinsicsExperimental branch March 1, 2018 18:19
@eerhardt
Copy link
Copy Markdown
Member Author

eerhardt commented Mar 1, 2018

@sdmaclea - I'm not very familiar with coreclr's tests, but don't they just compile directly against System.Private.CoreLib? (I have no idea, just my assumption)

See https://github.com/dotnet/coreclr/blob/a16a8a852077b0c471cf12ef75dc5e8a41333533/tests/override.targets#L21

@sdmaclea
Copy link
Copy Markdown
Contributor

sdmaclea commented Mar 1, 2018

@eerhardt The Arm64 coreclr tests did not compile w/o corefx HW Intrinsic ref dll. They are currently disabled in CoreCLR because API is not published.

@AndyAyersMS
Copy link
Copy Markdown
Member

CoreCLR tests can be set up to compile against corelib directly, but it's not commonly done.

So when this CoreFX change hits CoreCLR likely the test builds will need fixes.

@sdmaclea
Copy link
Copy Markdown
Contributor

sdmaclea commented Mar 1, 2018

@AndyAyersMS If you have instructions/sample of a test building directly against corelib, it would be helpful as we add lots of HW Intrinsic APIs. (Eliiminates the circular dependency implement in coreclr, build ref in corefx, build tests in coreclr). The tests shouldn't probably check in that way, but it would make dev life simpler.

@AndyAyersMS
Copy link
Copy Markdown
Member

I think the magic is to add:

<ReferenceSystemPrivateCoreLib>true</ReferenceSystemPrivateCoreLib>

to the csproj. For example look at Interop\ICastable\Castable.csproj. Not sure if this causes other complications.

@ViktorHofer
Copy link
Copy Markdown
Member

Am I the only one who can't build anymore because of the renamed package?

C:\git\corefx\Tools\Packaging.targets(1231,5): error : Error when creating nuget packed package from C:\git\corefx\bin\
packages\Debug\specs\Microsoft.Private.CoreFx.NETCoreApp.nuspec. System.IO.DirectoryNotFoundException: Could not find a
 part of the path 'C:\git\corefx\src\System.Runtime.Intrinsics\ref'. [C:\git\corefx\pkg\Microsoft.Private.CoreFx.NETCor
eApp\Microsoft.Private.CoreFx.NETCoreApp.pkgproj]

@eerhardt
Copy link
Copy Markdown
Member Author

eerhardt commented Mar 2, 2018

I'd suggest git clean -xdf and build again.

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.

9 participants