Skip to content

Conversation

@tannergooding
Copy link
Member

This resolves #34587 by adding in X64 and Arm64 nested classes to the System.Runtime.Intriniscs classes where they were missing.

@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI new-api-needs-documentation labels Jun 26, 2020
@tannergooding
Copy link
Member Author

CC. @echesakovMSFT, @CarolEidt for review.

CC. @EgorBo, this seems to have broken Mono, is there something specific I need to update?

@EgorBo
Copy link
Member

EgorBo commented Jul 6, 2020

@tannergooding looks like it's a different issue, so should be OK from mono side.

@tannergooding
Copy link
Member Author

Alright, it looked to be a failure in the test this PR is adding, so I wasn't sure. Thanks!

@tannergooding
Copy link
Member Author

@EgorBo, looks like its still failing. Based on the failure, some API is returning IsSupported where a lower ISA is not. It isn't clear which API that is as there is no line information in the failure.

@EgorBo
Copy link
Member

EgorBo commented Jul 6, 2020

@tannergooding where can I find that test? I don't see any Runtime_34587 test in the sources, is it internal?

@tannergooding
Copy link
Member Author

Right here: https://github.com/dotnet/runtime/pull/38460/files#diff-e4e58d0c6a832040d081a159b95e83bb

It just validates that for any IsSupported == true, the relevant related conditions are met.

@EgorBo
Copy link
Member

EgorBo commented Jul 6, 2020

Right here: https://github.com/dotnet/runtime/pull/38460/files#diff-e4e58d0c6a832040d081a159b95e83bb

It just validates that for any IsSupported == true, the relevant related conditions are met.

can you please add this test to issues.targets to ignore on Mono and I'll take a look tomorrow.
it looks like our hierarchy is a bit outdated e.g. we don't support X86Base

Copy link
Contributor

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

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

LGTM

@jkotas
Copy link
Member

jkotas commented Jul 22, 2020

This was JIT/EE interface breaking change and it should have updated the JIT/EE interface GUID.

I am adding a note about it in #39777

@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
@tannergooding tannergooding deleted the fix-34587 branch November 11, 2022 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI new-api-needs-documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add empty X64/Arm64 nested classes to some System.Runtime.Intrinsics

5 participants