Skip to content

Commit

Permalink
Add support for doubly nested intrinsic classes (dotnet#105761)
Browse files Browse the repository at this point in the history
`System.Runtime.Intrinsics.X86.Avx10v1+V512+X64.IsSupported` currently results
in an infinite loop. This PR fixes the problem by adding support in various
places for hardware intrinsic classes nested more than once.

- Expand `getMethodNameFromMetadata` to allow returning names of multiple
  enclosing classes
- Expand the type loader to recognize intrinsics in up to 2x nested classes
- Fix the JIT recognition to handle potentially multi-nested intrinsic classes
- Fix NAOT support for opportunistically querying for `Avx10v1` support

Fix dotnet#105402
  • Loading branch information
jakobbotsch authored Aug 10, 2024
1 parent b47961a commit 8fee24f
Show file tree
Hide file tree
Showing 29 changed files with 247 additions and 110 deletions.
18 changes: 11 additions & 7 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -2994,15 +2994,19 @@ class ICorStaticInfo
size_t* pRequiredBufferSize = nullptr
) = 0;

// Return method name as in metadata, or nullptr if there is none,
// and optionally return the class, enclosing class, and namespace names
// as in metadata.
// Return method name as in metadata, or nullptr if there is none, and
// optionally return the class, enclosing classes, and namespace name as
// in metadata. Enclosing classes are returned from inner-most enclosed class
// to outer-most, with nullptr in the array indicating that no more
// enclosing classes were left. The namespace returned corresponds to the
// outer most (potentially enclosing) class that was returned.
// Suitable for non-debugging use.
virtual const char* getMethodNameFromMetadata(
CORINFO_METHOD_HANDLE ftn, /* IN */
const char **className, /* OUT */
const char **namespaceName, /* OUT */
const char **enclosingClassName /* OUT */
CORINFO_METHOD_HANDLE ftn, /* IN */
const char** className, /* OUT */
const char** namespaceName, /* OUT */
const char** enclosingClassNames, /* OUT */
size_t maxEnclosingClassNames /* IN */
) = 0;

// this function is for debugging only. It returns a value that
Expand Down
3 changes: 2 additions & 1 deletion src/coreclr/inc/icorjitinfoimpl_generated.h
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,8 @@ const char* getMethodNameFromMetadata(
CORINFO_METHOD_HANDLE ftn,
const char** className,
const char** namespaceName,
const char** enclosingClassName) override;
const char** enclosingClassNames,
size_t maxEnclosingClassNames) override;

unsigned getMethodHash(
CORINFO_METHOD_HANDLE ftn) override;
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
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 = { /* 4f0e8e22-e2e9-4a80-b140-37bbb6e68bca */
0x4f0e8e22,
0xe2e9,
0x4a80,
{0xb1, 0x40, 0x37, 0xbb, 0xb6, 0xe6, 0x8b, 0xca}
constexpr GUID JITEEVersionIdentifier = { /* f43f9022-8795-4791-ba55-c450d76cfeb9 */
0xf43f9022,
0x8795,
0x4791,
{0xba, 0x55, 0xc4, 0x50, 0xd7, 0x6c, 0xfe, 0xb9}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1202,10 +1202,11 @@ const char* WrapICorJitInfo::getMethodNameFromMetadata(
CORINFO_METHOD_HANDLE ftn,
const char** className,
const char** namespaceName,
const char** enclosingClassName)
const char** enclosingClassNames,
size_t maxEnclosingClassNames)
{
API_ENTER(getMethodNameFromMetadata);
const char* temp = wrapHnd->getMethodNameFromMetadata(ftn, className, namespaceName, enclosingClassName);
const char* temp = wrapHnd->getMethodNameFromMetadata(ftn, className, namespaceName, enclosingClassNames, maxEnclosingClassNames);
API_LEAVE(getMethodNameFromMetadata);
return temp;
}
Expand Down
5 changes: 3 additions & 2 deletions src/coreclr/jit/hwintrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,8 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
CORINFO_SIG_INFO* sig,
const char* className,
const char* methodName,
const char* enclosingClassName)
const char* innerEnclosingClassName,
const char* outerEnclosingClassName)
{
#if defined(DEBUG)
static bool validationCompleted = false;
Expand All @@ -998,7 +999,7 @@ NamedIntrinsic HWIntrinsicInfo::lookupId(Compiler* comp,
return NI_Illegal;
}

CORINFO_InstructionSet isa = lookupIsa(className, enclosingClassName);
CORINFO_InstructionSet isa = lookupIsa(className, innerEnclosingClassName, outerEnclosingClassName);

if (isa == InstructionSet_ILLEGAL)
{
Expand Down
7 changes: 5 additions & 2 deletions src/coreclr/jit/hwintrinsic.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,11 @@ struct HWIntrinsicInfo
CORINFO_SIG_INFO* sig,
const char* className,
const char* methodName,
const char* enclosingClassName);
static CORINFO_InstructionSet lookupIsa(const char* className, const char* enclosingClassName);
const char* innerEnclosingClassName,
const char* outerEnclosingClassName);
static CORINFO_InstructionSet lookupIsa(const char* className,
const char* innerEnclosingClassName,
const char* outerEnclosingClassName);

static unsigned lookupSimdSize(Compiler* comp, NamedIntrinsic id, CORINFO_SIG_INFO* sig);

Expand Down
12 changes: 8 additions & 4 deletions src/coreclr/jit/hwintrinsicarm64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,19 @@ static CORINFO_InstructionSet lookupInstructionSet(const char* className)
//
// Arguments:
// className -- The name of the class associated with the InstructionSet to lookup
// enclosingClassName -- The name of the enclosing class or nullptr if one doesn't exist
// innerEnclosingClassName -- The name of the inner enclosing class or nullptr if one doesn't exist
// outerEnclosingClassName -- The name of the outer enclosing class or nullptr if one doesn't exist
//
// Return Value:
// The InstructionSet associated with className and enclosingClassName
CORINFO_InstructionSet HWIntrinsicInfo::lookupIsa(const char* className, const char* enclosingClassName)
//
CORINFO_InstructionSet HWIntrinsicInfo::lookupIsa(const char* className,
const char* innerEnclosingClassName,
const char* outerEnclosingClassName)
{
assert(className != nullptr);

if (enclosingClassName == nullptr)
if (innerEnclosingClassName == nullptr)
{
// No nested class is the most common, so fast path it
return lookupInstructionSet(className);
Expand All @@ -142,7 +146,7 @@ CORINFO_InstructionSet HWIntrinsicInfo::lookupIsa(const char* className, const c
// or intrinsics in the platform specific namespace, we assume
// that it will be one we can handle and don't try to early out.

CORINFO_InstructionSet enclosingIsa = lookupInstructionSet(enclosingClassName);
CORINFO_InstructionSet enclosingIsa = lookupIsa(innerEnclosingClassName, outerEnclosingClassName, nullptr);

if (strcmp(className, "Arm64") == 0)
{
Expand Down
15 changes: 11 additions & 4 deletions src/coreclr/jit/hwintrinsicxarch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ static CORINFO_InstructionSet V512VersionOfIsa(CORINFO_InstructionSet isa)
{
case InstructionSet_AVX10v1:
return InstructionSet_AVX10v1_V512;
case InstructionSet_AVX10v1_X64:
return InstructionSet_AVX10v1_V512_X64;
default:
return InstructionSet_NONE;
}
Expand Down Expand Up @@ -313,15 +315,18 @@ static CORINFO_InstructionSet lookupInstructionSet(const char* className)
//
// Arguments:
// className -- The name of the class associated with the InstructionSet to lookup
// enclosingClassName -- The name of the enclosing class of X64 classes
// innerEnclosingClassName -- The name of the inner enclosing class of X64 classes
// outerEnclosingClassName -- The name of the outer enclosing class of X64 classes
//
// Return Value:
// The InstructionSet associated with className and enclosingClassName
CORINFO_InstructionSet HWIntrinsicInfo::lookupIsa(const char* className, const char* enclosingClassName)
CORINFO_InstructionSet HWIntrinsicInfo::lookupIsa(const char* className,
const char* innerEnclosingClassName,
const char* outerEnclosingClassName)
{
assert(className != nullptr);

if (enclosingClassName == nullptr)
if (innerEnclosingClassName == nullptr)
{
// No nested class is the most common, so fast path it
return lookupInstructionSet(className);
Expand All @@ -331,7 +336,7 @@ CORINFO_InstructionSet HWIntrinsicInfo::lookupIsa(const char* className, const c
// or intrinsics in the platform specific namespace, we assume
// that it will be one we can handle and don't try to early out.

CORINFO_InstructionSet enclosingIsa = lookupInstructionSet(enclosingClassName);
CORINFO_InstructionSet enclosingIsa = lookupIsa(innerEnclosingClassName, outerEnclosingClassName, nullptr);

if (className[0] == 'V')
{
Expand Down Expand Up @@ -879,7 +884,9 @@ bool HWIntrinsicInfo::isFullyImplementedIsa(CORINFO_InstructionSet isa)
case InstructionSet_X86Serialize:
case InstructionSet_X86Serialize_X64:
case InstructionSet_AVX10v1:
case InstructionSet_AVX10v1_X64:
case InstructionSet_AVX10v1_V512:
case InstructionSet_AVX10v1_V512_X64:
case InstructionSet_EVEX:
{
return true;
Expand Down
27 changes: 17 additions & 10 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ var_types Compiler::impImportCall(OPCODE opcode,
const char* namespaceName;
const char* className;
const char* methodName =
info.compCompHnd->getMethodNameFromMetadata(methHnd, &className, &namespaceName, nullptr);
info.compCompHnd->getMethodNameFromMetadata(methHnd, &className, &namespaceName, nullptr, 0);
if ((namespaceName != nullptr) && (className != nullptr) && (methodName != nullptr) &&
(strcmp(namespaceName, "System.Runtime.CompilerServices") == 0) &&
(strcmp(className, "JitTestLabel") == 0) && (strcmp(methodName, "Mark") == 0))
Expand Down Expand Up @@ -9862,21 +9862,26 @@ GenTree* Compiler::impMinMaxIntrinsic(CORINFO_METHOD_HANDLE method,
//
NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
{
const char* className = nullptr;
const char* namespaceName = nullptr;
const char* enclosingClassName = nullptr;
const char* className = nullptr;
const char* namespaceName = nullptr;
const char* enclosingClassNames[2] = {nullptr};
const char* methodName =
info.compCompHnd->getMethodNameFromMetadata(method, &className, &namespaceName, &enclosingClassName);
info.compCompHnd->getMethodNameFromMetadata(method, &className, &namespaceName, enclosingClassNames,
ArrLen(enclosingClassNames));

JITDUMP("Named Intrinsic ");

if (namespaceName != nullptr)
{
JITDUMP("%s.", namespaceName);
}
if (enclosingClassName != nullptr)
if (enclosingClassNames[1] != nullptr)
{
JITDUMP("%s.", enclosingClassName);
JITDUMP("%s.", enclosingClassNames[1]);
}
if (enclosingClassNames[0] != nullptr)
{
JITDUMP("%s.", enclosingClassNames[0]);
}
if (className != nullptr)
{
Expand Down Expand Up @@ -10328,7 +10333,8 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
CORINFO_SIG_INFO sig;
info.compCompHnd->getMethodSig(method, &sig);

result = SimdAsHWIntrinsicInfo::lookupId(this, &sig, className, methodName, enclosingClassName);
result =
SimdAsHWIntrinsicInfo::lookupId(this, &sig, className, methodName, enclosingClassNames[0]);
#endif // FEATURE_HW_INTRINSICS

if (result == NI_Illegal)
Expand Down Expand Up @@ -10567,7 +10573,8 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
CORINFO_SIG_INFO sig;
info.compCompHnd->getMethodSig(method, &sig);

result = HWIntrinsicInfo::lookupId(this, &sig, className, methodName, enclosingClassName);
result = HWIntrinsicInfo::lookupId(this, &sig, className, methodName,
enclosingClassNames[0], enclosingClassNames[1]);
}
#endif // FEATURE_HW_INTRINSICS

Expand Down Expand Up @@ -10641,7 +10648,7 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
{
if (strcmp(methodName, "ReadUtf8") == 0)
{
assert(strcmp(enclosingClassName, "UTF8Encoding") == 0);
assert(strcmp(enclosingClassNames[0], "UTF8Encoding") == 0);
result = NI_System_Text_UTF8Encoding_UTF8EncodingSealed_ReadUtf8;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ public static bool IsHardwareIntrinsic(MethodDesc method)
if (owningType.IsIntrinsic && !owningType.HasInstantiation)
{
var owningMdType = (MetadataType)owningType;
string ns = owningMdType.ContainingType?.Namespace ?? owningMdType.Namespace;
DefType containingType = owningMdType.ContainingType;
string ns = containingType?.ContainingType?.Namespace ??
containingType?.Namespace ??
owningMdType.Namespace;
return method.Context.Target.Architecture switch
{
TargetArchitecture.ARM64 => ns == "System.Runtime.Intrinsics.Arm",
Expand Down
14 changes: 13 additions & 1 deletion src/coreclr/tools/Common/Compiler/InstructionSetSupport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,31 @@ public static string GetHardwareIntrinsicId(TargetArchitecture architecture, Typ
if (!potentialTypeDesc.IsIntrinsic || !(potentialTypeDesc is MetadataType potentialType))
return "";

string suffix = "";
if (architecture == TargetArchitecture.X64)
{
if (potentialType.Name == "X64")
potentialType = (MetadataType)potentialType.ContainingType;
if (potentialType.Name == "VL")
potentialType = (MetadataType)potentialType.ContainingType;
if (potentialType.Name == "V512")
{
suffix = "_V512";
potentialType = (MetadataType)potentialType.ContainingType;
}

if (potentialType.Namespace != "System.Runtime.Intrinsics.X86")
return "";
}
else if (architecture == TargetArchitecture.X86)
{
if (potentialType.Name == "VL")
potentialType = (MetadataType)potentialType.ContainingType;
if (potentialType.Name == "V512")
{
suffix = "_V512";
potentialType = (MetadataType)potentialType.ContainingType;
}
if (potentialType.Namespace != "System.Runtime.Intrinsics.X86")
return "";
}
Expand Down Expand Up @@ -110,7 +122,7 @@ public static string GetHardwareIntrinsicId(TargetArchitecture architecture, Typ
throw new InternalCompilerErrorException($"Unknown architecture '{architecture}'");
}

return potentialType.Name;
return potentialType.Name + suffix;
}

public SimdVectorLength GetVectorTSimdVector()
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/tools/Common/InstructionSetHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ public static InstructionSetSupport ConfigureInstructionSetSupport(string instru

optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("avx512vbmi");
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("avx512vbmi_vl");
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("avx10v1");
optimisticInstructionSetSupportBuilder.AddSupportedInstructionSet("avx10v1_v512");
}
}
else if (targetArchitecture == TargetArchitecture.ARM64)
Expand Down
Loading

0 comments on commit 8fee24f

Please sign in to comment.