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

Add GFNI Intrinsics #109537

Merged
merged 8 commits into from
Nov 22, 2024
Merged

Add GFNI Intrinsics #109537

merged 8 commits into from
Nov 22, 2024

Conversation

saucecontrol
Copy link
Member

Fixes #96170

Copy link

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, 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.

1 similar comment
Copy link

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, 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-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 5, 2024
if (((cpuFeatures & XArchIntrinsicConstants_Evex) != 0) &&
((cpuFeatures & XArchIntrinsicConstants_Avx10v1) != 0))
{
if (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_EnableAVX10v1))
{
CPUCompileFlags.Set(InstructionSet_EVEX);
CPUCompileFlags.Set(InstructionSet_AVX10v1);

if((cpuFeatures & XArchIntrinsicConstants_Avx512) != 0)
Copy link
Member Author

@saucecontrol saucecontrol Nov 5, 2024

Choose a reason for hiding this comment

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

This check wasn't really correct because it looks only at the cpuid bit and not the config knob. However, it doesn't matter anyway because if AVX512 is disabled, AVX10v1_V512 will be removed by implication in the call to EnsureValidInstructionSetSupport.

For simplicity and consistency with the other combo flags to come, I changed it to add the whole set as above, relying on the implications to do their job.

@tannergooding
Copy link
Member

CC. @dotnet/jit-contrib for secondary review

-- The JIT format errors are notably unrelated and are under #109987 (comment)

@tannergooding tannergooding merged commit 35f2b13 into dotnet:main Nov 22, 2024
166 of 169 checks passed
@saucecontrol saucecontrol deleted the gfni branch November 22, 2024 04:55
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this pull request Dec 10, 2024
* add GFNI intrinsics

* add tests

* rename file

* add missing tests and AOT handling

* fix build

* fix test result
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime.Intrinsics community-contribution Indicates that the PR has been added by a community member new-api-needs-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: GFNI Intrinsics
3 participants