[RyuJit/ARM] Fix lsra BuildShiftRotate (#17103)

* add repro

* delete BuildShiftRotate for arm

* fix GT_LSH_HI and GT_RSH_LO

* return the special handling for GT_LSH_HI and GT_RSH_LO

* fix the header

4 files changed, 60 insertions, 69 deletions

diff --git a/src/jit/lsra.h b/src/jit/lsra.h
index 62ba38674f..d3d3f99b00 100644
--- a/src/jit/lsra.h
+++ b/src/jit/lsra.h
@@ -1654,9 +1654,14 @@ private:
 
     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);
diff --git a/src/jit/lsraarm.cpp b/src/jit/lsraarm.cpp
index df4a435589..63e93fa8ea 100644
--- a/src/jit/lsraarm.cpp
+++ b/src/jit/lsraarm.cpp
@@ -113,6 +113,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.
 //
 // Notes:
@@ -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:
diff --git a/src/jit/lsraarm64.cpp b/src/jit/lsraarm64.cpp
index 6497ac877a..6c1fc4af29 100644
--- a/src/jit/lsraarm64.cpp
+++ b/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:
diff --git a/src/jit/lsraarmarch.cpp b/src/jit/lsraarmarch.cpp
index 72acf829d1..22c96f919a 100644
--- a/src/jit/lsraarmarch.cpp
+++ b/src/jit/lsraarmarch.cpp
@@ -112,59 +112,6 @@ void LinearScan::BuildIndir(GenTreeIndir* indirTree)
 }
 
 //------------------------------------------------------------------------
-// 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.
 //
 // Arguments: