From: Andy Ayers Date: Sat, 6 Jan 2018 01:47:09 +0000 (-0800) Subject: JIT: fix decompose long left shift for overshift cases (#15704) X-Git-Tag: accepted/tizen/base/20180629.140029~193 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=16246c9117aa456eb0786d4e23e827dce585b6c0;p=platform%2Fupstream%2Fcoreclr.git JIT: fix decompose long left shift for overshift cases (#15704) Need to reduce the shift amount modulo 64 to match the helper and `gtFoldExpr` behavior. Since reduced amount must be less than 64 we can remove handling for that case. Also updating the arm LLSH helper. Re-enable the test case disabled by #15567 and also enable for arm/arm64. Closes #15566. --- diff --git a/src/jit/decomposelongs.cpp b/src/jit/decomposelongs.cpp index 7654c13..89092b0 100644 --- a/src/jit/decomposelongs.cpp +++ b/src/jit/decomposelongs.cpp @@ -1112,6 +1112,24 @@ GenTree* DecomposeLongs::DecomposeShift(LIR::Use& use) { case GT_LSH: { + // Reduce count modulo 64 to match behavior found in + // the LLSH helper and in gtFoldExpr. + count &= 0x3F; + + // Retest for zero shift + if (count == 0) + { + GenTree* next = shift->gtNext; + // Remove shift and don't do anything else. + if (shift->IsUnusedValue()) + { + gtLong->SetUnusedValue(); + } + Range().Remove(shift); + use.ReplaceWith(m_compiler, gtLong); + return next; + } + if (count < 32) { // For shifts of < 32 bits, we transform the code to: @@ -1157,7 +1175,7 @@ GenTree* DecomposeLongs::DecomposeShift(LIR::Use& use) } else { - assert(count >= 32); + assert(count >= 32 && count < 64); // Since we're left shifting at least 32 bits, we can remove the hi part of the shifted value iff // it has no side effects. @@ -1173,57 +1191,30 @@ GenTree* DecomposeLongs::DecomposeShift(LIR::Use& use) hiOp1->SetUnusedValue(); } - if (count < 64) + if (count == 32) { - if (count == 32) - { - // Move loOp1 into hiResult (shift of 32 bits is just a mov of lo to hi) - // We need to make sure that we save lo to a temp variable so that we don't overwrite lo - // before saving it to hi in the case that we are doing an inplace shift. I.e.: - // x = x << 32 + // Move loOp1 into hiResult (shift of 32 bits is just a mov of lo to hi) + // We need to make sure that we save lo to a temp variable so that we don't overwrite lo + // before saving it to hi in the case that we are doing an inplace shift. I.e.: + // x = x << 32 - LIR::Use loOp1Use(Range(), >Long->gtOp.gtOp1, gtLong); - loOp1Use.ReplaceWithLclVar(m_compiler, m_blockWeight); - - hiResult = loOp1Use.Def(); - Range().Remove(gtLong); - } - else - { - Range().Remove(gtLong); - assert(count > 32 && count < 64); + LIR::Use loOp1Use(Range(), >Long->gtOp.gtOp1, gtLong); + loOp1Use.ReplaceWithLclVar(m_compiler, m_blockWeight); - // Move loOp1 into hiResult, do a GT_LSH with count - 32. - // We will compute hiResult before loResult in this case, so we don't need to store lo to a - // temp - GenTree* shiftBy = m_compiler->gtNewIconNode(count - 32, TYP_INT); - hiResult = m_compiler->gtNewOperNode(oper, TYP_INT, loOp1, shiftBy); - Range().InsertBefore(shift, shiftBy, hiResult); - } + hiResult = loOp1Use.Def(); + Range().Remove(gtLong); } else { - assert(count >= 64); - Range().Remove(gtLong); - - // Since we're left shifting at least 64 bits, we can remove the lo part of the shifted value - // iff it has no side effects. - // - // TODO-CQ: we could go perform this removal transitively (i.e. iteratively remove everything - // that feeds the lo operand while there are no side effects) - if ((loOp1->gtFlags & (GTF_ALL_EFFECT | GTF_SET_FLAGS)) == 0) - { - Range().Remove(loOp1, true); - } - else - { - loOp1->SetUnusedValue(); - } - - // Zero out hi (shift of >= 64 bits moves all the bits out of the two registers) - hiResult = m_compiler->gtNewZeroConNode(TYP_INT); - Range().InsertBefore(shift, hiResult); + assert(count > 32 && count < 64); + + // Move loOp1 into hiResult, do a GT_LSH with count - 32. + // We will compute hiResult before loResult in this case, so we don't need to store lo to a + // temp + GenTree* shiftBy = m_compiler->gtNewIconNode(count - 32, TYP_INT); + hiResult = m_compiler->gtNewOperNode(oper, TYP_INT, loOp1, shiftBy); + Range().InsertBefore(shift, shiftBy, hiResult); } // Zero out loResult (shift of >= 32 bits shifts all lo bits to hiResult) diff --git a/src/vm/jithelpers.cpp b/src/vm/jithelpers.cpp index dda73f2..12f110c 100644 --- a/src/vm/jithelpers.cpp +++ b/src/vm/jithelpers.cpp @@ -456,7 +456,7 @@ HCIMPLEND HCIMPL2_VV(UINT64, JIT_LLsh, UINT64 num, int shift) { FCALL_CONTRACT; - return num << shift; + return num << (shift & 0x3F); } HCIMPLEND diff --git a/tests/arm/Tests.lst b/tests/arm/Tests.lst index 2b929e9..122985e 100644 --- a/tests/arm/Tests.lst +++ b/tests/arm/Tests.lst @@ -90803,3 +90803,19 @@ Expected=0 MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 + +[GitHub_15077.cmd_11400] +RelativePath=JIT\Regression\JitBlue\GitHub_15077\GitHub_15077\GitHub_15077.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_15077\GitHub_15077 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_15291.cmd_11401] +RelativePath=JIT\Regression\JitBlue\GitHub_15291\GitHub_15291\GitHub_15291.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_15291\GitHub_15291 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 diff --git a/tests/arm64/Tests.lst b/tests/arm64/Tests.lst index 061c247..5402125 100644 --- a/tests/arm64/Tests.lst +++ b/tests/arm64/Tests.lst @@ -90803,3 +90803,20 @@ Expected=0 MaxAllowedDurationSeconds=600 Categories=EXPECTED_PASS HostStyle=0 + +[GitHub_15077.cmd_11718] +RelativePath=JIT\Regression\JitBlue\GitHub_15077\GitHub_15077\GitHub_15077.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_15077\GitHub_15077 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + +[GitHub_15291.cmd_11719] +RelativePath=JIT\Regression\JitBlue\GitHub_15291\GitHub_15291\GitHub_15291.cmd +WorkingDir=JIT\Regression\JitBlue\GitHub_15291\GitHub_15291 +Expected=0 +MaxAllowedDurationSeconds=600 +Categories=EXPECTED_PASS +HostStyle=0 + diff --git a/tests/issues.targets b/tests/issues.targets index 2960ddb..795369a 100644 --- a/tests/issues.targets +++ b/tests/issues.targets @@ -219,9 +219,6 @@ 11469, The test causes OutOfMemory exception in crossgen mode. - - 15566 -