From 1d2a385bde30187b056d0cb527cfe3045fb73742 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 19 May 2022 14:09:48 -0700 Subject: [PATCH 01/11] Updating Vector*.IsHardwareAccelerated to be recursive so it works in the debugger --- .../System.Private.CoreLib/src/System/Numerics/Vector.cs | 2 +- .../src/System/Runtime/Intrinsics/Vector128.cs | 2 +- .../src/System/Runtime/Intrinsics/Vector256.cs | 2 +- .../src/System/Runtime/Intrinsics/Vector64.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector.cs b/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector.cs index b9eb29598bb6e..2e504cbf110bc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Numerics/Vector.cs @@ -18,7 +18,7 @@ public static partial class Vector public static bool IsHardwareAccelerated { [Intrinsic] - get => false; + get => IsHardwareAccelerated; } /// Computes the absolute value of each element in a vector. diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128.cs index 9995bc1d447ea..630acbd6e5132 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector128.cs @@ -44,7 +44,7 @@ public static class Vector128 public static bool IsHardwareAccelerated { [Intrinsic] - get => false; + get => IsHardwareAccelerated; } /// Computes the absolute value of each element in a vector. diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector256.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector256.cs index 27796494f0ade..d9eae53d73167 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector256.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector256.cs @@ -45,7 +45,7 @@ public static class Vector256 public static bool IsHardwareAccelerated { [Intrinsic] - get => false; + get => IsHardwareAccelerated; } /// Computes the absolute value of each element in a vector. diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector64.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector64.cs index b84bae7ab015f..def97ea6320bb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector64.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Vector64.cs @@ -21,7 +21,7 @@ public static class Vector64 public static bool IsHardwareAccelerated { [Intrinsic] - get => false; + get => IsHardwareAccelerated; } /// Computes the absolute value of each element in a vector. From 8ed8977921f8f05f0259afc33a2e0a6844b837d2 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 19 May 2022 14:21:45 -0700 Subject: [PATCH 02/11] Adding tests validating indirect and direct invocation of get_IsHardwareAccelerated return the same value --- .../System.Numerics.Vectors/tests/GenericVectorTests.cs | 7 +++++++ .../tests/Vectors/Vector128Tests.cs | 7 +++++++ .../tests/Vectors/Vector256Tests.cs | 7 +++++++ .../tests/Vectors/Vector64Tests.cs | 7 +++++++ 4 files changed, 28 insertions(+) diff --git a/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs b/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs index c7fa1feeea4d7..225b81b21c6e7 100644 --- a/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs +++ b/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs @@ -25,6 +25,13 @@ static GenericVectorTests() dummy = System.Numerics.Vector.One; } + [Fact] + public unsafe void IsHardwareAcceleratedTest() + { + var methodInfo = typeof(Vector).GetMethod("get_IsHardwareAccelerated"); + Assert.Equal(Vector.IsHardwareAccelerated, methodInfo.Invoke(null, null)); + } + #region Constructor Tests [Fact] diff --git a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs index f5b991552bb51..81efdbd200c91 100644 --- a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs +++ b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs @@ -8,6 +8,13 @@ namespace System.Runtime.Intrinsics.Tests.Vectors { public sealed class Vector128Tests { + [Fact] + public unsafe void Vector128IsHardwareAcceleratedTest() + { + var methodInfo = typeof(Vector128).GetMethod("get_IsHardwareAccelerated"); + Assert.Equal(Vector128.IsHardwareAccelerated, methodInfo.Invoke(null, null)); + } + [Fact] public unsafe void Vector128ByteExtractMostSignificantBitsTest() { diff --git a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs index 5662ef9b56c7f..9a8f407770b6c 100644 --- a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs +++ b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs @@ -8,6 +8,13 @@ namespace System.Runtime.Intrinsics.Tests.Vectors { public sealed class Vector256Tests { + [Fact] + public unsafe void Vector256IsHardwareAcceleratedTest() + { + var methodInfo = typeof(Vector256).GetMethod("get_IsHardwareAccelerated"); + Assert.Equal(Vector256.IsHardwareAccelerated, methodInfo.Invoke(null, null)); + } + [Fact] public unsafe void Vector256ByteExtractMostSignificantBitsTest() { diff --git a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs index 64e37290d1930..840daa73c22f0 100644 --- a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs +++ b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs @@ -8,6 +8,13 @@ namespace System.Runtime.Intrinsics.Tests.Vectors { public sealed class Vector64Tests { + [Fact] + public unsafe void Vector64IsHardwareAcceleratedTest() + { + var methodInfo = typeof(Vector64).GetMethod("get_IsHardwareAccelerated"); + Assert.Equal(Vector64.IsHardwareAccelerated, methodInfo.Invoke(null, null)); + } + [Fact] public unsafe void Vector64ByteExtractMostSignificantBitsTest() { From 34794f19462d6b072adccad8f0fd7587a3a6275f Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 19 May 2022 15:14:26 -0700 Subject: [PATCH 03/11] Ensure that Vector.IsHardwareAccelerated supports being recursive --- src/coreclr/jit/importer.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index e2360e7f7c80d..909011a0d3111 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -5810,17 +5810,25 @@ 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); - } #endif // FEATURE_HW_INTRINSICS + + if (result == NI_Illegal) + { + if (strcmp(methodName, "get_IsHardwareAccelerated") == 0) + { + return IsBaselineSimdIsaSupported() ? NI_IsSupported_True : NI_IsSupported_False; + } + } + } else if (strcmp(namespaceName, "System.Runtime.CompilerServices") == 0) { if (strcmp(className, "Unsafe") == 0) From 0a2951dcc9421d2c4463b5e848699769488614a4 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 19 May 2022 15:19:20 -0700 Subject: [PATCH 04/11] Apply suggestions from code review Co-authored-by: Stephen Toub --- .../System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs | 2 +- .../System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs | 2 +- .../System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs index 81efdbd200c91..c38365c2c282d 100644 --- a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs +++ b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector128Tests.cs @@ -11,7 +11,7 @@ public sealed class Vector128Tests [Fact] public unsafe void Vector128IsHardwareAcceleratedTest() { - var methodInfo = typeof(Vector128).GetMethod("get_IsHardwareAccelerated"); + MethodInfo methodInfo = typeof(Vector128).GetMethod("get_IsHardwareAccelerated"); Assert.Equal(Vector128.IsHardwareAccelerated, methodInfo.Invoke(null, null)); } diff --git a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs index 9a8f407770b6c..fbd7715e6ba52 100644 --- a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs +++ b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector256Tests.cs @@ -11,7 +11,7 @@ public sealed class Vector256Tests [Fact] public unsafe void Vector256IsHardwareAcceleratedTest() { - var methodInfo = typeof(Vector256).GetMethod("get_IsHardwareAccelerated"); + MethodInfo methodInfo = typeof(Vector256).GetMethod("get_IsHardwareAccelerated"); Assert.Equal(Vector256.IsHardwareAccelerated, methodInfo.Invoke(null, null)); } diff --git a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs index 840daa73c22f0..c9643eef1f8c9 100644 --- a/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs +++ b/src/libraries/System.Runtime.Intrinsics/tests/Vectors/Vector64Tests.cs @@ -11,7 +11,7 @@ public sealed class Vector64Tests [Fact] public unsafe void Vector64IsHardwareAcceleratedTest() { - var methodInfo = typeof(Vector64).GetMethod("get_IsHardwareAccelerated"); + MethodInfo methodInfo = typeof(Vector64).GetMethod("get_IsHardwareAccelerated"); Assert.Equal(Vector64.IsHardwareAccelerated, methodInfo.Invoke(null, null)); } From e44e041bfc0b01053d1469069ef87887436ab537 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 19 May 2022 15:19:55 -0700 Subject: [PATCH 05/11] Update src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs --- .../System.Numerics.Vectors/tests/GenericVectorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs b/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs index 225b81b21c6e7..bc88267815fba 100644 --- a/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs +++ b/src/libraries/System.Numerics.Vectors/tests/GenericVectorTests.cs @@ -28,7 +28,7 @@ static GenericVectorTests() [Fact] public unsafe void IsHardwareAcceleratedTest() { - var methodInfo = typeof(Vector).GetMethod("get_IsHardwareAccelerated"); + MethodInfo methodInfo = typeof(Vector).GetMethod("get_IsHardwareAccelerated"); Assert.Equal(Vector.IsHardwareAccelerated, methodInfo.Invoke(null, null)); } From 2c8e7bd008c1600ce4d060da01203356448de4fc Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 22 Aug 2022 16:44:20 -0700 Subject: [PATCH 06/11] Detect the one legal violation of the behaves the same rule for intrinsics in CoreLib - The functions which detect if an intrinsic is actually available are allowed to have behavior which differs based on which intrinsics are available at runtime --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index c1f59c124b676..8369451a14397 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -3890,8 +3890,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); } From e378e950069ed3a8e5738f5c1fdbfb02d227ed36 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Tue, 13 Sep 2022 20:19:30 -0400 Subject: [PATCH 07/11] [mono] Intrinsify IsHardwareAccelerated in the interpreter. --- src/mono/mono/mini/interp/transform.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index d76a5c121b789..6ea7d1483e965 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -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; From fbe4b30ead8b52867f345f451d6fc951b233da61 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 14 Sep 2022 08:45:00 -0700 Subject: [PATCH 08/11] Add a fallback path for unhandled recursive intrinsics in System.Numerics --- src/coreclr/jit/importer.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c45a069ffacae..f6cbd00d072ed 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -5981,7 +5981,15 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { if (strcmp(methodName, "get_IsHardwareAccelerated") == 0) { - return IsBaselineSimdIsaSupported() ? NI_IsSupported_True : NI_IsSupported_False; + result = IsBaselineSimdIsaSupported() ? NI_IsSupported_True : 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; } } } From 2640d4083a0d4dbc43a1516d45e9b4752bbabd79 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 14 Sep 2022 08:52:43 -0700 Subject: [PATCH 09/11] Move the Vector.IsHardwareAccelerated handling into SimdAsHWIntrinsicInfo.lookupId --- src/coreclr/jit/importer.cpp | 5 ++++- src/coreclr/jit/simdashwintrinsic.cpp | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index f6cbd00d072ed..f330b84c89fc9 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -5981,7 +5981,10 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { if (strcmp(methodName, "get_IsHardwareAccelerated") == 0) { - result = IsBaselineSimdIsaSupported() ? NI_IsSupported_True : NI_IsSupported_False; + // 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)) { diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index ac19a94ade999..432c1eaa11053 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -73,6 +73,11 @@ NamedIntrinsic SimdAsHWIntrinsicInfo::lookupId(CORINFO_SIG_INFO* sig, isInstanceMethod = true; } + if (strcmp(methodName, "get_IsHardwareAccelerated") == 0) + { + return 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]; From c36498a77da48d8d481295b0b25fb34dc1e53ca0 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 14 Sep 2022 11:10:07 -0700 Subject: [PATCH 10/11] Fixinng a compiler error due to an unresolved member --- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/importer.cpp | 2 +- src/coreclr/jit/simdashwintrinsic.cpp | 5 +++-- src/coreclr/jit/simdashwintrinsic.h | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index f98fe75b61bd2..0809a1421d994 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1830,6 +1830,7 @@ class Compiler #ifdef FEATURE_HW_INTRINSICS friend struct HWIntrinsicInfo; + friend struct SimdAsHWIntrinsicInfo; #endif // FEATURE_HW_INTRINSICS #ifndef TARGET_64BIT diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index f330b84c89fc9..c42da86a1e8f1 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -5974,7 +5974,7 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) 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) diff --git a/src/coreclr/jit/simdashwintrinsic.cpp b/src/coreclr/jit/simdashwintrinsic.cpp index 432c1eaa11053..6d9da02d0bb9b 100644 --- a/src/coreclr/jit/simdashwintrinsic.cpp +++ b/src/coreclr/jit/simdashwintrinsic.cpp @@ -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, @@ -75,7 +76,7 @@ NamedIntrinsic SimdAsHWIntrinsicInfo::lookupId(CORINFO_SIG_INFO* sig, if (strcmp(methodName, "get_IsHardwareAccelerated") == 0) { - return IsBaselineSimdIsaSupported() ? NI_IsSupported_True : NI_IsSupported_False; + 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++) diff --git a/src/coreclr/jit/simdashwintrinsic.h b/src/coreclr/jit/simdashwintrinsic.h index ae377ff574010..cea53927c815e 100644 --- a/src/coreclr/jit/simdashwintrinsic.h +++ b/src/coreclr/jit/simdashwintrinsic.h @@ -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, From cde95a2fc02c1ccfe5f29b3518f472c8b348d7d8 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 15 Sep 2022 16:12:31 -0700 Subject: [PATCH 11/11] Adjust the NI_IsSupported_Dynamic checks to be NAOT only --- src/coreclr/jit/hwintrinsic.cpp | 34 +++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index 33e8530debb36..654b93b890502 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -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) {