JIT: fix decompose long left shift for overshift cases (#15704)
authorAndy Ayers <andya@microsoft.com>
Sat, 6 Jan 2018 01:47:09 +0000 (17:47 -0800)
committerGitHub <noreply@github.com>
Sat, 6 Jan 2018 01:47:09 +0000 (17:47 -0800)
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.

src/jit/decomposelongs.cpp
src/vm/jithelpers.cpp
tests/arm/Tests.lst
tests/arm64/Tests.lst
tests/issues.targets

index 7654c13..89092b0 100644 (file)
@@ -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(), &gtLong->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(), &gtLong->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)
index dda73f2..12f110c 100644 (file)
@@ -456,7 +456,7 @@ HCIMPLEND
 HCIMPL2_VV(UINT64, JIT_LLsh, UINT64 num, int shift)
 {
     FCALL_CONTRACT;
-    return num << shift;
+    return num << (shift & 0x3F);
 }
 HCIMPLEND
 
index 2b929e9..122985e 100644 (file)
@@ -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
index 061c247..5402125 100644 (file)
@@ -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
+
index 2960ddb..795369a 100644 (file)
         <ExcludeList Include="$(XunitTestBinBase)\JIT\Regression\JitBlue\DevDiv_255294\DevDiv_255294\DevDiv_255294.cmd">
             <Issue>11469, The test causes OutOfMemory exception in crossgen mode.</Issue> 
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)\JIT\Regression\JitBlue\GitHub_15291\GitHub_15291\GitHub_15291.cmd">
-            <Issue>15566</Issue> 
-        </ExcludeList>
     </ItemGroup>
 
     <!-- The following are ARM32 failures -->