-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Expose AVX512 variants of AVX, AVX2, and FMA instructions #85228
Conversation
…iftRightLogicalVariable on Avx512F and Avx512BW
Note regarding the 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. |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThis exposes 512-bit variants of all the relevant AVX, AVX2, and FMA instructions making progress towards completing:
|
src/coreclr/inc/clrconfigvalues.h
Outdated
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_EnableAVX512DQ_VL, W("EnableAVX512DQ_VL"), 1, "Allows AVX512DQ_VL+ hardware intrinsics to be disabled") | ||
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_EnableAVX512F, W("EnableAVX512F"), 1, "Allows AVX512F+ hardware intrinsics to be disabled") | ||
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_EnableAVX512F_VL, W("EnableAVX512F_VL"), 1, "Allows AVX512F_VL+ hardware intrinsics to be disabled") | ||
RETAIL_CONFIG_DWORD_INFO(EXTERNAL_EnableAVX512_VBMI, W("EnableAVX512_VBMI"), 1, "Allows AVX512_VBMI+ hardware intrinsics to be disabled") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend viewing with hide whitespace
: https://github.com/dotnet/runtime/pull/85228/files?diff=split&w=1
We tend to keep a lot of the columns aligned for various files, so when one column needs to expand we get whitespace changes that detract from the real changes.
if (IsXMMReg(reg)) | ||
{ | ||
return emitZMMregName(reg); | ||
} | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we would ideally pass in the "correct" attribute all the time, we have a number of instructions where that doesn't happen and where we end up with things like xrdx
or yrdx
.
Doing the IsXMMReg(reg)
check at least ensures we don't do that and simply get edx
/rdx
instead for general purpose registers. This helps fix a number of diffs that were printing slightly incorrectly and gives us an easier way to identify operands with the "wrong" attribute size being passed in for anyone interested in cleaning things up for a future PR.
CC. @dotnet/jit-contrib, @dotnet/avx512-contrib As with the last change, most of this is simply updating entries in the JIT intrinsic tables and exposing the managed side + xml docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
SPMI collection job was kicked off. |
This exposes 512-bit variants of all the relevant AVX, AVX2, and FMA instructions making progress towards completing: