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

Expose 64-bit only hardware intrinsic in nested classes#33286

Closed
fiigii wants to merge 2 commits into
dotnet:masterfrom
fiigii:x8664
Closed

Expose 64-bit only hardware intrinsic in nested classes#33286
fiigii wants to merge 2 commits into
dotnet:masterfrom
fiigii:x8664

Conversation

@fiigii
Copy link
Copy Markdown
Contributor

@fiigii fiigii commented Nov 7, 2018

Match the CoreCLR change dotnet/coreclr#20146

@tannergooding @eerhardt

@fiigii
Copy link
Copy Markdown
Contributor Author

fiigii commented Nov 8, 2018

@dotnet-bot test this please

MembersMustExist : Member 'System.Runtime.Intrinsics.X86.Avx2.BroadcastScalarToVector256(System.Runtime.Intrinsics.Vector128<System.UInt16>)' does not exist in the implementation but it does exist in the contract.
MembersMustExist : Member 'System.Runtime.Intrinsics.X86.Avx2.BroadcastScalarToVector256(System.Runtime.Intrinsics.Vector128<System.UInt32>)' does not exist in the implementation but it does exist in the contract.
MembersMustExist : Member 'System.Runtime.Intrinsics.X86.Avx2.BroadcastScalarToVector256(System.Runtime.Intrinsics.Vector128<System.UInt64>)' does not exist in the implementation but it does exist in the contract.
warning : Did not find type 'T:System.Runtime.Intrinsics.X86.Bmi1.X64' in any of the seed assemblies.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure we need this. I had seen it in my PR, initially as well. But it appears to have gone away after rebasing onto the current HEAD.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(#33302)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

According to my experience, we need this for "Packaging All Configurations x64 Debug Build"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ericstj, I think we might've discussed this previously.

For some reason, the tooling is not picking the System.Private.Corelib that we are building against, and is instead picking up an older copy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking at the binlog, the seed assemblies being passed in are:

C:\Users\tagoo\.nuget\packages\microsoft.targetingpack.private.netnative\1.1.0-beta-27106-00\lib\uap10.1\System.Private.CoreLib.dll
E:\Users\tagoo\Repos\corefx\artifacts\bin\ref/netcoreapp/netstandard.dll

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmmm, actually; even the updated package still doesn't have the new types ☹️, so we might still need the changes after all.

  • You can validate that the changes pass locally with .\build.cmd -allConfigurations

@ericstj, do you know what is required to get it to ignore the Did not find type 'T:System.Runtime.Intrinsics...' in any of the seed assemblies. messages? Just adding the exact text doesn't appear to work.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a scenario that is only handled by ignoreMissingTypes?

  • It would also be good to find out why the uap corelib isn't updated here.... These are part of the shared types and have been checked in on the CoreCLR side for a few days now

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I talked with @ericstj and he helped me resolve the issues.

I've cherry-picked your changes into my PR to avoid any merge conflicts between ours: #33302

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried adding Errors were encountered when generating facade(s)., but still does not work...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tannergooding Great, thank you.

@fiigii fiigii closed this Nov 12, 2018
@fiigii fiigii deleted the x8664 branch November 12, 2018 19:12
@karelz karelz added this to the 3.0 milestone Nov 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants