From 8d34db07ce56536882d4dca97a7e49b45fb5f362 Mon Sep 17 00:00:00 2001 From: Carol Eidt Date: Sat, 3 Nov 2018 09:17:48 -0700 Subject: [PATCH] Add Windows support for Arm64 HW Intrinsics Also, fix Crypto test and ensure that featureSIMD is taken into account Fix #18175 Fix #20088 --- src/jit/compiler.h | 3 +- src/jit/hwintrinsicArm64.cpp | 89 ++++++++++++++++++++++-- src/jit/hwintrinsicArm64.h | 10 +++ src/vm/codeman.cpp | 25 +++++-- tests/src/JIT/HardwareIntrinsics/Arm64/Crypto.cs | 5 +- 5 files changed, 118 insertions(+), 14 deletions(-) diff --git a/src/jit/compiler.h b/src/jit/compiler.h index f686426..e1a76ab 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -3400,6 +3400,8 @@ protected: bool mustExpand); protected: + bool compSupportsHWIntrinsic(InstructionSet isa); + #ifdef _TARGET_XARCH_ GenTree* impSSEIntrinsic(NamedIntrinsic intrinsic, CORINFO_METHOD_HANDLE method, @@ -3445,7 +3447,6 @@ protected: CORINFO_METHOD_HANDLE method, CORINFO_SIG_INFO* sig, bool mustExpand); - bool compSupportsHWIntrinsic(InstructionSet isa); protected: GenTree* getArgForHWIntrinsic(var_types argType, CORINFO_CLASS_HANDLE argClass); diff --git a/src/jit/hwintrinsicArm64.cpp b/src/jit/hwintrinsicArm64.cpp index 35c4154..d82eef5 100644 --- a/src/jit/hwintrinsicArm64.cpp +++ b/src/jit/hwintrinsicArm64.cpp @@ -108,7 +108,7 @@ NamedIntrinsic Compiler::lookupHWIntrinsic(const char* className, const char* me { if ((isaFlag & hwIntrinsicInfoArray[i].isaflags) && strcmp(methodName, hwIntrinsicInfoArray[i].name) == 0) { - if (compSupports(isa)) + if (compSupportsHWIntrinsic(isa)) { // Intrinsic is supported on platform result = hwIntrinsicInfoArray[i].id; @@ -137,6 +137,84 @@ bool Compiler::impCheckImmediate(GenTree* immediateOp, unsigned int max) } //------------------------------------------------------------------------ +// isFullyImplementedIsa: Gets a value that indicates whether the InstructionSet is fully implemented +// +// Arguments: +// isa - The InstructionSet to check +// +// Return Value: +// true if isa is supported; otherwise, false +// +// Notes: +// This currently returns true for all partially-implemented ISAs. +// TODO-Bug: Set this to return the correct values as GH 20427 is resolved. +// +bool HWIntrinsicInfo::isFullyImplementedIsa(InstructionSet isa) +{ + switch (isa) + { + case InstructionSet_Base: + case InstructionSet_Crc32: + case InstructionSet_Aes: + case InstructionSet_Simd: + case InstructionSet_Sha1: + case InstructionSet_Sha256: + return true; + + default: + assert(!"Unexpected Arm64 HW intrinsics ISA"); + return false; + } +} + +//------------------------------------------------------------------------ +// isScalarIsa: Gets a value that indicates whether the InstructionSet is scalar +// +// Arguments: +// isa - The InstructionSet to check +// +// Return Value: +// true if isa is scalar; otherwise, false +bool HWIntrinsicInfo::isScalarIsa(InstructionSet isa) +{ + switch (isa) + { + case InstructionSet_Base: + case InstructionSet_Crc32: + return true; + + case InstructionSet_Aes: + case InstructionSet_Simd: + case InstructionSet_Sha1: + case InstructionSet_Sha256: + return false; + + default: + assert(!"Unexpected Arm64 HW intrinsics ISA"); + return true; + } +} + +//------------------------------------------------------------------------ +// compSupportsHWIntrinsic: compiler support of hardware intrinsics +// +// Arguments: +// isa - Instruction set +// Return Value: +// true if +// - isa is a scalar ISA +// - isa is a SIMD ISA and featureSIMD=true +// - isa is fully implemented or EnableIncompleteISAClass=true +bool Compiler::compSupportsHWIntrinsic(InstructionSet isa) +{ + return (featureSIMD || HWIntrinsicInfo::isScalarIsa(isa)) && ( +#ifdef DEBUG + JitConfig.EnableIncompleteISAClass() || +#endif + HWIntrinsicInfo::isFullyImplementedIsa(isa)); +} + +//------------------------------------------------------------------------ // lookupNumArgs: gets the number of arguments for the hardware intrinsic. // This attempts to do a table based lookup but will fallback to the number // of operands in 'node' if the table entry is -1. @@ -235,13 +313,16 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic, // Simd instantiation type check if (simdClass != nullptr) { - compFloatingPointUsed = true; + if (featureSIMD) + { + compFloatingPointUsed = true; - simdBaseType = getBaseTypeAndSizeOfSIMDType(simdClass, &simdSizeBytes); + simdBaseType = getBaseTypeAndSizeOfSIMDType(simdClass, &simdSizeBytes); + } if (simdBaseType == TYP_UNKNOWN) { - return impUnsupportedHWIntrinsic(CORINFO_HELP_THROW_TYPE_NOT_SUPPORTED, method, sig, mustExpand); + return impUnsupportedHWIntrinsic(CORINFO_HELP_THROW_PLATFORM_NOT_SUPPORTED, method, sig, mustExpand); } simdType = getSIMDTypeForSize(simdSizeBytes); } diff --git a/src/jit/hwintrinsicArm64.h b/src/jit/hwintrinsicArm64.h index e60288a..0cd5955 100644 --- a/src/jit/hwintrinsicArm64.h +++ b/src/jit/hwintrinsicArm64.h @@ -54,6 +54,16 @@ struct HWIntrinsicInfo static const HWIntrinsicInfo& lookup(NamedIntrinsic id); static int lookupNumArgs(const GenTreeHWIntrinsic* node); + static bool isFullyImplementedIsa(InstructionSet isa); + static bool isScalarIsa(InstructionSet isa); + + // Member lookup + + static NamedIntrinsic lookupId(NamedIntrinsic id) + { + return lookup(id).id; + } + static const char* lookupName(NamedIntrinsic id) { return lookup(id).name; diff --git a/src/vm/codeman.cpp b/src/vm/codeman.cpp index 1cb54f2..1ea4303 100644 --- a/src/vm/codeman.cpp +++ b/src/vm/codeman.cpp @@ -1475,17 +1475,32 @@ void EEJitManager::SetCpuInfo() } #endif // defined(_TARGET_X86_) || defined(_TARGET_AMD64_) -#if defined(_TARGET_ARM64_) && defined(FEATURE_PAL) - PAL_GetJitCpuCapabilityFlags(&CPUCompileFlags); -#endif - #if defined(_TARGET_ARM64_) static ConfigDWORD fFeatureSIMD; if (fFeatureSIMD.val(CLRConfig::EXTERNAL_FeatureSIMD) != 0) { CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_FEATURE_SIMD); } -#endif +#if defined(FEATURE_PAL) + PAL_GetJitCpuCapabilityFlags(&CPUCompileFlags); +#elif defined(_WIN64) + // FP and SIMD support are enabled by default + CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_HAS_ARM64_SIMD); + CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_HAS_ARM64_FP); + // PF_ARM_V8_CRYPTO_INSTRUCTIONS_AVAILABLE (30) + if (IsProcessorFeaturePresent(PF_ARM_V8_CRYPTO_INSTRUCTIONS_AVAILABLE)) + { + CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_HAS_ARM64_AES); + CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_HAS_ARM64_SHA1); + CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_HAS_ARM64_SHA256); + } + // PF_ARM_V8_CRC32_INSTRUCTIONS_AVAILABLE (31) + if (IsProcessorFeaturePresent(PF_ARM_V8_CRC32_INSTRUCTIONS_AVAILABLE)) + { + CPUCompileFlags.Set(CORJIT_FLAGS::CORJIT_FLAG_HAS_ARM64_CRC32); + } +#endif // _WIN64 +#endif // _TARGET_ARM64_ m_CPUCompileFlags = CPUCompileFlags; } diff --git a/tests/src/JIT/HardwareIntrinsics/Arm64/Crypto.cs b/tests/src/JIT/HardwareIntrinsics/Arm64/Crypto.cs index f68f749..fded506 100644 --- a/tests/src/JIT/HardwareIntrinsics/Arm64/Crypto.cs +++ b/tests/src/JIT/HardwareIntrinsics/Arm64/Crypto.cs @@ -195,12 +195,11 @@ namespace Arm64intrisicsTest Func cryptoOp) where TVectorType : new() { - TVectorType v = new TVectorType(); - bool notSupported = false; try { + TVectorType v = new TVectorType(); cryptoOp(v,v,v); } catch (PlatformNotSupportedException) // TODO-Fixme check for Type not supported exception @@ -266,8 +265,6 @@ namespace Arm64intrisicsTest testCryptoOp, uint, Vector128 >(name, (x, y, z) => Sha1.SchedulePart2(x, y), sha1su2Res); if(Sha1.FixedRotate(100) != 25) throw new Exception("Sha1 FixedRotate failed.\n"); - - } else { -- 2.7.4