diff options
author | Pat Gavlin <pgavlin@gmail.com> | 2016-11-18 10:46:46 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-11-18 10:46:46 -0800 |
commit | 4807a27422964b3ae7fca2deca2afb3d1b383594 (patch) | |
tree | 1c85d0ee5fed2debc85ff3b9ac988f6452714b62 | |
parent | 4f8ba1e05e53e40a52fd66d7660efa8a992da9a9 (diff) | |
parent | 7e00ee6be9590670d060117fd3932808bd7a86e6 (diff) | |
download | coreclr-4807a27422964b3ae7fca2deca2afb3d1b383594.tar.gz coreclr-4807a27422964b3ae7fca2deca2afb3d1b383594.tar.bz2 coreclr-4807a27422964b3ae7fca2deca2afb3d1b383594.zip |
Merge pull request #8198 from pgavlin/GH8170
Fix codegen for `(umod (gt_long) (const int))`.
-rw-r--r-- | src/jit/codegenlinear.h | 4 | ||||
-rw-r--r-- | src/jit/codegenxarch.cpp | 167 | ||||
-rw-r--r-- | src/jit/lowerxarch.cpp | 4 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.cs | 23 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.csproj | 46 |
5 files changed, 188 insertions, 56 deletions
diff --git a/src/jit/codegenlinear.h b/src/jit/codegenlinear.h index c0260ce405..6ad72cf781 100644 --- a/src/jit/codegenlinear.h +++ b/src/jit/codegenlinear.h @@ -16,6 +16,10 @@ void genCodeForTreeNode(GenTreePtr treeNode); void genCodeForBinary(GenTreePtr treeNode); +#if defined(_TARGET_X86_) +void genCodeForLongUMod(GenTreeOp* node); +#endif // _TARGET_X86_ + void genCodeForDivMod(GenTreeOp* treeNode); void genCodeForMulHi(GenTreeOp* treeNode); diff --git a/src/jit/codegenxarch.cpp b/src/jit/codegenxarch.cpp index 6553d8c6ee..d7f1ba52db 100644 --- a/src/jit/codegenxarch.cpp +++ b/src/jit/codegenxarch.cpp @@ -546,6 +546,90 @@ void CodeGen::genCodeForMulHi(GenTreeOp* treeNode) } } +#ifdef _TARGET_X86_ +//------------------------------------------------------------------------ +// genCodeForLongUMod: Generate code for a tree of the form +// `(umod (gt_long x y) (const int))` +// +// Arguments: +// node - the node for which to generate code +// +void CodeGen::genCodeForLongUMod(GenTreeOp* node) +{ + assert(node != nullptr); + assert(node->OperGet() == GT_UMOD); + assert(node->TypeGet() == TYP_INT); + + GenTreeOp* const dividend = node->gtOp1->AsOp(); + assert(dividend->OperGet() == GT_LONG); + assert(varTypeIsLong(dividend)); + + GenTree* const dividendLo = dividend->gtOp1; + GenTree* const dividendHi = dividend->gtOp2; + assert(!dividendLo->isContained()); + assert(!dividendHi->isContained()); + + GenTreeIntCon* const divisor = node->gtOp2->AsIntCon(); + assert(!divisor->isContained()); + assert(divisor->gtIconVal >= 2); + assert(divisor->gtIconVal <= 0x3fffffff); + + genConsumeOperands(node); + + // dividendLo must be in RAX; dividendHi must be in RDX + genCopyRegIfNeeded(dividendLo, REG_EAX); + genCopyRegIfNeeded(dividendHi, REG_EDX); + + // At this point, EAX:EDX contains the 64bit dividend and op2->gtRegNum + // contains the 32bit divisor. We want to generate the following code: + // + // cmp edx, divisor->gtRegNum + // jb noOverflow + // + // mov temp, eax + // mov eax, edx + // xor edx, edx + // div divisor->gtRegNum + // mov eax, temp + // + // noOverflow: + // div divisor->gtRegNum + // + // This works because (a * 2^32 + b) % c = ((a % c) * 2^32 + b) % c. + + BasicBlock* const noOverflow = genCreateTempLabel(); + + // cmp edx, divisor->gtRegNum + // jb noOverflow + inst_RV_RV(INS_cmp, REG_EDX, divisor->gtRegNum); + inst_JMP(EJ_jb, noOverflow); + + // mov temp, eax + // mov eax, edx + // xor edx, edx + // div divisor->gtRegNum + // mov eax, temp + const regNumber tempReg = genRegNumFromMask(node->gtRsvdRegs); + inst_RV_RV(INS_mov, tempReg, REG_EAX, TYP_INT); + inst_RV_RV(INS_mov, REG_EAX, REG_EDX, TYP_INT); + instGen_Set_Reg_To_Zero(EA_PTRSIZE, REG_EDX); + inst_RV(INS_div, divisor->gtRegNum, TYP_INT); + inst_RV_RV(INS_mov, REG_EAX, tempReg, TYP_INT); + + // noOverflow: + // div divisor->gtRegNum + genDefineTempLabel(noOverflow); + inst_RV(INS_div, divisor->gtRegNum, TYP_INT); + + const regNumber targetReg = node->gtRegNum; + if (targetReg != REG_EDX) + { + inst_RV_RV(INS_mov, targetReg, REG_RDX, TYP_INT); + } + genProduceReg(node); +} +#endif // _TARGET_X86_ + //------------------------------------------------------------------------ // genCodeForDivMod: Generate code for a DIV or MOD operation. // @@ -554,7 +638,15 @@ void CodeGen::genCodeForMulHi(GenTreeOp* treeNode) // void CodeGen::genCodeForDivMod(GenTreeOp* treeNode) { - GenTree* dividend = treeNode->gtOp1; + GenTree* dividend = treeNode->gtOp1; +#ifdef _TARGET_X86_ + if (varTypeIsLong(dividend->TypeGet())) + { + genCodeForLongUMod(treeNode); + return; + } +#endif // _TARGET_X86_ + GenTree* divisor = treeNode->gtOp2; genTreeOps oper = treeNode->OperGet(); emitAttr size = emitTypeSize(treeNode); @@ -562,27 +654,8 @@ void CodeGen::genCodeForDivMod(GenTreeOp* treeNode) var_types targetType = treeNode->TypeGet(); emitter* emit = getEmitter(); -#ifdef _TARGET_X86_ - bool dividendIsLong = varTypeIsLong(dividend->TypeGet()); - GenTree* dividendLo = nullptr; - GenTree* dividendHi = nullptr; - - if (dividendIsLong) - { - // If dividend is a GT_LONG, the we need to make sure its lo and hi parts are not contained. - dividendLo = dividend->gtGetOp1(); - dividendHi = dividend->gtGetOp2(); - - assert(!dividendLo->isContained()); - assert(!dividendHi->isContained()); - assert(divisor->IsCnsIntOrI()); - } - else -#endif - { - // dividend is not contained. - assert(!dividend->isContained()); - } + // dividend is not contained. + assert(!dividend->isContained()); genConsumeOperands(treeNode->AsOp()); if (varTypeIsFloating(targetType)) @@ -617,37 +690,19 @@ void CodeGen::genCodeForDivMod(GenTreeOp* treeNode) } else { -#ifdef _TARGET_X86_ - if (dividendIsLong) - { - assert(dividendLo != nullptr && dividendHi != nullptr); + // dividend must be in RAX + genCopyRegIfNeeded(dividend, REG_RAX); - // dividendLo must be in RAX; dividendHi must be in RDX - genCopyRegIfNeeded(dividendLo, REG_EAX); - genCopyRegIfNeeded(dividendHi, REG_EDX); - } - else -#endif + // zero or sign extend rax to rdx + if (oper == GT_UMOD || oper == GT_UDIV) { - // dividend must be in RAX - genCopyRegIfNeeded(dividend, REG_RAX); + instGen_Set_Reg_To_Zero(EA_PTRSIZE, REG_EDX); } - -#ifdef _TARGET_X86_ - if (!dividendIsLong) -#endif + else { - // zero or sign extend rax to rdx - if (oper == GT_UMOD || oper == GT_UDIV) - { - instGen_Set_Reg_To_Zero(EA_PTRSIZE, REG_EDX); - } - else - { - emit->emitIns(INS_cdq, size); - // the cdq instruction writes RDX, So clear the gcInfo for RDX - gcInfo.gcMarkRegSetNpt(RBM_RDX); - } + emit->emitIns(INS_cdq, size); + // the cdq instruction writes RDX, So clear the gcInfo for RDX + gcInfo.gcMarkRegSetNpt(RBM_RDX); } // Perform the 'targetType' (64-bit or 32-bit) divide instruction @@ -7016,27 +7071,27 @@ void CodeGen::genCkfinite(GenTreePtr treeNode) // // For TYP_DOUBLE, we'll generate (for targetReg != op1->gtRegNum): // movaps targetReg, op1->gtRegNum - // shufps targetReg, targetReg, 0xB1 // WZYX => ZWXY - // mov_xmm2i tmpReg, targetReg // tmpReg <= Y + // shufps targetReg, targetReg, 0xB1 // WZYX => ZWXY + // mov_xmm2i tmpReg, targetReg // tmpReg <= Y // and tmpReg, <mask> // cmp tmpReg, <mask> // je <throw block> // movaps targetReg, op1->gtRegNum // copy the value again, instead of un-shuffling it // // For TYP_DOUBLE with (targetReg == op1->gtRegNum): - // shufps targetReg, targetReg, 0xB1 // WZYX => ZWXY - // mov_xmm2i tmpReg, targetReg // tmpReg <= Y + // shufps targetReg, targetReg, 0xB1 // WZYX => ZWXY + // mov_xmm2i tmpReg, targetReg // tmpReg <= Y // and tmpReg, <mask> // cmp tmpReg, <mask> // je <throw block> - // shufps targetReg, targetReg, 0xB1 // ZWXY => WZYX + // shufps targetReg, targetReg, 0xB1 // ZWXY => WZYX // // For TYP_FLOAT, it's the same as _TARGET_64BIT_: - // mov_xmm2i tmpReg, targetReg // tmpReg <= low 32 bits + // mov_xmm2i tmpReg, targetReg // tmpReg <= low 32 bits // and tmpReg, <mask> // cmp tmpReg, <mask> // je <throw block> - // movaps targetReg, op1->gtRegNum // only if targetReg != op1->gtRegNum + // movaps targetReg, op1->gtRegNum // only if targetReg != op1->gtRegNum regNumber copyToTmpSrcReg; // The register we'll copy to the integer temp. diff --git a/src/jit/lowerxarch.cpp b/src/jit/lowerxarch.cpp index da51f3a8fc..a1711efb95 100644 --- a/src/jit/lowerxarch.cpp +++ b/src/jit/lowerxarch.cpp @@ -2493,6 +2493,10 @@ void Lowering::TreeNodeInfoInitModDiv(GenTree* tree) assert(op2->IsCnsIntOrI()); info->srcCount++; + // This situation also requires an internal register. + info->internalIntCount = 1; + info->setInternalCandidates(l, l->allRegs(TYP_INT)); + loVal->gtLsraInfo.setSrcCandidates(l, RBM_EAX); hiVal->gtLsraInfo.setSrcCandidates(l, RBM_EDX); } diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.cs b/tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.cs new file mode 100644 index 0000000000..d3fe8d9cf3 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Runtime.CompilerServices; + +// This test checks for proper behavior w.r.t. overflow for expressions of the form `val % constant` where `val` is an +// unsigned long and `constant` is a 32- or 64-bit integer constant in the range [2,0x3fffffff]. These expressions +// should never produce an overflow exception. + +static class C +{ + [MethodImpl(MethodImplOptions.NoInlining)] + static uint M(ulong l) + { + return (uint)(l % 10000000); + } + + static int Main() + { + return M(ulong.MaxValue) == 9551615 ? 100 : 101; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.csproj new file mode 100644 index 0000000000..81aa7299d0 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_8170/GitHub_8170.csproj @@ -0,0 +1,46 @@ +<?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> + <AssemblyName>$(MSBuildProjectName)</AssemblyName> + <SchemaVersion>2.0</SchemaVersion> + <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid> + <OutputType>Exe</OutputType> + <AppDesignerFolder>Properties</AppDesignerFolder> + <FileAlignment>512</FileAlignment> + <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> + <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath> + <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> + + <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp> + </PropertyGroup> + <!-- Default configurations to help VS understand the configurations --> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> + </PropertyGroup> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "> + </PropertyGroup> + <ItemGroup> + <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies"> + <Visible>False</Visible> + </CodeAnalysisDependentAssemblyPaths> + </ItemGroup> + <PropertyGroup> + <DebugType>None</DebugType> + <Optimize>True</Optimize> + </PropertyGroup> + <ItemGroup> + <Compile Include="GitHub_8170.cs" /> + </ItemGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <PropertyGroup> + <ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson> + <ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson> + </PropertyGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "> + </PropertyGroup> +</Project> |