-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Some minor cleanup post the addition of TYP_SIMD64 and ZMM support - P1 #83044
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsThe initial support was added in #80960 and a few unrelated cleanup opportunities were seen. This is the first cleanup and covers the simpler things which can be easily relegated to TARGET_XARCH only. There will be a separate PR cleaning up the SIMD handle logic to reduce code duplication and then another which ensures that
|
466a63d
to
fd06fc5
Compare
Minor TP impact is because we need to call This is called for each We only need to report that info "once" per compilation, so we should probably look at refactoring the |
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. Added couple of questions for better understanding.
@@ -10709,7 +10709,6 @@ void CodeGen::genZeroInitFrameUsingBlockInit(int untrLclHi, int untrLclLo, regNu | |||
assert(compiler->compGeneratingProlog); | |||
assert(genUseBlockInit); | |||
assert(untrLclHi > untrLclLo); | |||
assert(compiler->getSIMDSupportLevel() >= SIMD_SSE2_Supported); |
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.
Replacing this assert is not needed? Is the idea that if AVX2 is not supported, then the default is SSE2 now?
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.
The default has been SSE2 for many years (always for x64 and since around .NET Core 2.1 for x86). We require it and consider it part of the baseline ISA, so there is no need to check for its existence outside of supporting the DOTNET_EnableSSE2=0
switch for HWIntrinsic importation.
The initial support was added in #80960 and a few unrelated cleanup opportunities were seen.
This is the first cleanup and covers the simpler things which can be easily relegated to TARGET_XARCH only.
There will be a separate PR cleaning up the SIMD handle logic to reduce code duplication and then another which ensures that
simd32_t
andsimd64_t
are xarch only as well.