From 2f49fcff6df15a200ef01eea16b3ce7930f75c5c Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 18 May 2023 21:22:12 -0700 Subject: [PATCH] Remove getMaxIntrinsicSIMDVectorLength from the JIT/EE interface (#86479) * Fixing a couple small typos * Remove getMaxIntrinsicSIMDVectorLength from the JIT/EE interface * Update src/coreclr/vm/methodtablebuilder.cpp --------- Co-authored-by: Jan Kotas --- docs/design/coreclr/jit/ryujit-overview.md | 3 +- src/coreclr/inc/corjit.h | 7 +--- src/coreclr/inc/jiteeversionguid.h | 10 +++--- src/coreclr/jit/compiler.cpp | 2 +- src/coreclr/jit/ee_il_dll.cpp | 35 ------------------- src/coreclr/jit/ee_il_dll.hpp | 2 -- .../tools/Common/JitInterface/CorInfoImpl.cs | 3 -- .../tools/aot/jitinterface/jitwrapper.cpp | 7 ---- .../superpmi-shared/icorjitcompilerimpl.h | 5 --- .../icorjitcompiler.cpp | 5 --- .../superpmi-shim-counter/icorjitcompiler.cpp | 6 ---- .../superpmi-shim-simple/icorjitcompiler.cpp | 5 --- src/coreclr/vm/cgensys.h | 11 ------ src/coreclr/vm/codeman.cpp | 6 ++-- src/coreclr/vm/methodtablebuilder.cpp | 30 ++++++++-------- 15 files changed, 27 insertions(+), 110 deletions(-) diff --git a/docs/design/coreclr/jit/ryujit-overview.md b/docs/design/coreclr/jit/ryujit-overview.md index e38e487058c74..5635815aaaae5 100644 --- a/docs/design/coreclr/jit/ryujit-overview.md +++ b/docs/design/coreclr/jit/ryujit-overview.md @@ -34,8 +34,7 @@ The following are the key methods on this interface: It returns a pointer to the code, its size, and additional GC, EH and (optionally) debug info. * `getVersionIdentifier` is the mechanism by which the JIT/EE interface is versioned. There is a single GUID (manually generated) which the JIT and EE must agree on. - * `getMaxIntrinsicSIMDVectorLength` communicates to the EE the largest SIMD vector length that the JIT can support. -* `ICorJitInfo` – this is the interface that the EE implements. It has many methods defined on it that allow the JIT to + * `ICorJitInfo` – this is the interface that the EE implements. It has many methods defined on it that allow the JIT to look up metadata tokens, traverse type signatures, compute field and vtable offsets, find method entry points, construct string literals, etc. This bulk of this interface is inherited from `ICorDynamicInfo` which is defined in [src/inc/corinfo.h](https://github.com/dotnet/runtime/blob/main/src/coreclr/inc/corinfo.h). The implementation diff --git a/src/coreclr/inc/corjit.h b/src/coreclr/inc/corjit.h index 0f29b274f1b03..c48bf938da09d 100644 --- a/src/coreclr/inc/corjit.h +++ b/src/coreclr/inc/corjit.h @@ -209,11 +209,6 @@ class ICorJitCompiler GUID* versionIdentifier /* OUT */ ) = 0; - // When the EE loads the System.Numerics.Vectors assembly, it asks the JIT what length (in bytes) of - // SIMD vector it supports as an intrinsic type. Zero means that the JIT does not support SIMD - // intrinsics, so the EE should use the default size (i.e. the size of the IL implementation). - virtual unsigned getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags) { return 0; } - // Some JIT's may support multiple OSs. This api provides a means to specify to the JIT what OS it should // be trying to compile. This api does not produce any errors, any errors are to be generated by the // the compileMethod call, which will call back into the VM to ensure bits are correctly setup. @@ -340,7 +335,7 @@ class ICorJitInfo : public ICorDynamicInfo // // SAMPLE_INTERVAL must be >= SIZE. SAMPLE_INTERVAL / SIZE // gives the average number of calls between table updates. - // + // struct HandleHistogram32 { enum diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 53e3edd5b7264..eee46d6068395 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* c540b287-0d17-4fc0-bac8-abd055acccb8 */ - 0xc540b287, - 0x0d17, - 0x4fc0, - {0xba, 0xc8, 0xab, 0xd0, 0x55, 0xac, 0xcc, 0xb8} +constexpr GUID JITEEVersionIdentifier = { /* dfc41bc9-f134-4c50-897e-fc9304a82059 */ + 0xdfc41bc9, + 0xf134, + 0x4c50, + {0x89, 0x7e, 0xfc, 0x93, 0x04, 0xa8, 0x20, 0x59} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index dab3b58543146..1d7fbbffeab82 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -2304,7 +2304,7 @@ void Compiler::compSetProcessor() // Some architectures can experience frequency throttling when // executing 512-bit width instructions. To account for this we set the // default preferred vector width to 256-bits in some scenarios. Power - // users can override this with `DOTNET_PreferredVectorBitWith=512` to + // users can override this with `DOTNET_PreferredVectorBitWidth=512` to // allow using such instructions where hardware support is available. preferredVectorByteLength = 256; diff --git a/src/coreclr/jit/ee_il_dll.cpp b/src/coreclr/jit/ee_il_dll.cpp index cc5ff2999b4fc..5d7c5b9b9c4eb 100644 --- a/src/coreclr/jit/ee_il_dll.cpp +++ b/src/coreclr/jit/ee_il_dll.cpp @@ -314,41 +314,6 @@ void CILJit::setTargetOS(CORINFO_OS os) #endif } -/***************************************************************************** - * Determine the maximum length of SIMD vector supported by this JIT. - */ - -unsigned CILJit::getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags) -{ - JitFlags jitFlags; - jitFlags.SetFromFlags(cpuCompileFlags); - -#ifdef FEATURE_SIMD -#if defined(TARGET_XARCH) - if (!jitFlags.IsSet(JitFlags::JIT_FLAG_PREJIT) && - jitFlags.GetInstructionSetFlags().HasInstructionSet(InstructionSet_AVX2)) - { - if (GetJitTls() != nullptr && JitTls::GetCompiler() != nullptr) - { - JITDUMP("getMaxIntrinsicSIMDVectorLength: returning 32\n"); - } - return 32; - } -#endif // defined(TARGET_XARCH) - if (GetJitTls() != nullptr && JitTls::GetCompiler() != nullptr) - { - JITDUMP("getMaxIntrinsicSIMDVectorLength: returning 16\n"); - } - return 16; -#else // !FEATURE_SIMD - if (GetJitTls() != nullptr && JitTls::GetCompiler() != nullptr) - { - JITDUMP("getMaxIntrinsicSIMDVectorLength: returning 0\n"); - } - return 0; -#endif // !FEATURE_SIMD -} - //------------------------------------------------------------------------ // eeGetArgSize: Returns the number of bytes required for the given type argument // including padding after the actual value. diff --git a/src/coreclr/jit/ee_il_dll.hpp b/src/coreclr/jit/ee_il_dll.hpp index cfdc1f87c680e..0f8191078918f 100644 --- a/src/coreclr/jit/ee_il_dll.hpp +++ b/src/coreclr/jit/ee_il_dll.hpp @@ -17,8 +17,6 @@ class CILJit : public ICorJitCompiler void getVersionIdentifier(GUID* versionIdentifier /* OUT */ ); - unsigned getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags); - void setTargetOS(CORINFO_OS os); }; diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 2a16f1102fec0..30f0b145a1864 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -148,9 +148,6 @@ private static extern CorJitResult JitCompileMethod(out IntPtr exception, IntPtr jit, IntPtr thisHandle, IntPtr callbacks, ref CORINFO_METHOD_INFO info, uint flags, out IntPtr nativeEntry, out uint codeSize); - [DllImport(JitSupportLibrary)] - private static extern uint GetMaxIntrinsicSIMDVectorLength(IntPtr jit, CORJIT_FLAGS* flags); - [DllImport(JitSupportLibrary)] private static extern IntPtr AllocException([MarshalAs(UnmanagedType.LPWStr)]string message, int messageLength); diff --git a/src/coreclr/tools/aot/jitinterface/jitwrapper.cpp b/src/coreclr/tools/aot/jitinterface/jitwrapper.cpp index 4b763aec27fbf..1a091b2fab197 100644 --- a/src/coreclr/tools/aot/jitinterface/jitwrapper.cpp +++ b/src/coreclr/tools/aot/jitinterface/jitwrapper.cpp @@ -50,10 +50,3 @@ DLL_EXPORT void JitProcessShutdownWork(ICorJitCompiler * pJit) { return pJit->ProcessShutdownWork(nullptr); } - -DLL_EXPORT unsigned GetMaxIntrinsicSIMDVectorLength( - ICorJitCompiler * pJit, - CORJIT_FLAGS * flags) -{ - return pJit->getMaxIntrinsicSIMDVectorLength(*flags); -} diff --git a/src/coreclr/tools/superpmi/superpmi-shared/icorjitcompilerimpl.h b/src/coreclr/tools/superpmi/superpmi-shared/icorjitcompilerimpl.h index 49cfb3b7a1482..66f7e0992a300 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/icorjitcompilerimpl.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/icorjitcompilerimpl.h @@ -42,11 +42,6 @@ void ProcessShutdownWork(ICorStaticInfo* info); /* {}; */ void getVersionIdentifier(GUID* versionIdentifier /* OUT */ ); -// When the EE loads the System.Numerics.Vectors assembly, it asks the JIT what length (in bytes) of -// SIMD vector it supports as an intrinsic type. Zero means that the JIT does not support SIMD -// intrinsics, so the EE should use the default size (i.e. the size of the IL implementation). -unsigned getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags); /* { return 0; } */ - // Some JIT's may support multiple OSs. This api provides a means to specify to the JIT what OS it should // be trying to compile. This api does not produce any errors, any errors are to be generated by the // the compileMethod call, which will call back into the VM to ensure bits are correctly setup. diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp index 0ef956bd62787..331361a3e500b 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitcompiler.cpp @@ -158,8 +158,3 @@ void interceptor_ICJC::getVersionIdentifier(GUID* versionIdentifier /* OUT */) { original_ICorJitCompiler->getVersionIdentifier(versionIdentifier); } - -unsigned interceptor_ICJC::getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags) -{ - return original_ICorJitCompiler->getMaxIntrinsicSIMDVectorLength(cpuCompileFlags); -} diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitcompiler.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitcompiler.cpp index 1f1f1d210ef6e..f67c5cbf6863b 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitcompiler.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitcompiler.cpp @@ -40,9 +40,3 @@ void interceptor_ICJC::getVersionIdentifier(GUID* versionIdentifier /* OUT */) mcs->AddCall("getVersionIdentifier"); original_ICorJitCompiler->getVersionIdentifier(versionIdentifier); } - -unsigned interceptor_ICJC::getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags) -{ - mcs->AddCall("getMaxIntrinsicSIMDVectorLength"); - return original_ICorJitCompiler->getMaxIntrinsicSIMDVectorLength(cpuCompileFlags); -} diff --git a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitcompiler.cpp b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitcompiler.cpp index 53442ced042e1..5f266c0fc96e9 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitcompiler.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitcompiler.cpp @@ -35,8 +35,3 @@ void interceptor_ICJC::getVersionIdentifier(GUID* versionIdentifier /* OUT */) { original_ICorJitCompiler->getVersionIdentifier(versionIdentifier); } - -unsigned interceptor_ICJC::getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags) -{ - return original_ICorJitCompiler->getMaxIntrinsicSIMDVectorLength(cpuCompileFlags); -} diff --git a/src/coreclr/vm/cgensys.h b/src/coreclr/vm/cgensys.h index 12f3a31778065..40f3f2bd79fed 100644 --- a/src/coreclr/vm/cgensys.h +++ b/src/coreclr/vm/cgensys.h @@ -80,17 +80,6 @@ extern "C" DWORD xmmYmmStateSupport(); extern "C" DWORD avx512StateSupport(); #endif -inline bool TargetHasAVXSupport() -{ -#if (defined(TARGET_X86) || defined(TARGET_AMD64)) - int cpuInfo[4]; - __cpuid(cpuInfo, 0x00000001); // All x86/AMD64 targets support cpuid. - const int CPUID_ECX = 2; - return ((cpuInfo[CPUID_ECX] & (1 << 28)) != 0); // The AVX feature is ECX bit 28. -#endif // (defined(TARGET_X86) || defined(TARGET_AMD64)) - return false; -} - #ifdef DACCESS_COMPILE // Used by dac/strike to make sense of non-jit/non-jit-helper call targets diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index d2c7c4c3478cc..eba0c2cd1e88f 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -1256,6 +1256,8 @@ EEJitManager::EEJitManager() m_storeRichDebugInfo = false; m_cleanupList = NULL; + + SetCpuInfo(); } #if defined(TARGET_X86) || defined(TARGET_AMD64) @@ -1940,7 +1942,7 @@ void EEJitManager::SetCpuInfo() // Some architectures can experience frequency throttling when executing // executing 512-bit width instructions. To account for this we set the // default preferred vector width to 256-bits in some scenarios. Power - // users can override this with `DOTNET_PreferredVectorBitWith=512` to + // users can override this with `DOTNET_PreferredVectorBitWidth=512` to // allow using such instructions where hardware support is available. if (xarchCpuInfo.FamilyId == 0x06) @@ -2236,8 +2238,6 @@ BOOL EEJitManager::LoadJIT() if (IsJitLoaded()) return TRUE; - SetCpuInfo(); - m_storeRichDebugInfo = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_RichDebugInfo) != 0; ICorJitCompiler* newJitCompiler = NULL; diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp index 147b2221fad47..0349ca2684e99 100644 --- a/src/coreclr/vm/methodtablebuilder.cpp +++ b/src/coreclr/vm/methodtablebuilder.cpp @@ -1141,25 +1141,27 @@ BOOL MethodTableBuilder::CheckIfSIMDAndUpdateSize() if (strcmp(className, "Vector`1") != 0 || strcmp(nameSpace, "System.Numerics") != 0) return false; - if (!TargetHasAVXSupport()) - return false; + CORJIT_FLAGS CPUCompileFlags = ExecutionManager::GetEEJitManager()->GetCPUCompileFlags(); + uint32_t numInstanceFieldBytes = 16; + + if (CPUCompileFlags.IsSet(InstructionSet_AVX2)) + { + numInstanceFieldBytes = 32; + } - EEJitManager *jitMgr = ExecutionManager::GetEEJitManager(); - if (jitMgr->LoadJIT()) + if (numInstanceFieldBytes != 16) { - CORJIT_FLAGS cpuCompileFlags = jitMgr->GetCPUCompileFlags(); - unsigned intrinsicSIMDVectorLength = jitMgr->m_jit->getMaxIntrinsicSIMDVectorLength(cpuCompileFlags); - if (intrinsicSIMDVectorLength != 0) + bmtFP->NumInstanceFieldBytes = numInstanceFieldBytes; + + if (HasLayout()) { - bmtFP->NumInstanceFieldBytes = intrinsicSIMDVectorLength; - if (HasLayout()) - { - GetLayoutInfo()->m_cbManagedSize = intrinsicSIMDVectorLength; - } - return true; + GetLayoutInfo()->m_cbManagedSize = numInstanceFieldBytes; } + + return true; } -#endif // defined(TARGET_X86) || defined(TARGET_AMD64) +#endif // TARGET_X86 || TARGET_AMD64 + return false; }