From 6829bad6b671187a4025e9c4eb22aad9da59a0d8 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Thu, 13 Dec 2018 07:48:10 -0800 Subject: [PATCH] Fixing up configEnableISA and Compiler::compSetProcessor to be consistent with EEJitManager::SetCpuInfo in how ISA support checks are done (dotnet/coreclr#21499) Commit migrated from https://github.com/dotnet/coreclr/commit/7455270acfbace8aa3f77c9cbec641952d51ab58 --- src/coreclr/src/jit/compiler.cpp | 298 ++++++++++++-------------------------- src/coreclr/src/jit/compiler.h | 5 +- src/coreclr/src/jit/ee_il_dll.cpp | 7 +- src/coreclr/src/vm/codeman.cpp | 1 + 4 files changed, 104 insertions(+), 207 deletions(-) diff --git a/src/coreclr/src/jit/compiler.cpp b/src/coreclr/src/jit/compiler.cpp index 9a06d47..2e7f8ae 100644 --- a/src/coreclr/src/jit/compiler.cpp +++ b/src/coreclr/src/jit/compiler.cpp @@ -2318,86 +2318,12 @@ const char* Compiler::compLocalVarName(unsigned varNum, unsigned offs) #endif // DEBUG /*****************************************************************************/ -#ifdef _TARGET_XARCH_ -static bool configEnableISA(InstructionSet isa) -{ - switch (isa) - { - case InstructionSet_AVX2: - if (JitConfig.EnableAVX2() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_AVX: - if (JitConfig.EnableAVX() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_SSE42: - if (JitConfig.EnableSSE42() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_SSE41: - if (JitConfig.EnableSSE41() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_SSSE3: - if (JitConfig.EnableSSSE3() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_SSE3: - if (JitConfig.EnableSSE3() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_SSE2: - if (JitConfig.EnableSSE2() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_SSE: - if (JitConfig.EnableSSE() == 0) - { - return false; - } - __fallthrough; - case InstructionSet_Base: - return (JitConfig.EnableHWIntrinsic() != 0); - - // TODO: BMI1/BMI2 actually don't depend on AVX, they depend on the VEX encoding; which is currently controlled - // by InstructionSet_AVX - case InstructionSet_BMI1: - return JitConfig.EnableBMI1() != 0 && configEnableISA(InstructionSet_AVX); - case InstructionSet_BMI2: - return JitConfig.EnableBMI2() != 0 && configEnableISA(InstructionSet_AVX); - case InstructionSet_FMA: - return JitConfig.EnableFMA() != 0 && configEnableISA(InstructionSet_AVX); - case InstructionSet_AES: - return JitConfig.EnableAES() != 0 && configEnableISA(InstructionSet_SSE2); - case InstructionSet_LZCNT: - return JitConfig.EnableLZCNT() != 0; - case InstructionSet_PCLMULQDQ: - return JitConfig.EnablePCLMULQDQ() != 0 && configEnableISA(InstructionSet_SSE2); - case InstructionSet_POPCNT: - return JitConfig.EnablePOPCNT() != 0 && configEnableISA(InstructionSet_SSE42); - default: - return false; - } -} -#endif // _TARGET_XARCH_ - void Compiler::compSetProcessor() { + // + // NOTE: This function needs to be kept in sync with EEJitManager::SetCpuInfo() in vm\codemap.cpp + // + const JitFlags& jitFlags = *opts.jitFlags; #if defined(_TARGET_ARM_) @@ -2440,147 +2366,116 @@ void Compiler::compSetProcessor() if (!jitFlags.IsSet(JitFlags::JIT_FLAG_PREJIT)) { - if (configEnableISA(InstructionSet_Base)) + if (JitConfig.EnableHWIntrinsic()) { opts.setSupportedISA(InstructionSet_Base); - } - if (configEnableISA(InstructionSet_SSE)) - { - opts.setSupportedISA(InstructionSet_SSE); + + if (JitConfig.EnableSSE()) + { + opts.setSupportedISA(InstructionSet_SSE); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_SSE_X64); -#endif - } - if (configEnableISA(InstructionSet_SSE2)) - { - opts.setSupportedISA(InstructionSet_SSE2); + opts.setSupportedISA(InstructionSet_SSE_X64); +#endif // _TARGET_AMD64_ + + if (JitConfig.EnableSSE2()) + { + opts.setSupportedISA(InstructionSet_SSE2); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_SSE2_X64); -#endif - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_LZCNT)) - { - if (configEnableISA(InstructionSet_LZCNT)) - { - opts.setSupportedISA(InstructionSet_LZCNT); + opts.setSupportedISA(InstructionSet_SSE2_X64); +#endif // _TARGET_AMD64_ + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AES) && JitConfig.EnableAES()) + { + opts.setSupportedISA(InstructionSet_AES); + } + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_PCLMULQDQ) && JitConfig.EnablePCLMULQDQ()) + { + opts.setSupportedISA(InstructionSet_PCLMULQDQ); + } + + // We need to additionaly check that COMPlus_EnableSSE3_4 is set, as that + // is a prexisting config flag that controls the SSE3+ ISAs + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSE3) && JitConfig.EnableSSE3() && + JitConfig.EnableSSE3_4()) + { + opts.setSupportedISA(InstructionSet_SSE3); + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSSE3) && JitConfig.EnableSSSE3()) + { + opts.setSupportedISA(InstructionSet_SSSE3); + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSE41) && JitConfig.EnableSSE41()) + { + opts.setSupportedISA(InstructionSet_SSE41); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_LZCNT_X64); -#endif - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_POPCNT)) - { - if (configEnableISA(InstructionSet_POPCNT)) - { - opts.setSupportedISA(InstructionSet_POPCNT); + opts.setSupportedISA(InstructionSet_SSE41_X64); +#endif // _TARGET_AMD64_ + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSE42) && JitConfig.EnableSSE42()) + { + opts.setSupportedISA(InstructionSet_SSE42); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_POPCNT_X64); -#endif - } - } + opts.setSupportedISA(InstructionSet_SSE42_X64); +#endif // _TARGET_AMD64_ - // There are currently two sets of flags that control SSE3 through SSE4.2 support: - // These are the general EnableSSE3_4 flag and the individual ISA flags. We need to - // check both for any given ISA. - if (JitConfig.EnableSSE3_4()) - { - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSE3)) - { - if (configEnableISA(InstructionSet_SSE3)) - { - opts.setSupportedISA(InstructionSet_SSE3); - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSE41)) - { - if (configEnableISA(InstructionSet_SSE41)) - { - opts.setSupportedISA(InstructionSet_SSE41); + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_POPCNT) && JitConfig.EnablePOPCNT()) + { + opts.setSupportedISA(InstructionSet_POPCNT); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_SSE41_X64); -#endif + opts.setSupportedISA(InstructionSet_POPCNT_X64); +#endif // _TARGET_AMD64_ + } + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AVX) && JitConfig.EnableAVX()) + { + opts.setSupportedISA(InstructionSet_AVX); + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_FMA) && JitConfig.EnableFMA()) + { + opts.setSupportedISA(InstructionSet_FMA); + } + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AVX2) && JitConfig.EnableAVX2()) + { + opts.setSupportedISA(InstructionSet_AVX2); + } + } + } + } + } + } } } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSE42)) + + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_LZCNT) && JitConfig.EnableLZCNT()) { - if (configEnableISA(InstructionSet_SSE42)) - { - opts.setSupportedISA(InstructionSet_SSE42); + opts.setSupportedISA(InstructionSet_LZCNT); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_SSE42_X64); -#endif - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_SSSE3)) - { - if (configEnableISA(InstructionSet_SSSE3)) - { - opts.setSupportedISA(InstructionSet_SSSE3); - } - } - // AES and PCLMULQDQ requires 0x660F38/A encoding that is - // only used by SSSE3 and above ISAs - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AES)) - { - if (configEnableISA(InstructionSet_AES)) - { - opts.setSupportedISA(InstructionSet_AES); - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_PCLMULQDQ)) - { - if (configEnableISA(InstructionSet_PCLMULQDQ)) - { - opts.setSupportedISA(InstructionSet_PCLMULQDQ); - } + opts.setSupportedISA(InstructionSet_LZCNT_X64); +#endif // _TARGET_AMD64_ } - } - // There are currently two sets of flags that control instruction sets that require the VEX encoding: - // These are the general EnableAVX flag and the individual ISA flags. We need to - // check both for any given isa. - if (JitConfig.EnableAVX()) - { - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AVX)) - { - if (configEnableISA(InstructionSet_AVX)) - { - opts.setSupportedISA(InstructionSet_AVX); - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_FMA)) - { - if (configEnableISA(InstructionSet_FMA)) - { - opts.setSupportedISA(InstructionSet_FMA); - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AVX2)) - { - if (configEnableISA(InstructionSet_AVX2)) - { - opts.setSupportedISA(InstructionSet_AVX2); - } - } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_BMI1)) + // We currently need to also check that AVX is supported as that controls the support for the VEX encoding + // in the emitter. + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_BMI1) && JitConfig.EnableBMI1() && + compSupports(InstructionSet_AVX)) { - if (configEnableISA(InstructionSet_BMI1)) - { - opts.setSupportedISA(InstructionSet_BMI1); + opts.setSupportedISA(InstructionSet_BMI1); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_BMI1_X64); -#endif - } + opts.setSupportedISA(InstructionSet_BMI1_X64); +#endif // _TARGET_AMD64_ } - if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_BMI2)) + + // We currently need to also check that AVX is supported as that controls the support for the VEX encoding + // in the emitter. + if (jitFlags.IsSet(JitFlags::JIT_FLAG_USE_BMI2) && JitConfig.EnableBMI2() && + compSupports(InstructionSet_AVX)) { - if (configEnableISA(InstructionSet_BMI2)) - { - opts.setSupportedISA(InstructionSet_BMI2); + opts.setSupportedISA(InstructionSet_BMI2); #ifdef _TARGET_AMD64_ - opts.setSupportedISA(InstructionSet_BMI2_X64); -#endif - } + opts.setSupportedISA(InstructionSet_BMI2_X64); +#endif // _TARGET_AMD64_ } } } @@ -2594,8 +2489,7 @@ void Compiler::compSetProcessor() codeGen->getEmitter()->SetContainsAVX(false); codeGen->getEmitter()->SetContains256bitAVX(false); } - else if (compSupports(InstructionSet_SSSE3) || compSupports(InstructionSet_SSE41) || - compSupports(InstructionSet_SSE42) || compSupports(InstructionSet_AES) || + else if (compSupports(InstructionSet_SSSE3) || compSupports(InstructionSet_AES) || compSupports(InstructionSet_PCLMULQDQ)) { // Emitter::UseSSE4 controls whether we support the 4-byte encoding for certain diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index 703fc71..7e80070 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -7518,10 +7518,7 @@ private: return SIMD_AVX2_Supported; } - // SIMD_SSE4_Supported actually requires all of SSE3, SSSE3, SSE4.1, and SSE4.2 - // to be supported. We can only enable it if all four are enabled in the compiler - if (compSupports(InstructionSet_SSE42) && compSupports(InstructionSet_SSE41) && - compSupports(InstructionSet_SSSE3) && compSupports(InstructionSet_SSE3)) + if (compSupports(InstructionSet_SSE42)) { return SIMD_SSE4_Supported; } diff --git a/src/coreclr/src/jit/ee_il_dll.cpp b/src/coreclr/src/jit/ee_il_dll.cpp index 8fd494e..9038453 100644 --- a/src/coreclr/src/jit/ee_il_dll.cpp +++ b/src/coreclr/src/jit/ee_il_dll.cpp @@ -402,7 +402,12 @@ unsigned CILJit::getMaxIntrinsicSIMDVectorLength(CORJIT_FLAGS cpuCompileFlags) if (!jitFlags.IsSet(JitFlags::JIT_FLAG_PREJIT) && jitFlags.IsSet(JitFlags::JIT_FLAG_FEATURE_SIMD) && jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AVX2)) { - if (JitConfig.EnableAVX() != 0 && JitConfig.EnableAVX2() != 0) + // Since the ISAs can be disabled individually and since they are hierarchical in nature (that is + // disabling SSE also disables SSE2 through AVX2), we need to check each ISA in the hierarchy to + // ensure that AVX2 is actually supported. Otherwise, we will end up getting asserts downstream. + if ((JitConfig.EnableAVX2() != 0) && (JitConfig.EnableAVX() != 0) && (JitConfig.EnableSSE42() != 0) && + (JitConfig.EnableSSE41() != 0) && (JitConfig.EnableSSSE3() != 0) && (JitConfig.EnableSSE3() != 0) && + (JitConfig.EnableSSE2() != 0) && (JitConfig.EnableSSE() != 0) && (JitConfig.EnableHWIntrinsic() != 0)) { if (GetJitTls() != nullptr && JitTls::GetCompiler() != nullptr) { diff --git a/src/coreclr/src/vm/codeman.cpp b/src/coreclr/src/vm/codeman.cpp index 57d58d8..c02d4e8 100644 --- a/src/coreclr/src/vm/codeman.cpp +++ b/src/coreclr/src/vm/codeman.cpp @@ -1263,6 +1263,7 @@ void EEJitManager::SetCpuInfo() // // NOTE: This function needs to be kept in sync with Zapper::CompileAssembly() + // NOTE: This function needs to be kept in sync with compSetProcesor() in jit\compiler.cpp // CORJIT_FLAGS CPUCompileFlags; -- 2.7.4