Skip to content

Commit

Permalink
Ensure that the CpuId tests set preferredVectorByteLength to a non-ze…
Browse files Browse the repository at this point in the history
…ro value (#88623)

* Ensure that the CpuId tests set preferredVectorByteLength to a non-zero value

* Ensure getPreferredVectorByteLength takes NAOT into account

* Don't condition the preferred vector byte length based on stress mode
  • Loading branch information
tannergooding authored Jul 11, 2023
1 parent f44e2e6 commit 83bf4b6
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 22 deletions.
9 changes: 3 additions & 6 deletions src/coreclr/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2306,13 +2306,10 @@ void Compiler::compSetProcessor()
// users can override this with `DOTNET_PreferredVectorBitWidth=512` to
// allow using such instructions where hardware support is available.
//
// Under stress, sometimes leave the preferred vector width at 512, even if that means
// throttling. This helps with test coverage on test machines that might be older.
// Do not condition this based on stress mode as it makes the support
// reported inconsistent across methods and breaks expectations/functionality

if (!compStressCompile(STRESS_GENERIC_VARN, 20))
{
preferredVectorByteLength = 256 / 8;
}
preferredVectorByteLength = 256 / 8;
}
}

Expand Down
26 changes: 12 additions & 14 deletions src/coreclr/jit/hwintrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,9 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
return NI_Illegal;
}

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

#ifdef TARGET_XARCH
if (isHardwareAcceleratedProp)
Expand All @@ -505,25 +506,21 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
// but we want IsHardwareAccelerated to return true only when all of them are (there are
// still can be cases where e.g. Sse41 might give an additional boost for Vector128, but it's
// not important enough to bump the minimal Sse version here)

if (strcmp(className, "Vector128") == 0)
{
isa = InstructionSet_SSE2;
isa = InstructionSet_SSE2;
vectorByteLength = 16;
}
else if (strcmp(className, "Vector256") == 0)
{
if (comp->getPreferredVectorByteLength() < 32)
{
return NI_IsSupported_False;
}
isa = InstructionSet_AVX2;
isa = InstructionSet_AVX2;
vectorByteLength = 32;
}
else if (strcmp(className, "Vector512") == 0)
{
if (comp->getPreferredVectorByteLength() < 64)
{
return NI_IsSupported_False;
}
isa = InstructionSet_AVX512F;
isa = InstructionSet_AVX512F;
vectorByteLength = 64;
}
}
#endif
Expand Down Expand Up @@ -556,7 +553,8 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
// 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 && comp->compSupportsHWIntrinsic(isa))
if (isIsaSupported && comp->compSupportsHWIntrinsic(isa) &&
(vectorByteLength <= comp->getPreferredVectorByteLength()))
{
if (!comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI) || comp->compExactlyDependsOn(isa))
{
Expand Down
6 changes: 5 additions & 1 deletion src/tests/JIT/HardwareIntrinsics/X86/X86Base/CpuId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,14 @@ public unsafe static void CpuId()
}
}

if (isVector512Throttling)
if (isAvx512HierarchyDisabled || isVector512Throttling)
{
preferredVectorByteLength = 256 / 8;
}
else
{
preferredVectorByteLength = 512 / 8;
}
}

if (IsBitIncorrect(ecx, 1, typeof(Avx512Vbmi), Avx512Vbmi.IsSupported, "AVX512VBMI", ref isHierarchyDisabled))
Expand Down
6 changes: 5 additions & 1 deletion src/tests/readytorun/HardwareIntrinsics/X86/CpuId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -251,10 +251,14 @@ public unsafe static int Main()
}
}

if (isVector512Throttling)
if (isAvx512HierarchyDisabled || isVector512Throttling)
{
preferredVectorByteLength = 256 / 8;
}
else
{
preferredVectorByteLength = 512 / 8;
}
}

if (IsBitIncorrect(ecx, 1, typeof(Avx512Vbmi), Avx512Vbmi.IsSupported, "AVX512VBMI", ref isHierarchyDisabled))
Expand Down

0 comments on commit 83bf4b6

Please sign in to comment.