disable LZCNT in crossgen (#35598)
authorClinton Ingram <clinton.ingram@outlook.com>
Thu, 30 Apr 2020 17:24:54 +0000 (10:24 -0700)
committerGitHub <noreply@github.com>
Thu, 30 Apr 2020 17:24:54 +0000 (10:24 -0700)
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
src/coreclr/src/zap/zapper.cpp

index 0892717..1f4f83c 100644 (file)
@@ -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
index e7e22b4..eb79402 100644 (file)
@@ -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)