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.

Sign up for GitHub

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

Fixing the InstructionSetDesc implications #86486

Merged
merged 28 commits into from
Jun 2, 2023
Merged

Fixing the InstructionSetDesc implications #86486

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d5e1c77
Fixing the InstructionSetDesc implications
tannergooding May 19, 2023
2cf307d
Merge remote-tracking branch 'dotnet/main' into prefer-vector-width-4
tannergooding May 19, 2023
8e518a6
Adding more NAOT smoke tests covering the missed instruction sets
tannergooding May 19, 2023
5e4eb47
Simplify the HasInstructionSet(Avx512F) check in compSetProcessor
tannergooding May 19, 2023
89b5aff
Fixing the NAOT smoke tests
tannergooding May 19, 2023
e4831f2
Fixing some stale comments
tannergooding May 19, 2023
a3fc57d
Fixing build failure
tannergooding May 19, 2023
25843cb
Ensure the X86Serialize test lambda returns a bool
tannergooding May 19, 2023
83cdd19
Fixing build failure
tannergooding May 19, 2023
38c0005
Merge remote-tracking branch 'dotnet/main' into prefer-vector-width-4
tannergooding May 20, 2023
c4e9163
Ensure AVX2 isn't opportunistically supported and that dynamic checks…
tannergooding May 20, 2023
3f771fd
Ensure Avx512Vbmi has [Intrinsic] on the right members
tannergooding May 21, 2023
c549949
Fix the secondary isIsaSupported check to be properly opportunistic f…
tannergooding May 22, 2023
569d624
Ensure vpermb is covered
tannergooding May 22, 2023
8b464fa
Allow opportunistic AvxVnni when Avx2 is opted into
tannergooding May 22, 2023
49ec14d
Don't expect opportunistic Avx2 or AvxVnni in the smoke tests
tannergooding May 22, 2023
e7d7146
Ensure Avx2.X64 checks ExpectedAvx2, not ExpectedAvx
tannergooding May 22, 2023
209938b
Merge remote-tracking branch 'dotnet/main' into prefer-vector-width-4
tannergooding May 23, 2023
ba1316e
Change the filter the AVX512 NAOT smoke test on OSX
tannergooding May 23, 2023
2468ef2
Merge remote-tracking branch 'dotnet/main' into prefer-vector-width-4
tannergooding Jun 1, 2023
86ed734
Updating the CPUID test to cover new ISAs and correctly validate the …
tannergooding Jun 1, 2023
53e7858
Add two more NAOT smoke tests which cover explicit ISA exclusion
tannergooding Jun 1, 2023
2447b21
Add additional CpuId validation covering R2R scenarios using various …
tannergooding Jun 1, 2023
e9f8b88
Ensure new R2R tests actually use R2R
tannergooding Jun 1, 2023
da584c5
Don't try to expose an invalid --instruction-set combination
tannergooding Jun 1, 2023
67f309b
Ensure xarch r2r tests only run on xarch
tannergooding Jun 1, 2023
0c69dbd
Don't compare manufacturer name of CPUID 0x00000000 to 0x80000000
tannergooding Jun 1, 2023
5c47316
Filter out the CPUID test on Mono and account for AVX-512 being unsup…
tannergooding Jun 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/coreclr/inc/corinfoinstructionset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,24 +566,40 @@ inline CORINFO_InstructionSetFlags EnsureInstructionSetFlagsAreValid(CORINFO_Ins
resultflags.RemoveInstructionSet(InstructionSet_X86Serialize);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX2))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_FMA))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512CD) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512CD);
if (resultflags.HasInstructionSet(InstructionSet_AVX512CD_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512CD))
resultflags.RemoveInstructionSet(InstructionSet_AVX512CD_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512CD_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512CD_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512BW) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512BW);
if (resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW))
resultflags.RemoveInstructionSet(InstructionSet_AVX512BW_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512BW_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512DQ) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512DQ);
if (resultflags.HasInstructionSet(InstructionSet_AVX512DQ_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512DQ))
resultflags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512DQ_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512VBMI) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW))
resultflags.RemoveInstructionSet(InstructionSet_AVX512VBMI);
if (resultflags.HasInstructionSet(InstructionSet_AVX512VBMI_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512VBMI))
resultflags.RemoveInstructionSet(InstructionSet_AVX512VBMI_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512VBMI_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512VBMI_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX512CD_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX512DQ_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
#endif // TARGET_AMD64
#ifdef TARGET_X86
if (resultflags.HasInstructionSet(InstructionSet_SSE) && !resultflags.HasInstructionSet(InstructionSet_X86Base))
Expand Down Expand Up @@ -630,24 +646,40 @@ inline CORINFO_InstructionSetFlags EnsureInstructionSetFlagsAreValid(CORINFO_Ins
resultflags.RemoveInstructionSet(InstructionSet_X86Serialize);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX2))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_FMA))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512CD) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512CD);
if (resultflags.HasInstructionSet(InstructionSet_AVX512CD_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512CD))
resultflags.RemoveInstructionSet(InstructionSet_AVX512CD_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512CD_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512CD_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512BW) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512BW);
if (resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW))
resultflags.RemoveInstructionSet(InstructionSet_AVX512BW_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512BW_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512DQ) && !resultflags.HasInstructionSet(InstructionSet_AVX512F))
resultflags.RemoveInstructionSet(InstructionSet_AVX512DQ);
if (resultflags.HasInstructionSet(InstructionSet_AVX512DQ_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512DQ))
resultflags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512DQ_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512F_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512VBMI) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW))
resultflags.RemoveInstructionSet(InstructionSet_AVX512VBMI);
if (resultflags.HasInstructionSet(InstructionSet_AVX512VBMI_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512VBMI))
resultflags.RemoveInstructionSet(InstructionSet_AVX512VBMI_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512VBMI_VL) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512VBMI_VL);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX512BW_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX512CD_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
if (resultflags.HasInstructionSet(InstructionSet_AVX512F) && !resultflags.HasInstructionSet(InstructionSet_AVX512DQ_VL))
resultflags.RemoveInstructionSet(InstructionSet_AVX512F);
#endif // TARGET_X86

} while (!oldflags.Equals(resultflags));
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* dfc41bc9-f134-4c50-897e-fc9304a82059 */
0xdfc41bc9,
0xf134,
0x4c50,
{0x89, 0x7e, 0xfc, 0x93, 0x04, 0xa8, 0x20, 0x59}
constexpr GUID JITEEVersionIdentifier = { /* d4414be1-70e4-46ac-8866-ca3a6c2f8422 */
0xd4414be1,
0x70e4,
0x46ac,
{0x88, 0x66, 0xca, 0x3a, 0x6c, 0x2f, 0x84, 0x22}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
37 changes: 6 additions & 31 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2287,15 +2287,16 @@ void Compiler::compSetProcessor()
// the overall JIT implementation, we currently require the entire set of ISAs to be
// supported and disable AVX512 support otherwise.

if (instructionSetFlags.HasInstructionSet(InstructionSet_AVX512BW_VL) &&
instructionSetFlags.HasInstructionSet(InstructionSet_AVX512CD_VL) &&
instructionSetFlags.HasInstructionSet(InstructionSet_AVX512DQ_VL))
if (instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F))
{
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F_VL));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512BW));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512BW_VL));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512CD));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512CD_VL));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512DQ));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512F_VL));
assert(instructionSetFlags.HasInstructionSet(InstructionSet_AVX512DQ_VL));

instructionSetFlags.AddInstructionSet(InstructionSet_Vector512);

Expand All @@ -2310,32 +2311,6 @@ void Compiler::compSetProcessor()
preferredVectorByteLength = 256 / 8;
}
}
else
{
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512F_VL);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512BW_VL);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512CD_VL);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512DQ_VL);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512VBMI);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512VBMI_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_AVX512VBMI_X64);
instructionSetFlags.RemoveInstructionSet(InstructionSet_AVX512VBMI_VL_X64);
#endif // TARGET_AMD64
}

opts.preferredVectorByteLength = preferredVectorByteLength;
#elif defined(TARGET_ARM64)
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8645,7 +8645,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX

// Get the number of bytes in a System.Numeric.Vector<T> for the current compilation.
// Note - cannot be used for System.Runtime.Intrinsic
unsigned getVectorTByteLength()
uint32_t getVectorTByteLength()
{
// We need to report the ISA dependency to the VM so that scenarios
// such as R2R work correctly for larger vector sizes, so we always
Expand Down
18 changes: 9 additions & 9 deletions src/coreclr/jit/hwintrinsic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,9 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
return NI_Illegal;
}

bool isIsaSupported = comp->compSupportsHWIntrinsic(isa);

bool isIsaSupported = comp->compSupportsHWIntrinsic(isa);
bool isHardwareAcceleratedProp = (strcmp(methodName, "get_IsHardwareAccelerated") == 0);

#ifdef TARGET_XARCH
if (isHardwareAcceleratedProp)
{
Expand Down Expand Up @@ -541,22 +541,22 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
//
// When the target hardware does support the instruction set, we can return a
// constant true. When it doesn't then we want to report the check as dynamically
// supported instead. This allows some targets, such as AOT, to emit a check against
// a cached CPU query so lightup can still happen (such as for SSE4.1 when the target
// hardware is SSE2).
// supported instead if the opportunistic support does exist. This allows some targets,
// such as AOT, to emit a check against a cached CPU query so lightup can still happen
// (such as for SSE4.1 when the target hardware is SSE2).
//
// When the compiler doesn't support ISA or when it does but the target hardware does
// not and we aren't in a scenario with support for a dynamic check, we want to return false.

if (isIsaSupported)
if (isIsaSupported && comp->compSupportsHWIntrinsic(isa))
{
if (comp->compExactlyDependsOn(isa))
if (!comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI) || comp->compExactlyDependsOn(isa))
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain what change is happening here? I don't see a description of what we're changing around NativeAOT behavior in the change description.

Copy link
Member Author

Choose a reason for hiding this comment

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

There was an existing bug here that was surfaced if crossgen/naot targeted avx but not avx2

For most cases, the ISA initially checked and tracked as part of isIsaSupported is the same as what is tracked by the InstructionSetDesc

However, for Vector256 in particular we have the case where the implication is on Avx and we will accelerate some APIs when only Avx is supported. But, we only want IsHardwareAccelerated to report true when Avx2 is also supported.

We were then ending up in a scenario where we'd end up failing to handle IsHardwareAccelerated for the recursive case when avx was supported but avx2 was not because isIsaSupported (AVX) would be true and then we'd fail the compExactlyDependsOn check for AVX2 and then return NI_IsSupported_Dynamic, which was incorrect since avx2 was not opportunistic.

This fixes that so we now ensure that we only go down the true/dynamic path if the compiler could support AVX2 at all.

{
return NI_IsSupported_True;
}

if (comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI))
else
{
assert(comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI));
return NI_IsSupported_Dynamic;
}
}
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/nativeaot/Runtime/IntrinsicConstants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum XArchIntrinsicConstants
XArchIntrinsicConstants_Avx512dq_vl = 0x400000,
XArchIntrinsicConstants_Avx512Vbmi = 0x800000,
XArchIntrinsicConstants_Avx512Vbmi_vl = 0x1000000,
XArchIntrinsicConstants_Serialize = 0x2000000,
};
#endif //HOST_X86 || HOST_AMD64

Expand Down
Loading