[RyuJit/ARM] Fix lsra BuildShiftRotate

src/jit/lsra.h

@@ -1654,9 +1654,14 @@ class LinearScan : public LinearScanInterface
 
     void BuildStoreLoc(GenTree* tree);
     void BuildReturn(GenTree* tree);
+#ifdef _TARGET_XARCH_
     // This method, unlike the others, returns the number of sources, since it may be called when
     // 'tree' is contained.
     int BuildShiftRotate(GenTree* tree);
+#endif // _TARGET_XARCH_
+#ifdef _TARGET_ARM_
+    void BuildShiftLongCarry(GenTree* tree);
+#endif
     void BuildPutArgReg(GenTreeUnOp* node);
     void BuildCall(GenTreeCall* call);
     void BuildCmp(GenTree* tree);

src/jit/lsraarm.cpp

@@ -112,6 +112,46 @@ void LinearScan::BuildLclHeap(GenTree* tree)
     }
 }
 
+//------------------------------------------------------------------------
+// BuildShiftLongCarry: Set the node info for GT_LSH_HI or GT_RSH_LO.
+//
+// Arguments:
+//    tree      - The node of interest
+//
+// Note: these operands have uses that interfere with the def and need the special handling.
+//
+void LinearScan::BuildShiftLongCarry(GenTree* tree)
+{
+    assert(tree->OperGet() == GT_LSH_HI || tree->OperGet() == GT_RSH_LO);
+
+    GenTree* source = tree->gtOp.gtOp1;
+    assert((source->OperGet() == GT_LONG) && source->isContained());
+
+    TreeNodeInfo* info = currentNodeInfo;
+    info->srcCount     = 2;
+
+    LocationInfoListNode* sourceLoInfo = getLocationInfo(source->gtOp.gtOp1);
+    LocationInfoListNode* sourceHiInfo = getLocationInfo(source->gtOp.gtOp2);
+    if (tree->OperGet() == GT_LSH_HI)
+    {
+        sourceLoInfo->info.isDelayFree = true;
+    }
+    else
+    {
+        sourceHiInfo->info.isDelayFree = true;
+    }
+    useList.Append(sourceLoInfo);
+    useList.Append(sourceHiInfo);
+    info->hasDelayFreeSrc = true;
+
+    GenTree* shiftBy = tree->gtOp.gtOp2;
+    if (!shiftBy->isContained())
+    {
+        appendLocationInfoToList(shiftBy);
+        info->srcCount += 1;
+    }
+}
+
 //------------------------------------------------------------------------
 // BuildNode: Set the register requirements for RA.
 //
@@ -335,11 +375,22 @@ void LinearScan::BuildNode(GenTree* tree)
         case GT_AND:
         case GT_OR:
         case GT_XOR:
+        case GT_LSH:
+        case GT_RSH:
+        case GT_RSZ:
+        case GT_ROR:
             assert(info->dstCount == 1);
             info->srcCount = appendBinaryLocationInfoToList(tree->AsOp());
             assert(info->srcCount == (tree->gtOp.gtOp2->isContained() ? 1 : 2));
             break;
 
+        case GT_LSH_HI:
+        case GT_RSH_LO:
+            assert(info->dstCount == 1);
+            BuildShiftLongCarry(tree);
+            assert(info->srcCount == (tree->gtOp.gtOp2->isContained() ? 2 : 3));
+            break;
+
         case GT_RETURNTRAP:
             // this just turns into a compare of its child with an int
             // + a conditional call
@@ -553,15 +604,6 @@ void LinearScan::BuildNode(GenTree* tree)
             appendLocationInfoToList(tree->gtOp.gtOp1);
             break;
 
-        case GT_LSH:
-        case GT_RSH:
-        case GT_RSZ:
-        case GT_ROR:
-        case GT_LSH_HI:
-        case GT_RSH_LO:
-            BuildShiftRotate(tree);
-            break;
-
         case GT_EQ:
         case GT_NE:
         case GT_LT:

src/jit/lsraarm64.cpp

@@ -219,6 +219,10 @@ void LinearScan::BuildNode(GenTree* tree)
         case GT_AND:
         case GT_OR:
         case GT_XOR:
+        case GT_LSH:
+        case GT_RSH:
+        case GT_RSZ:
+        case GT_ROR:
             info->srcCount = appendBinaryLocationInfoToList(tree->AsOp());
             assert(info->dstCount == 1);
             break;
@@ -341,13 +345,6 @@ void LinearScan::BuildNode(GenTree* tree)
             assert(info->dstCount == 1);
             break;
 
-        case GT_LSH:
-        case GT_RSH:
-        case GT_RSZ:
-        case GT_ROR:
-            BuildShiftRotate(tree);
-            break;
-
         case GT_EQ:
         case GT_NE:
         case GT_LT:

src/jit/lsraarmarch.cpp

@@ -111,59 +111,6 @@ void LinearScan::BuildIndir(GenTreeIndir* indirTree)
 #endif // FEATURE_SIMD
 }
 
-//------------------------------------------------------------------------
-// BuildShiftRotate: Set the NodeInfo for a shift or rotate.
-//
-// Arguments:
-//    tree      - The node of interest
-//
-// Return Value:
-//    None.
-//
-int LinearScan::BuildShiftRotate(GenTree* tree)
-{
-    TreeNodeInfo* info    = currentNodeInfo;
-    GenTree*      source  = tree->gtOp.gtOp1;
-    GenTree*      shiftBy = tree->gtOp.gtOp2;
-    assert(info->dstCount == 1);
-    if (!shiftBy->isContained())
-    {
-        appendLocationInfoToList(shiftBy);
-        info->srcCount = 1;
-    }
-
-#ifdef _TARGET_ARM_
-
-    // The first operand of a GT_LSH_HI and GT_RSH_LO oper is a GT_LONG so that
-    // we can have a three operand form. Increment the srcCount.
-    if (tree->OperGet() == GT_LSH_HI || tree->OperGet() == GT_RSH_LO)
-    {
-        assert((source->OperGet() == GT_LONG) && source->isContained());
-        info->srcCount += 2;
-
-        LocationInfoListNode* sourceLoInfo = getLocationInfo(source->gtOp.gtOp1);
-        useList.Append(sourceLoInfo);
-        LocationInfoListNode* sourceHiInfo = getLocationInfo(source->gtOp.gtOp2);
-        useList.Append(sourceHiInfo);
-        if (tree->OperGet() == GT_LSH_HI)
-        {
-            sourceLoInfo->info.isDelayFree = true;
-        }
-        else
-        {
-            sourceHiInfo->info.isDelayFree = true;
-        }
-        info->hasDelayFreeSrc = true;
-    }
-    else
-#endif // _TARGET_ARM_
-    {
-        appendLocationInfoToList(source);
-        info->srcCount++;
-    }
-    return info->srcCount;
-}
-
 //------------------------------------------------------------------------
 // BuildCall: Set the NodeInfo for a call.
 //

tests/src/JIT/Regression/JitBlue/DevDiv_545497/DevDiv_545497.il

@@ -0,0 +1,117 @@
+// 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.
+
+// The bug that this test captures was a case where arm LSRA built wrong uses order for
+// the IL_0090 shift operand.
+
+.assembly extern System.Runtime { auto }
+.assembly DevDiv_524309 {}
+
+.class private auto ansi beforefieldinit DevDiv_545497
+       extends [System.Runtime]System.Object
+{
+  .method private hidebysig static uint8 
+          ILGEN_METHOD(float32 a,
+                       uint32 b,
+                       uint64 c) cil managed
+  {
+	.maxstack  194
+	.locals init (float64, bool, unsigned int64)
+	IL_0000: ldloc.s 0x01
+	IL_0002: conv.r.un
+	IL_0028: conv.ovf.u8.un
+	IL_002c: ldloc.s 0x00
+	IL_002e: conv.ovf.i8.un
+	IL_002f: stloc 0x0002
+	IL_0033: ldarg 0x0002
+	IL_003f: ldloc 0x0001
+	IL_0043: ldloc.s 0x01
+	IL_0045: shl
+	IL_0047: shr.un
+	IL_0049: ldarg.s 0x01
+	IL_005b: conv.i1
+	IL_005d: conv.ovf.u
+	IL_005e: ldloc 0x0001
+	IL_0063: ldc.i4 0
+	IL_0071: ldloc 0x0002
+	IL_007a: conv.ovf.i1.un
+	IL_007b: stloc 0x0001
+	IL_007f: conv.i1
+	IL_0081: rem.un
+	IL_0082: ldarg 0x0002
+	IL_0087: dup
+	IL_0088: add.ovf.un
+	IL_008a: ldloc.s 0x01
+	IL_008c: ldloc 0x0001
+	IL_0090: shr           // This shift moves loc0x01 >> loc0x01 and exposed the original issue.
+	IL_0091: conv.ovf.i8
+	IL_0092: mul
+	IL_0093: ldloc.s 0x01
+	IL_0095: ldarg.s 0x01
+	IL_0097: div.un
+	IL_0098: starg 0x0001
+	IL_009c: ldloc 0x0002
+	IL_00b0: clt
+	IL_00b2: conv.ovf.i1.un
+	IL_00b3: sub.ovf.un
+	IL_00b7: sub.ovf
+	IL_00b9: shr.un
+	IL_00ba: ldloc.s 0x01
+	IL_00bc: conv.ovf.u8.un
+	IL_00bd: and
+	IL_00bf: clt.un
+	IL_00c1: ret
+  } // end of method DevDiv_545497::ILGEN_METHOD
+
+  .method private hidebysig static int32 
+          Main() cil managed
+  {
+    .entrypoint
+    // Code size       31 (0x1f)
+    .maxstack  3
+    .locals init (int32 V_0)
+    IL_0000:  nop
+    .try
+    {
+      IL_0001:  nop
+      IL_0002:  ldc.r4     0.0
+      IL_0007:  ldc.i4.0
+      IL_0008:  ldc.i4.0
+      IL_0009:  conv.i8
+      IL_000a:  call       uint8 DevDiv_545497::ILGEN_METHOD(float32,
+                                                             uint32,
+                                                             uint64)
+      IL_000f:  pop
+      IL_0010:  nop
+      IL_0011:  leave.s    IL_0018
+
+    }  // end .try
+    catch [System.Runtime]System.Object 
+    {
+      IL_0013:  pop
+      IL_0014:  nop
+      IL_0015:  nop
+      IL_0016:  leave.s    IL_0018
+
+    }  // end handler
+    IL_0018:  ldc.i4.s   100
+    IL_001a:  stloc.0
+    IL_001b:  br.s       IL_001d
+
+    IL_001d:  ldloc.0
+    IL_001e:  ret
+  } // end of method DevDiv_545497::Main
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    // Code size       8 (0x8)
+    .maxstack  8
+    IL_0000:  ldarg.0
+    IL_0001:  call       instance void [System.Runtime]System.Object::.ctor()
+    IL_0006:  nop
+    IL_0007:  ret
+  } // end of method DevDiv_545497::.ctor
+
+} // end of class DevDiv_545497

tests/src/JIT/Regression/JitBlue/DevDiv_545497/DevDiv_545497.ilproj

@@ -0,0 +1,23 @@
+<?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>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>