diff --git a/src/coreclr/jit/hwintrinsic.cpp b/src/coreclr/jit/hwintrinsic.cpp index f9808f6ff44063..3ad52f2b209912 100644 --- a/src/coreclr/jit/hwintrinsic.cpp +++ b/src/coreclr/jit/hwintrinsic.cpp @@ -1204,11 +1204,18 @@ static NamedIntrinsic binarySearchId(CORINFO_InstructionSet isa, // lookupId: Gets the NamedIntrinsic for a given method name and InstructionSet // // Arguments: -// comp -- The compiler -// sig -- The signature of the intrinsic -// className -- The name of the class associated with the HWIntrinsic to lookup -// methodName -- The name of the method associated with the HWIntrinsic to lookup -// enclosingClassName -- The name of the enclosing class of X64 classes +// comp -- The compiler +// sig -- The signature of the intrinsic +// className -- The name of the class associated with the HWIntrinsic to lookup +// methodName -- The name of the method associated with the HWIntrinsic to lookup +// innerEnclosingClassName -- The name of the inner enclosing class of nested 64-bit classes +// outerEnclosingClassName -- The name of the outer enclosing class of nested 64-bit classes +// isXplatIntrinsic -- True if the intrinsic lives directly under the cross-platform +// System.Runtime.Intrinsics (or System.Numerics) namespace and +// therefore has a managed fallback when the underlying ISA isn't +// available; false if it is platform-specific (under +// System.Runtime.Intrinsics.) and must throw a +// PlatformNotSupportedException when the ISA isn't available. // // Return Value: // The NamedIntrinsic associated with methodName and isa @@ -1217,7 +1224,8 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp, const char* className, const char* methodName, const char* innerEnclosingClassName, - const char* outerEnclosingClassName) + const char* outerEnclosingClassName, + bool isXplatIntrinsic) { #if defined(DEBUG) static bool validationCompleted = false; @@ -1335,7 +1343,24 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp, } else if (!isIsaSupported) { - return NI_Throw_PlatformNotSupportedException; + // We only want to surface a hard PlatformNotSupportedException when the ISA is + // unsupported AND there is no viable managed fallback. That is only the case for + // the platform-specific APIs under System.Runtime.Intrinsics. (e.g., + // Avx512.LoadVector512). The cross-platform APIs in System.Runtime.Intrinsics + // (e.g., Vector512.Load, Vector128.AsByte) and System.Numerics (e.g., + // Vector) all have a managed fallback that should be used when the underlying + // ISA isn't available. For those, we fall through to the per-ISA handling below + // which returns NI_Illegal, allowing the call to be treated as a normal managed + // method (so the body can be inlined / executed normally). + + if (!isXplatIntrinsic) + { + return NI_Throw_PlatformNotSupportedException; + } + else + { + return NI_Illegal; + } } // Special case: For Vector64/128/256 we currently don't accelerate any of the methods when diff --git a/src/coreclr/jit/hwintrinsic.h b/src/coreclr/jit/hwintrinsic.h index 7b7b211a8f9a75..f492cce89b7f12 100644 --- a/src/coreclr/jit/hwintrinsic.h +++ b/src/coreclr/jit/hwintrinsic.h @@ -532,7 +532,8 @@ struct HWIntrinsicInfo const char* className, const char* methodName, const char* innerEnclosingClassName, - const char* outerEnclosingClassName); + const char* outerEnclosingClassName, + bool isXplatIntrinsic); static unsigned lookupSimdSize(Compiler* comp, NamedIntrinsic id, CORINFO_SIG_INFO* sig); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index b974078d3a83ae..851e08288b3d35 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -3247,7 +3247,7 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, default: { - return impUnsupportedNamedIntrinsic(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, method, sig, + return impUnsupportedNamedIntrinsic(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, method, sig, mustExpand); } } @@ -5206,9 +5206,9 @@ GenTree* Compiler::impIntrinsic(CORINFO_CLASS_HANDLE clsHnd, #ifdef TARGET_WASM NYI_WASM("Unhandled must expand intrinsic"); #else - assert(!"Unhandled must expand intrinsic, throwing PlatformNotSupportedException"); + assert(!"Unhandled must expand intrinsic, throwing NotImplementedException"); #endif - return impUnsupportedNamedIntrinsic(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, method, sig, mustExpand); + return impUnsupportedNamedIntrinsic(CORINFO_HELP_THROW_NOT_IMPLEMENTED, method, sig, mustExpand); } // Optionally report if this intrinsic is special @@ -11091,8 +11091,12 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) CORINFO_SIG_INFO sig; info.compCompHnd->getMethodSig(method, &sig); + // System.Numerics.Vector and System.Numerics.Vector are cross-platform + // APIs with managed fallbacks; they must not throw PNSE when the ISA isn't + // available. result = HWIntrinsicInfo::lookupId(this, &sig, lookupClassName, lookupMethodName, - enclosingClassNames[0], enclosingClassNames[1]); + enclosingClassNames[0], enclosingClassNames[1], + /* isXplatIntrinsic */ true); } } #endif // FEATURE_HW_INTRINSICS @@ -11378,13 +11382,18 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) } } - if ((namespaceName[0] == '\0') || (strcmp(namespaceName, platformNamespaceName) == 0)) + bool isXplatIntrinsic = (namespaceName[0] == '\0'); + bool isPlatformMatchedIntrinsic = (strcmp(namespaceName, platformNamespaceName) == 0); + bool isPlatformMismatchedIntrinsic = !isXplatIntrinsic && !isPlatformMatchedIntrinsic; + + if (isXplatIntrinsic || isPlatformMatchedIntrinsic) { CORINFO_SIG_INFO sig; info.compCompHnd->getMethodSig(method, &sig); - result = HWIntrinsicInfo::lookupId(this, &sig, className, methodName, - enclosingClassNames[0], enclosingClassNames[1]); + result = + HWIntrinsicInfo::lookupId(this, &sig, className, methodName, enclosingClassNames[0], + enclosingClassNames[1], isXplatIntrinsic); } #endif // FEATURE_HW_INTRINSICS @@ -11429,6 +11438,16 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) result = NI_Throw_PlatformNotSupportedException; } +#ifdef FEATURE_HW_INTRINSICS + else if (isPlatformMismatchedIntrinsic) + { + // The API lives in a platform-specific sub-namespace under + // System.Runtime.Intrinsics (e.g., .X86 on ARM64, .Wasm on xarch) that + // does not match the target architecture. Such APIs are platform-specific + // with no managed fallback, so they must throw PlatformNotSupportedException. + result = NI_Throw_PlatformNotSupportedException; + } +#endif // FEATURE_HW_INTRINSICS else { // Otherwise mark this as a general intrinsic in the namespace @@ -11555,6 +11574,12 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method) { JITDUMP("Not recognized\n"); } + else if ((result == NI_System_Numerics_Intrinsic) || (result == NI_System_Runtime_Intrinsics_Intrinsic)) + { + // These are special markers used just to ensure we still get the inlining profitability + // boost. We actually have the implementation in managed, however, to keep the JIT simpler. + JITDUMP("Not recognized - inlining boost\n"); + } else if (result == NI_IsSupported_False) { JITDUMP("Unsupported - return false"); @@ -11980,7 +12005,7 @@ NamedIntrinsic Compiler::lookupPrimitiveIntNamedIntrinsic(CORINFO_METHOD_HANDLE // mustExpand - true if the intrinsic must return a GenTree*; otherwise, false // // Return Value: -// a gtNewMustThrowException if mustExpand is true; otherwise, nullptr +// a gtNewMustThrowException if mustExpand is true or optimizations are enabled; otherwise, nullptr // GenTree* Compiler::impUnsupportedNamedIntrinsic(unsigned helper, CORINFO_METHOD_HANDLE method, @@ -11989,18 +12014,36 @@ GenTree* Compiler::impUnsupportedNamedIntrinsic(unsigned helper, { // We've hit some error case and may need to return a node for the given error. // - // When `mustExpand=false`, we are attempting to inline the intrinsic directly into another method. In this - // scenario, we need to return `nullptr` so that a GT_CALL to the intrinsic is emitted instead. This is to - // ensure that everything continues to behave correctly when optimizations are enabled (e.g. things like the - // inliner may expect the node we return to have a certain signature, and the `MustThrowException` node won't - // match that). - // // When `mustExpand=true`, we are in a GT_CALL to the intrinsic and are attempting to JIT it. This will generally // be in response to an indirect call (e.g. done via reflection) or in response to an earlier attempt returning // `nullptr` (under `mustExpand=false`). In that scenario, we are safe to return the `MustThrowException` node. + // + // When `mustExpand=false`, we are attempting to expand the intrinsic directly at the call site (either at the + // root method or while importing an inlinee body). When optimizations are enabled it is preferable to surface + // a hard throw at the unsupported call site rather than fall back to a managed call into the (unsupported) + // intrinsic body, since the body itself will only end up throwing as well. We therefore try to also return the + // `MustThrowException` node when `opts.OptimizationEnabled()` is true. When optimizations are disabled (Tier0 + // / MinOpts / debug code) we keep the legacy behavior of returning `nullptr` and emitting a GT_CALL so as not + // to perturb debugging or tier-up scenarios. + + if ((helper == CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED) && IsAot()) + { + // However, if we're AOT and must expand, we need to throw a PlatformNotSupportedException since there is + // no NAOT/R2R helper for the regular NotSupportedException with the relevant type message. PNSE derives + // from NSE and so it is still accurate for most considerations. We can fix this the next time we bump + // MINIMUM_READYTORUN_MAJOR_VERSION + + if (!mustExpand) + { + return nullptr; + } + helper = CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED; + } - if (mustExpand) + if (mustExpand || opts.OptimizationEnabled()) { + impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("impUnsupportedNamedIntrinsic")); + for (unsigned i = 0; i < sig->numArgs; i++) { impPopStack();