From 6ed9f1b4f5c660ee15162c01c69a40e434c71274 Mon Sep 17 00:00:00 2001 From: Clinton Ingram Date: Thu, 30 Apr 2020 10:24:54 -0700 Subject: [PATCH] disable LZCNT in crossgen (#35598) Now that BitOperations.TrailingZeroCount has a fallback to BSR, the cost of querying Lzcnt.IsSupported before using LZCNT is higher than simply using the fallback. This is a situation very specific to LZCNT because it's most often a one-and-done instruction and the fallback is only very marginally slower. Note that this change applies only to crossgen, as the hardware intrinsic model differs for crossgen2 and it was decided that this change isn't appropriate for that different model. --- src/coreclr/src/zap/zapinfo.cpp | 8 +++++--- src/coreclr/src/zap/zapper.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/coreclr/src/zap/zapinfo.cpp b/src/coreclr/src/zap/zapinfo.cpp index 0892717..1f4f83c 100644 --- a/src/coreclr/src/zap/zapinfo.cpp +++ b/src/coreclr/src/zap/zapinfo.cpp @@ -2177,13 +2177,15 @@ DWORD FilterNamedIntrinsicMethodAttribs(ZapInfo* pZapInfo, DWORD attribs, CORINF } } #if defined(TARGET_X86) || defined(TARGET_AMD64) - else if ((strcmp(isaName, "Avx") == 0) || (strcmp(isaName, "Fma") == 0) || (strcmp(isaName, "Avx2") == 0) || (strcmp(isaName, "Bmi1") == 0) || (strcmp(isaName, "Bmi2") == 0)) + else if ((strcmp(isaName, "Avx") == 0) || (strcmp(isaName, "Fma") == 0) || (strcmp(isaName, "Avx2") == 0) + || (strcmp(isaName, "Bmi1") == 0) || (strcmp(isaName, "Bmi2") == 0) || (strcmp(isaName, "Lzcnt") == 0)) { if ((enclosingClassName == nullptr) || fIsPlatformSubArchitecture) { - // If it is the get_IsSupported method for an ISA which requires the VEX - // encoding we want to expand unconditionally. This will force those code + // If it is the get_IsSupported method for an ISA which is intentionally not enabled + // for crossgen, we want to expand unconditionally. This will force those code // paths to be treated as dead code and dropped from the compilation. + // See Zapper::InitializeCompilerFlags // // For all of the other intrinsics in an ISA which requires the VEX encoding // we need to treat them as regular method calls. This is done because RyuJIT diff --git a/src/coreclr/src/zap/zapper.cpp b/src/coreclr/src/zap/zapper.cpp index e7e22b4..eb79402 100644 --- a/src/coreclr/src/zap/zapper.cpp +++ b/src/coreclr/src/zap/zapper.cpp @@ -1213,11 +1213,11 @@ void Zapper::InitializeCompilerFlags(CORCOMPILE_VERSION_INFO * pVersionInfo) m_pOpt->m_compilerFlags.Set(InstructionSet_SSE41); m_pOpt->m_compilerFlags.Set(InstructionSet_SSE42); m_pOpt->m_compilerFlags.Set(InstructionSet_POPCNT); - // Leaving out CORJIT_FLAGS::CORJIT_FLAG_USE_AVX, CORJIT_FLAGS::CORJIT_FLAG_USE_FMA - // CORJIT_FLAGS::CORJIT_FLAG_USE_AVX2, CORJIT_FLAGS::CORJIT_FLAG_USE_BMI1, - // CORJIT_FLAGS::CORJIT_FLAG_USE_BMI2 on purpose - these require VEX encodings + // Leaving out InstructionSet_AVX, InstructionSet_FMA, InstructionSet_AVX2, + // InstructionSet_BMI1, InstructionSet_BMI2 on purpose - these require VEX encodings // and the JIT doesn't support generating code for methods with mixed encodings. - m_pOpt->m_compilerFlags.Set(InstructionSet_LZCNT); + // Also leaving out InstructionSet_LZCNT because BSR from InstructionSet_X86Base + // is used as a fallback in CoreLib and doesn't require an IsSupported check. #endif // defined(TARGET_X86) || defined(TARGET_AMD64) } #endif // defined(TARGET_X86) || defined(TARGET_AMD64) || defined(TARGET_ARM64) -- 2.7.4