Runtime: Enable AVX2, SSE42 HW intrinsics: remove unimplemented API surface and mark ISAs as fully implemented

[4creators]

Decision to disable not fully implemented Intel HW intrinsics was made due to the fact that corefx already exposed full ISA API while RyuJIT only partially supported them what would interfere with proper use of ISA.IsSupported check see issue: https://github.com/dotnet/coreclr/issues/14930 and PR resolving it https://github.com/dotnet/coreclr/pull/15514.

AVX2 and SSE42 intrinsics are partially implemented with many useful instructions already exposed. IMO it would be very helpful if we would expose in System.Runtime.Intrinsics.Experimental assembly all already supported intrinsics including partially implemented ISAs. To avoid false negative reporting by ISA.IsSupported check we should remove all unimplemented APIs and mark AVX2 and SSE42 ISAs as fully implemented.

In future APIs could be expanded as new functionality is added to RyuJIT.

@CarolEidt @eerhardt @fiigii @jkotas @tannergooding

[eerhardt]

Agreed. We should remove any APIs that are not implemented in 2.1, and then return true for all ISA.IsSupported properties when running on capable hardware.

[4creators]

Working on it

[tannergooding]

@4creators, are you working on just the CoreFX side?

@fiigii, @CarolEidt: Now that https://github.com/dotnet/coreclr/pull/17030 is merged, we need one more CoreCLR change that marks all ISAs as ~"feature complete"~ "supported" (or at the very least all partially implemented ISAs).

[4creators]

@tannergooding I am starting in coreclr and will change corefx once it will appear in dogfooding update.

[4creators]

we need one more CoreCLR change that marks all ISAs as "feature complete" (or at the very least all partially implemented ISAs)

Will fix it as well.

[tannergooding]

I am starting in coreclr

I don't believe we need (or want) to remove the methods from System.Private.Corelib. (@eerhardt or @CarolEidt might be able to confirm)

[4creators]

OK waiting for answer.

IMO argument in favor of removing this APIs is that it is possible by using Roslyn without SDK to reference System.Private.CorLIB directly and use any method which is exposed there.

[tannergooding]

IMO argument in favor of removing this APIs is that it is possible by using Roslyn without SDK to reference System.Private.CorLIB directly and use any method which is exposed there.

In which case they will get a NotSupportedException, which is probably acceptable.

The reason to keep it is so that work can continue happening to implement the remaining intrinsics (as we can also just directly reference S.P.Corelib from the CoreCLR tests).

[4creators]

as we can also just directly reference S.P.Corelib from the CoreCLR tests

My understanding is that development work will be done in master while 2.1 release will live in its own branch.

In which case they will get a NotSupportedException, which is probably acceptable.

It's not that bad but neither not that good.

[fiigii]

My understanding is that development work will be done in master while 2.1 release

Yes, I guess the changes (corefx and coreclr both) should be pull requests to the branch 2.1-release instead of master. @eerhardt could you confirm?

I don't believe we need (or want) to remove the methods from System.Private.Corelib.

Yes, I prefer to retain the APIs in System.Private.Corelib and just remove the unimplemented ones from the corefx package if we can.

[4creators]

I guess the changes (corefx and coreclr both) should be pill requests to the branch 2.1-release instead of master

Agree

[CarolEidt]

I don't believe we need (or want) to remove the methods from System.Private.Corelib.

Yes, I prefer to retain the APIs in System.Private.Corelib and just remove the unimplemented ones from the corefx package if we can.

I agree; I think that's the approach we should use.

the changes (corefx and coreclr both) should be pull requests to the branch 2.1-release instead of master.

@eerhardt can confirm WRT corefx, but I don't think we need to change coreclr.

With regard to getting a NotSupportedException, I think that is perfectly reasonable for an experimental feature that someone has gone spelunking into SPC.dll to find APIs to call.

[4creators]

There are 2 PRs to the branch release/2.1 in corefx and coreclr implementing changes required to enable all ISAs with partial implementations.

dotnet/corefx#28377 and https://github.com/dotnet/coreclr/pull/17134

[eerhardt]

Sorry, I didn't get back to this sooner - I've been super swamped.

As I've replied on the PRs - they should both target master. master is still on 2.1 versioning and will be integrated into release/2.1 again in the near future. So 2.1 related changes should all go into master first, and then if it is necessary to go into release/2.1 before the integration, they can be ported. But I don't think these need to go into release/2.1 early. They can just flow when the rest of master flows.

I agree that we can leave the unimplemented APIs in System.Private.CoreLib. Yes, someone can directly reference it to use them, but our tooling doesn't allow it without removing System.Runtime from the graph, and they are setting themselves up for failure by compiling directly against the corelib. It isn't recommended.