diff options
author | Fei Peng <fei.peng@intel.com> | 2019-01-09 15:56:24 -0800 |
---|---|---|
committer | Fei Peng <fei.peng@intel.com> | 2019-01-09 15:56:24 -0800 |
commit | 80566c08ddc93fa6af2e673f0a72eb070346ed4f (patch) | |
tree | 92a33cb18eddcf650655d8c6f7544951eecaf41b | |
parent | 0a2ceb3fd146d9171b8c362aa8679d996d3e5bde (diff) | |
download | coreclr-80566c08ddc93fa6af2e673f0a72eb070346ed4f.tar.gz coreclr-80566c08ddc93fa6af2e673f0a72eb070346ed4f.tar.bz2 coreclr-80566c08ddc93fa6af2e673f0a72eb070346ed4f.zip |
Fix CRC32 instruction encoding on containment form
-rw-r--r-- | src/jit/emitxarch.cpp | 39 | ||||
-rw-r--r-- | src/jit/emitxarch.h | 16 | ||||
-rw-r--r-- | src/jit/lowerxarch.cpp | 10 | ||||
-rw-r--r-- | tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666.cs | 271 | ||||
-rw-r--r-- | tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_r.csproj | 34 | ||||
-rw-r--r-- | tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_ro.csproj | 34 |
6 files changed, 381 insertions, 23 deletions
diff --git a/src/jit/emitxarch.cpp b/src/jit/emitxarch.cpp index c7e0c9ffed..a7e40407b9 100644 --- a/src/jit/emitxarch.cpp +++ b/src/jit/emitxarch.cpp @@ -2059,6 +2059,8 @@ UNATIVE_OFFSET emitter::emitInsSizeAM(instrDesc* id, code_t code) size += emitGetRexPrefixSize(ins); } + size += emitAdjustSizeCrc32(ins, attrSize); + if (rgx == REG_NA) { /* The address is of the form "[reg+disp]" */ @@ -2225,19 +2227,21 @@ inline UNATIVE_OFFSET emitter::emitInsSizeAM(instrDesc* id, code_t code, int val inline UNATIVE_OFFSET emitter::emitInsSizeCV(instrDesc* id, code_t code) { - instruction ins = id->idIns(); + instruction ins = id->idIns(); + emitAttr attrSize = id->idOpSize(); // fgMorph changes any statics that won't fit into 32-bit addresses // into constants with an indir, rather than GT_CLS_VAR // so we should only hit this path for statics that are RIP-relative UNATIVE_OFFSET size = sizeof(INT32); - size += emitGetVexPrefixAdjustedSize(ins, id->idOpSize(), code); + size += emitGetVexPrefixAdjustedSize(ins, attrSize, code); + size += emitAdjustSizeCrc32(ins, attrSize); // Most 16-bit operand instructions will need a prefix. // This refers to 66h size prefix override. - if (id->idOpSize() == EA_2BYTE && ins != INS_movzx && ins != INS_movsx) + if (attrSize == EA_2BYTE && ins != INS_movzx && ins != INS_movsx) { size++; } @@ -6508,10 +6512,7 @@ void emitter::emitIns_R_S(instruction ins, emitAttr attr, regNumber ireg, int va sz += emitGetRexPrefixSize(ins); } - if (ins == INS_crc32) - { - sz += 1; - } + sz += emitAdjustSizeCrc32(ins, attr); id->idIns(ins); id->idInsFmt(fmt); @@ -8348,24 +8349,21 @@ void emitter::emitDispIns( #ifdef _TARGET_AMD64_ if (ins == INS_movsxd) { - printf("%s, %s", emitRegName(id->idReg1(), EA_8BYTE), sstr); + attr = EA_8BYTE; } else #endif if (ins == INS_movsx || ins == INS_movzx) { - printf("%s, %s", emitRegName(id->idReg1(), EA_PTRSIZE), sstr); + attr = EA_PTRSIZE; } else if ((ins == INS_crc32) && (attr != EA_8BYTE)) { // The idReg1 is always 4 bytes, but the size of idReg2 can vary. // This logic ensures that we print `crc32 eax, bx` instead of `crc32 ax, bx` - printf("%s, %s", emitRegName(id->idReg1(), EA_4BYTE), emitRegName(id->idReg2(), attr)); - } - else - { - printf("%s, %s", emitRegName(id->idReg1(), attr), sstr); + attr = EA_4BYTE; } + printf("%s, %s", emitRegName(id->idReg1(), attr), sstr); emitDispAddrMode(id); break; @@ -8589,25 +8587,22 @@ void emitter::emitDispIns( #ifdef _TARGET_AMD64_ if (ins == INS_movsxd) { - printf("%s, %s", emitRegName(id->idReg1(), EA_8BYTE), sstr); + attr = EA_8BYTE; } else #endif if (ins == INS_movsx || ins == INS_movzx) { - printf("%s, %s", emitRegName(id->idReg1(), EA_PTRSIZE), sstr); + attr = EA_PTRSIZE; } else if ((ins == INS_crc32) && (attr != EA_8BYTE)) { // The idReg1 is always 4 bytes, but the size of idReg2 can vary. // This logic ensures that we print `crc32 eax, bx` instead of `crc32 ax, bx` - printf("%s, %s", emitRegName(id->idReg1(), EA_4BYTE), emitRegName(id->idReg2(), attr)); - } - else - { - printf("%s, %s", emitRegName(id->idReg1(), attr), sstr); + attr = EA_4BYTE; } + printf("%s, %s", emitRegName(id->idReg1(), attr), sstr); emitDispFrameRef(id->idAddr()->iiaLclVar.lvaVarNum(), id->idAddr()->iiaLclVar.lvaOffset(), id->idDebugOnlyInfo()->idVarRefOffs, asmfm); @@ -8819,7 +8814,7 @@ void emitter::emitDispIns( { // The idReg1 is always 4 bytes, but the size of idReg2 can vary. // This logic ensures that we print `crc32 eax, bx` instead of `crc32 ax, bx` - printf("%s, %s", emitRegName(id->idReg1(), EA_4BYTE), emitRegName(id->idReg2(), attr)); + attr = EA_4BYTE; } printf("%s, %s", emitRegName(id->idReg1(), attr), sstr); offs = emitGetInsDsp(id); diff --git a/src/jit/emitxarch.h b/src/jit/emitxarch.h index 8d9cafa03b..cfdfdc8cbf 100644 --- a/src/jit/emitxarch.h +++ b/src/jit/emitxarch.h @@ -94,6 +94,22 @@ code_t AddRexPrefix(instruction ins, code_t code); bool EncodedBySSE38orSSE3A(instruction ins); bool Is4ByteSSEInstruction(instruction ins); +// Adjust code size for CRC32 that has 4-byte opcode +// but does not use SSE38 or EES3A encoding. +UNATIVE_OFFSET emitAdjustSizeCrc32(instruction ins, emitAttr attr) +{ + UNATIVE_OFFSET szDelta = 0; + if (ins == INS_crc32) + { + szDelta += 1; + if (attr == EA_2BYTE) + { + szDelta += 1; + } + } + return szDelta; +} + bool hasRexPrefix(code_t code) { #ifdef _TARGET_AMD64_ diff --git a/src/jit/lowerxarch.cpp b/src/jit/lowerxarch.cpp index 7eb7edf899..1ce37dce42 100644 --- a/src/jit/lowerxarch.cpp +++ b/src/jit/lowerxarch.cpp @@ -2680,9 +2680,17 @@ bool Lowering::IsContainableHWIntrinsicOp(GenTreeHWIntrinsic* containingNode, Ge assert(supportsUnalignedSIMDLoads == false); assert(supportsSIMDScalarLoads == false); - const unsigned expectedSize = genTypeSize(containingNode->TypeGet()); + unsigned expectedSize = genTypeSize(containingNode->TypeGet()); const unsigned operandSize = genTypeSize(node->TypeGet()); + // CRC32 codegen depends on its second oprand's type. + // Currently, we are using SIMDBaseType to store the op2Type info. + if (containingIntrinsicId == NI_SSE42_Crc32) + { + var_types op2Type = containingNode->gtSIMDBaseType; + expectedSize = genTypeSize(op2Type); + } + supportsGeneralLoads = (operandSize >= expectedSize); break; } diff --git a/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666.cs b/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666.cs new file mode 100644 index 0000000000..fa436ae7e0 --- /dev/null +++ b/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666.cs @@ -0,0 +1,271 @@ +using System; +using System.Runtime.Intrinsics.X86; +using System.Runtime.Intrinsics; + +namespace GitHub_21666 +{ + // CRC32 is a special instruction that has 4-byte opcode but does not use SSE38 or SSE3A encoding, + // so the compiler backend needs to specially check its code size. + // Test LZCNT as well to ensure that future changes do not impact 3-byte opcode instructions. + class GitHub_21666 + { + const int Pass = 100; + const int Fail = 0; + + static byte byteSF = 1; + static ushort ushortSF = 1; + static uint uintSF = 1; + static ulong ulongSF = 1; + + readonly static byte[] byteArray = new byte[10]; + readonly static ushort[] ushortArray = new ushort[10]; + readonly static uint[] uintArray = new uint[10]; + readonly static ulong[] ulongArray = new ulong[10]; + + static int Main(string[] args) + { + bool sucess = true; + byteSF = 0; + ushortSF = 0; + uintSF = 0; + ulongSF = 0; + sucess = sucess && TestByteContainment(); + sucess = sucess && TestUInt16Containment(); + sucess = sucess && TestUInt32Containment(); + sucess = sucess && TestUInt64Containment(); + + return sucess ? Pass : Fail; + } + + static unsafe bool TestByteContainment() + { + byte value = (byte)0; + byte* ptr = &value; + if (Sse42.IsSupported) + { + if (Sse42.Crc32(0xffffffffU, (byte)0) != 0xad82acaeU) + { + Console.WriteLine("TestByteContainment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, value) != 0xad82acaeU) + { + Console.WriteLine("TestByteContainment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, *ptr) != 0xad82acaeU) + { + Console.WriteLine("TestByteContainment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, byteArray[1]) != 0xad82acaeU) + { + Console.WriteLine("TestByteContainment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, byteArray[*ptr + 1]) != 0xad82acaeU) + { + Console.WriteLine("TestByteContainment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, byteSF) != 0xad82acaeU) + { + Console.WriteLine("TestByteContainment failed on Crc32"); + return false; + } + } + + return true; + } + + static unsafe bool TestUInt16Containment() + { + ushort value = (ushort)0; + ushort* ptr = &value; + if (Sse42.IsSupported) + { + if (Sse42.Crc32(0xffffffffU, (ushort)0) != 0xe9e882dU) + { + Console.WriteLine("TestUInt16Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, value) != 0xe9e882dU) + { + Console.WriteLine("TestUInt16Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, *ptr) != 0xe9e882dU) + { + Console.WriteLine("TestUInt16Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, ushortArray[1]) != 0xe9e882dU) + { + Console.WriteLine("TestUInt16Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, ushortArray[*ptr + 1]) != 0xe9e882dU) + { + Console.WriteLine("TestUInt16Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, ushortSF) != 0xe9e882dU) + { + Console.WriteLine("TestUInt16Containment failed on Crc32"); + return false; + } + } + + return true; + } + + static unsafe bool TestUInt32Containment() + { + uint value = (uint)0; + uint* ptr = &value; + if (Lzcnt.IsSupported) + { + if (Lzcnt.LeadingZeroCount(*ptr) != 32) + { + Console.WriteLine("TestUInt32Containment failed on LeadingZeroCount"); + return false; + } + + if (Lzcnt.LeadingZeroCount(uintArray[2]) != 32) + { + Console.WriteLine("TestUInt32Containment failed on LeadingZeroCount"); + return false; + } + + if (Lzcnt.LeadingZeroCount(uintArray[*ptr + 2]) != 32) + { + Console.WriteLine("TestUInt32Containment failed on LeadingZeroCount"); + return false; + } + + } + + uint* ptr1 = &value; + if (Sse42.IsSupported) + { + if (Sse42.Crc32(0xffffffffU, (uint)0) != 0xb798b438U) + { + Console.WriteLine("TestUInt32Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, value) != 0xb798b438U) + { + Console.WriteLine("TestUInt32Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, *ptr1) != 0xb798b438U) + { + Console.WriteLine("TestUInt32Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, uintArray[1]) != 0xb798b438U) + { + Console.WriteLine("TestUInt32Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, uintArray[*ptr + 1]) != 0xb798b438U) + { + Console.WriteLine("TestUInt32Containment failed on Crc32"); + return false; + } + + if (Sse42.Crc32(0xffffffffU, uintSF) != 0xb798b438U) + { + Console.WriteLine("TestUInt32Containment failed on Crc32"); + return false; + } + } + + return true; + } + + static unsafe bool TestUInt64Containment() + { + ulong value = (ulong)0; + ulong* ptr = &value; + if (Lzcnt.X64.IsSupported) + { + if (Lzcnt.X64.LeadingZeroCount(*ptr) != 64) + { + Console.WriteLine("TestUInt64Containment failed on LeadingZeroCount"); + return false; + } + + if (Lzcnt.X64.LeadingZeroCount(ulongArray[2]) != 64) + { + Console.WriteLine("TestUInt64Containment failed on LeadingZeroCount"); + return false; + } + + if (Lzcnt.X64.LeadingZeroCount(ulongArray[*ptr + 2]) != 64) + { + Console.WriteLine("TestUInt64Containment failed on LeadingZeroCount"); + return false; + } + + } + + ulong* ptr1 = &value; + + if (Sse42.X64.IsSupported) + { + if (Sse42.X64.Crc32(0xffffffffffffffffUL, 0) != 0x0000000073d74d75UL) + { + Console.WriteLine("TestUInt64Containment failed on Crc32"); + return false; + } + + if (Sse42.X64.Crc32(0xffffffffffffffffUL, value) != 0x0000000073d74d75UL) + { + Console.WriteLine("TestUInt64Containment failed on Crc32"); + return false; + } + + if (Sse42.X64.Crc32(0xffffffffffffffffUL, *ptr1) != 0x0000000073d74d75UL) + { + Console.WriteLine("TestUInt64Containment failed on Crc32"); + return false; + } + + if (Sse42.X64.Crc32(0xffffffffffffffffUL, ulongArray[1]) != 0x0000000073d74d75UL) + { + Console.WriteLine("TestUInt64Containment failed on Crc32"); + return false; + } + + if (Sse42.X64.Crc32(0xffffffffffffffffUL, ulongArray[*ptr + 1]) != 0x0000000073d74d75UL) + { + Console.WriteLine("TestUInt64Containment failed on Crc32"); + return false; + } + + if (Sse42.X64.Crc32(0xffffffffffffffffUL, ulongSF) != 0x0000000073d74d75UL) + { + Console.WriteLine("TestUInt64Containment failed on Crc32"); + return false; + } + } + + return true; + } + } +} diff --git a/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_r.csproj b/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_r.csproj new file mode 100644 index 0000000000..23540a7600 --- /dev/null +++ b/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_r.csproj @@ -0,0 +1,34 @@ +<?xml version="1.0" encoding="utf-8"?> +<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> + <PropertyGroup> + <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> + <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> + <SchemaVersion>2.0</SchemaVersion> + <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid> + <OutputType>Exe</OutputType> + <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> + <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> + <AllowUnsafeBlocks>true</AllowUnsafeBlocks> + </PropertyGroup> + <!-- Default configurations to help VS understand the configurations --> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' " /> + <ItemGroup> + <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies"> + <Visible>False</Visible> + </CodeAnalysisDependentAssemblyPaths> + </ItemGroup> + <PropertyGroup> + <DebugType>Embedded</DebugType> + <Optimize></Optimize> + </PropertyGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <ItemGroup> + <Compile Include="GitHub_21666.cs" /> + </ItemGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup> +</Project> diff --git a/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_ro.csproj b/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_ro.csproj new file mode 100644 index 0000000000..4d46fc1bb8 --- /dev/null +++ b/tests/src/JIT/HardwareIntrinsics/X86/Regression/GitHub_21666/GitHub_21666_ro.csproj @@ -0,0 +1,34 @@ +<?xml version="1.0" encoding="utf-8"?> +<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> + <PropertyGroup> + <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> + <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> + <SchemaVersion>2.0</SchemaVersion> + <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid> + <OutputType>Exe</OutputType> + <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> + <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> + <AllowUnsafeBlocks>true</AllowUnsafeBlocks> + </PropertyGroup> + <!-- Default configurations to help VS understand the configurations --> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' " /> + <ItemGroup> + <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies"> + <Visible>False</Visible> + </CodeAnalysisDependentAssemblyPaths> + </ItemGroup> + <PropertyGroup> + <DebugType>Embedded</DebugType> + <Optimize>True</Optimize> + </PropertyGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <ItemGroup> + <Compile Include="GitHub_21666.cs" /> + </ItemGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup> +</Project> |