Skip to content

Updating Vector*.IsHardwareAccelerated to be recursive #69578

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1830,6 +1830,7 @@ class Compiler

#ifdef FEATURE_HW_INTRINSICS
friend struct HWIntrinsicInfo;
friend struct SimdAsHWIntrinsicInfo;
#endif // FEATURE_HW_INTRINSICS

#ifndef TARGET_64BIT
Expand Down
34 changes: 32 additions & 2 deletions src/coreclr/jit/hwintrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,38 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,

if ((strcmp(methodName, "get_IsSupported") == 0) || isHardwareAcceleratedProp)
{
return isIsaSupported ? (comp->compExactlyDependsOn(isa) ? NI_IsSupported_True : NI_IsSupported_Dynamic)
: NI_IsSupported_False;
// The `compSupportsHWIntrinsic` above validates `compSupportsIsa` indicating
// that the compiler can emit instructions for the ISA but not whether the
// hardware supports them.
//
// The `compExactlyDependsOn` on call then validates that the target hardware
// supports the instruction. Normally this is the same ISA as we just checked
// but for Vector128/256 on xarch this can be a different ISA since we accelerate
// some APIs even when we can't accelerate all APIs.
//
// 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).
//
// 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 (comp->compExactlyDependsOn(isa))
{
return NI_IsSupported_True;
}

if (comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI))
{
return NI_IsSupported_Dynamic;
}
}

return NI_IsSupported_False;
}
else if (!isIsaSupported)
{
Expand Down
25 changes: 22 additions & 3 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5966,17 +5966,36 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
result = NI_System_Numerics_BitOperations_PopCount;
}
}
#ifdef FEATURE_HW_INTRINSICS
else if (strcmp(namespaceName, "System.Numerics") == 0)
{
#ifdef FEATURE_HW_INTRINSICS
CORINFO_SIG_INFO sig;
info.compCompHnd->getMethodSig(method, &sig);

int sizeOfVectorT = getSIMDVectorRegisterByteLength();

result = SimdAsHWIntrinsicInfo::lookupId(&sig, className, methodName, enclosingClassName, sizeOfVectorT);
}
result = SimdAsHWIntrinsicInfo::lookupId(this, &sig, className, methodName, enclosingClassName, sizeOfVectorT);
#endif // FEATURE_HW_INTRINSICS

if (result == NI_Illegal)
{
if (strcmp(methodName, "get_IsHardwareAccelerated") == 0)
{
// This allows the relevant code paths to be dropped as dead code even
// on platforms where FEATURE_HW_INTRINSICS is not supported.

result = NI_IsSupported_False;
}
else if (gtIsRecursiveCall(method))
{
// For the framework itself, any recursive intrinsics will either be
// only supported on a single platform or will be guarded by a relevant
// IsSupported check so the throw PNSE will be valid or dropped.

result = NI_Throw_PlatformNotSupportedException;
}
}
}
else if (strcmp(namespaceName, "System.Runtime.CompilerServices") == 0)
{
if (strcmp(className, "Unsafe") == 0)
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/jit/simdashwintrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ const SimdAsHWIntrinsicInfo& SimdAsHWIntrinsicInfo::lookup(NamedIntrinsic id)
//
// Return Value:
// The NamedIntrinsic associated with methodName and classId
NamedIntrinsic SimdAsHWIntrinsicInfo::lookupId(CORINFO_SIG_INFO* sig,
NamedIntrinsic SimdAsHWIntrinsicInfo::lookupId(Compiler* comp,
CORINFO_SIG_INFO* sig,
const char* className,
const char* methodName,
const char* enclosingClassName,
Expand All @@ -73,6 +74,11 @@ NamedIntrinsic SimdAsHWIntrinsicInfo::lookupId(CORINFO_SIG_INFO* sig,
isInstanceMethod = true;
}

if (strcmp(methodName, "get_IsHardwareAccelerated") == 0)
{
return comp->IsBaselineSimdIsaSupported() ? NI_IsSupported_True : NI_IsSupported_False;
}

for (int i = 0; i < (NI_SIMD_AS_HWINTRINSIC_END - NI_SIMD_AS_HWINTRINSIC_START - 1); i++)
{
const SimdAsHWIntrinsicInfo& intrinsicInfo = simdAsHWIntrinsicInfoArray[i];
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/jit/simdashwintrinsic.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ struct SimdAsHWIntrinsicInfo

static const SimdAsHWIntrinsicInfo& lookup(NamedIntrinsic id);

static NamedIntrinsic lookupId(CORINFO_SIG_INFO* sig,
static NamedIntrinsic lookupId(Compiler* comp,
CORINFO_SIG_INFO* sig,
const char* className,
const char* methodName,
const char* enclosingClassName,
Expand Down
7 changes: 5 additions & 2 deletions src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3924,8 +3924,11 @@ private bool notifyInstructionSetUsage(InstructionSet instructionSet, bool suppo
else
{
// By policy we code review all changes into corelib, such that failing to use an instruction
// set is not a reason to not support usage of it.
if (!isMethodDefinedInCoreLib())
// set is not a reason to not support usage of it. Except for functions which check if a given
// feature is supported or hardware accelerated.
if (!isMethodDefinedInCoreLib() ||
MethodBeingCompiled.Name == "get_IsSupported" ||
MethodBeingCompiled.Name == "get_IsHardwareAccelerated")
{
_actualInstructionSetUnsupported.AddInstructionSet(instructionSet);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ static GenericVectorTests()
dummy = System.Numerics.Vector<float>.One;
}

[Fact]
public unsafe void IsHardwareAcceleratedTest()
{
MethodInfo methodInfo = typeof(Vector).GetMethod("get_IsHardwareAccelerated");
Assert.Equal(Vector.IsHardwareAccelerated, methodInfo.Invoke(null, null));
}

#region Constructor Tests

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static partial class Vector
public static bool IsHardwareAccelerated
{
[Intrinsic]
get => false;
get => IsHardwareAccelerated;
}

/// <summary>Computes the absolute value of each element in a vector.</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public static class Vector128
public static bool IsHardwareAccelerated
{
[Intrinsic]
get => false;
get => IsHardwareAccelerated;
}

/// <summary>Computes the absolute value of each element in a vector.</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static class Vector256
public static bool IsHardwareAccelerated
{
[Intrinsic]
get => false;
get => IsHardwareAccelerated;
}

/// <summary>Computes the absolute value of each element in a vector.</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static class Vector64
public static bool IsHardwareAccelerated
{
[Intrinsic]
get => false;
get => IsHardwareAccelerated;
}

/// <summary>Computes the absolute value of each element in a vector.</summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ namespace System.Runtime.Intrinsics.Tests.Vectors
{
public sealed class Vector128Tests
{
[Fact]
public unsafe void Vector128IsHardwareAcceleratedTest()
{
MethodInfo methodInfo = typeof(Vector128).GetMethod("get_IsHardwareAccelerated");
Assert.Equal(Vector128.IsHardwareAccelerated, methodInfo.Invoke(null, null));
}

[Fact]
public unsafe void Vector128ByteExtractMostSignificantBitsTest()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ namespace System.Runtime.Intrinsics.Tests.Vectors
{
public sealed class Vector256Tests
{
[Fact]
public unsafe void Vector256IsHardwareAcceleratedTest()
{
MethodInfo methodInfo = typeof(Vector256).GetMethod("get_IsHardwareAccelerated");
Assert.Equal(Vector256.IsHardwareAccelerated, methodInfo.Invoke(null, null));
}

[Fact]
public unsafe void Vector256ByteExtractMostSignificantBitsTest()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ namespace System.Runtime.Intrinsics.Tests.Vectors
{
public sealed class Vector64Tests
{
[Fact]
public unsafe void Vector64IsHardwareAcceleratedTest()
{
MethodInfo methodInfo = typeof(Vector64).GetMethod("get_IsHardwareAccelerated");
Assert.Equal(Vector64.IsHardwareAccelerated, methodInfo.Invoke(null, null));
}

[Fact]
public unsafe void Vector64ByteExtractMostSignificantBitsTest()
{
Expand Down
10 changes: 10 additions & 0 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -2470,6 +2470,16 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
(!strncmp ("System.Runtime.Intrinsics.Arm", klass_name_space, 29) ||
!strncmp ("System.Runtime.Intrinsics.X86", klass_name_space, 29))) {
interp_generate_void_throw (td, MONO_JIT_ICALL_mono_throw_platform_not_supported);
} else if (in_corlib &&
(!strncmp ("System.Numerics", klass_name_space, 15) &&
!strcmp ("Vector", klass_name) &&
!strcmp (tm, "get_IsHardwareAccelerated"))) {
*op = MINT_LDC_I4_0;
} else if (in_corlib &&
(!strncmp ("System.Runtime.Intrinsics", klass_name_space, 25) &&
!strncmp ("Vector", klass_name, 6) &&
!strcmp (tm, "get_IsHardwareAccelerated"))) {
*op = MINT_LDC_I4_0;
}

return FALSE;
Expand Down