-
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
Enable EVEX support by default #83648
Merged
Merged
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
bdce1f4
Remove JitForceEVEXEncoding in favor of the existing AltJit enablement
tannergooding 30f54f1
Rename IsVexEncodingInstruction to IsVexEncodableInstruction
tannergooding 63ccbb5
Updating the JIT to support EVEX by default
tannergooding 1db8c9a
Mark the AVX512 ISAs as "fully implemented" since they have no unimpl…
tannergooding 15a0755
Simplify some of the EVEX related checks in emitxarch
tannergooding 56f27aa
Tweak the Vector512 ISA check to properly account for VL
tannergooding d8f9ef7
Applying formatting patch
tannergooding 0974171
Ensure we're checking for actual KMask usage and not just potential u…
tannergooding 2328f46
Fixing CORJIT_ALLOCMEM_FLG_RODATA_64BYTE_ALIGN for the managed VM
tannergooding 081516d
Fixing the DoJitStressEvexEncoding check to account for VEX vs EVEX d…
tannergooding 9e780f8
Merge remote-tracking branch 'dotnet/main' into evex
tannergooding 222e972
Break apart an overly long assert
tannergooding 0b9dbb3
Ensure IsBaselineVector512IsaSupported works on x86
tannergooding 9dfbd7f
Block NI_Vector512_ExtractMostSignificantBits on x86 pending decompos…
tannergooding 1f6fa3d
Ensure we don't overwrite 64-byte alignment for SPMI
tannergooding b713f69
Include Vector512 HWIntrinsic tests by default
tannergooding File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2275,64 +2275,51 @@ void Compiler::compSetProcessor() | |
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_Vector128); | ||
} | ||
|
||
if (instructionSetFlags.HasInstructionSet(InstructionSet_AVX)) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_Vector256); | ||
} | ||
// x86-64-v4 feature level supports AVX512F, AVX512BW, AVX512CD, AVX512DQ, AVX512VL and | ||
// AVX512F/AVX512BW/AVX512CD/AVX512DQ/VX512VL have been shipped together historically. | ||
// It is therefore unlikely that future CPUs only support "just one" and | ||
// not worth the additional complexity in the JIT to support. | ||
if (instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F) && | ||
instructionSetFlags.HasInstructionSet(InstructionSet_AVX512BW) && | ||
instructionSetFlags.HasInstructionSet(InstructionSet_AVX512DQ)) | ||
{ | ||
// Using JitStressEVEXEncoding flag will force instructions which would | ||
// otherwise use VEX encoding but can be EVEX encoded to use EVEX encoding | ||
// This requires AVX512VL support. JitForceEVEXEncoding forces this encoding, thus | ||
// causing failure if not running on compatible hardware. | ||
|
||
// We can't use !DoJitStressEvexEncoding() yet because opts.compSupportsISA hasn't | ||
// been set yet as that's what we're trying to set here | ||
// x86-64-v4 feature level supports AVX512F, AVX512BW, AVX512CD, AVX512DQ, AVX512VL | ||
// These have been shipped together historically and at the time of this writing | ||
// there exists no hardware which doesn't support the entire feature set. To simplify | ||
// the overall JIT implementation, we currently require the entire set of ISAs to be | ||
// supported and disable AVX512 support otherwise. | ||
|
||
bool enableAvx512 = false; | ||
if (instructionSetFlags.HasInstructionSet(InstructionSet_AVX512BW_VL) && | ||
instructionSetFlags.HasInstructionSet(InstructionSet_AVX512CD_VL) && | ||
instructionSetFlags.HasInstructionSet(InstructionSet_AVX512DQ_VL)) | ||
{ | ||
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512BW)); | ||
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512CD)); | ||
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512DQ)); | ||
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F)); | ||
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F_VL)); | ||
|
||
#if defined(DEBUG) | ||
if (JitConfig.JitForceEVEXEncoding()) | ||
{ | ||
enableAvx512 = true; | ||
} | ||
else if (JitConfig.JitStressEvexEncoding() && instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F_VL)) | ||
{ | ||
enableAvx512 = true; | ||
} | ||
#endif // DEBUG | ||
instructionSetFlags.AddInstructionSet(InstructionSet_Vector512); | ||
} | ||
else | ||
{ | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_VL); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_VL); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_VL); | ||
|
||
if (!enableAvx512) | ||
{ | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_VL); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_VL); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_VL); | ||
#ifdef TARGET_AMD64 | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_VL_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_X64); | ||
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL_X64); | ||
#endif // TARGET_AMD64 | ||
} | ||
else | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_Vector512); | ||
} | ||
} | ||
#elif defined(TARGET_ARM64) | ||
if (instructionSetFlags.HasInstructionSet(InstructionSet_AdvSimd)) | ||
|
@@ -3399,7 +3386,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags) | |
rbmFltCalleeTrash = RBM_FLT_CALLEE_TRASH_INIT; | ||
cntCalleeTrashFloat = CNT_CALLEE_TRASH_FLOAT_INIT; | ||
|
||
if (DoJitStressEvexEncoding()) | ||
if (canUseEvexEncoding()) | ||
{ | ||
rbmAllFloat |= RBM_HIGHFLOAT; | ||
rbmFltCalleeTrash |= RBM_HIGHFLOAT; | ||
|
@@ -6028,6 +6015,46 @@ int Compiler::compCompile(CORINFO_MODULE_HANDLE classPtr, | |
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVXVNNI); | ||
} | ||
|
||
if (JitConfig.EnableAVX512F() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512F); | ||
} | ||
Comment on lines
+6019
to
+6022
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and the below are what allows AVX512 to light up in the AltJit. |
||
|
||
if (JitConfig.EnableAVX512F_VL() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512F_VL); | ||
} | ||
|
||
if (JitConfig.EnableAVX512BW() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512BW); | ||
} | ||
|
||
if (JitConfig.EnableAVX512BW_VL() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512BW_VL); | ||
} | ||
|
||
if (JitConfig.EnableAVX512CD() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512CD); | ||
} | ||
|
||
if (JitConfig.EnableAVX512CD_VL() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512CD_VL); | ||
} | ||
|
||
if (JitConfig.EnableAVX512DQ() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512DQ); | ||
} | ||
|
||
if (JitConfig.EnableAVX512DQ_VL() != 0) | ||
{ | ||
instructionSetFlags.AddInstructionSet(InstructionSet_AVX512DQ_VL); | ||
} | ||
#endif | ||
|
||
// These calls are important and explicitly ordered to ensure that the flags are correct in | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Like the comment says, we want to support AVX512 only if all of
AVX512F
,AVX512BW
,AVX512CD
,AVX512DQ
, andAVX512VL
are supported.These ISAs form the
x86-64-v4
baseline and there has never shipped a piece of hardware without all of them.Some examples of things we'd have to consider are that legacy-encoded
xorps
isSSE
and VEX-encodedvxorps
is AVX. However, EVEX-encodedxorps
is AVX512DQ. Likewise the EVEX support for XMM/YMM basedxorps
is thenAVX512DQ + AVX512VL
(AVX512DQ_VL
).Supporting this "properly" would require us to add some fairly complex checks to import and likely LSRA to handle the difference and ensure that some suitable fallback is generated. We could write all the logic to support them, but without hardware existing that will will be "needless" overhead and negatively impact throughput. So its much easier to just write the JIT to disable AVX512 entirely if any of the "core" ISAs are unsupported.