JIT: Strip redundant shift-count mask earlier in gtFoldExprBinary

src/coreclr/jit/compiler.h

@@ -4050,6 +4050,7 @@ class Compiler
     GenTree* gtFoldExprUnaryConstDbl(GenTreeUnOp* tree, GenTreeDblCon* dblCon);
 
     GenTree* gtFoldExprBinary(GenTreeOp* tree);
+    GenTree* gtFoldExprShiftCountMask(GenTreeOp* shift);
     GenTree* gtFoldExprBinaryConst(GenTreeOp* tree);
     GenTree* gtFoldExprBinaryConstInt(GenTreeOp* tree, GenTreeIntCon* intCon1, GenTreeIntCon* intCon2);
     GenTree* gtFoldExprBinaryConstLng(GenTreeOp* tree, GenTreeIntConCommon* intConCommon1, GenTreeIntConCommon* intConCommon2);

src/coreclr/jit/gentree.cpp

@@ -15564,6 +15564,67 @@ GenTree* Compiler::gtFoldExprUnary(GenTreeUnOp* tree)
     return tree;
 }
 
+//------------------------------------------------------------------------
+// gtFoldExprShiftCountMask: strip or normalize a redundant shift-count mask
+//
+// Arguments:
+//    shift - a GT_LSH, GT_RSH or GT_RSZ node
+//
+// Returns:
+//    The shift's op2 after any rewrite (the caller reloads its cached copy).
+//
+GenTree* Compiler::gtFoldExprShiftCountMask(GenTreeOp* shift)
+{
+    assert(shift->OperIs(GT_LSH, GT_RSH, GT_RSZ));
+
+    size_t width = varTypeIsLong(shift->TypeGet()) ? 0x3f : 0x1f;
+
+    GenTree* count = shift->gtGetOp2();
+
+    if (count->OperIs(GT_AND))
+    {
+        GenTree* andOp1 = count->gtGetOp1();
+        GenTree* andOp2 = count->gtGetOp2();
+
+        GenTree* maskCns  = nullptr;
+        GenTree* shiftCnt = nullptr;
+
+        if (andOp2->IsCnsIntOrI())
+        {
+            maskCns  = andOp2;
+            shiftCnt = andOp1;
+        }
+        else if (andOp1->IsCnsIntOrI())
+        {
+            maskCns  = andOp1;
+            shiftCnt = andOp2;
+        }
+        if ((maskCns != nullptr) && ((static_cast<size_t>(maskCns->AsIntCon()->IconValue()) & width) == width))
+        {
+            JITDUMP("Removing redundant shift-count mask [%06u]\n", count->gtTreeID);
+            count        = shiftCnt;
+            shift->gtOp2 = count;
+            // No need to call gtUpdateNodeSideEffects: AND(count, C) propagates exactly
+            // count's flags (C is a side-effect-free constant), so shift's flags are unchanged.
+        }
+    }
+
+    // Normalize out-of-range constant counts. This also handles the case where the AND
+    // above was stripped and the inner count was itself a constant (e.g. AND(cns, cns)).
+    if (count->IsCnsIntOrI())
+    {
+        size_t value = static_cast<size_t>(count->AsIntCon()->IconValue());
+
+        if ((value & width) != value)
+        {
+            count->AsIntCon()->SetIconValue(static_cast<ssize_t>(value & width));
+            fgUpdateConstTreeValueNumber(count);
+        }
+    }
+
+    return count;
+}
+
 //------------------------------------------------------------------------
 // gtFoldExprBinary: see if a binary operation is foldable
 //
@@ -15591,6 +15652,16 @@ GenTree* Compiler::gtFoldExprBinary(GenTreeOp* tree)
         return tree;
     }
 
+    if (tree->OperIs(GT_LSH, GT_RSH, GT_RSZ) && opts.OptimizationEnabled())
+    {
+        // IL masks the shift count to the operand bit width, so `x >> n` is imported as
+        // `x >> (n & 31)` (or `& 63` for 64-bit). Strip the redundant mask in optimized
+        // compilations so it disappears before CSE and later phases never have to account
+        // for it. For targets where the hardware does not mask (currently ARM32),
+        // LowerShift re-inserts the AND so that lowered code remains correct.
+        op2 = gtFoldExprShiftCountMask(tree);
+    }
+
     if (op1->OperIsConst())
     {
         if (op2->OperIsConst())

src/coreclr/jit/lower.cpp

@@ -556,11 +556,7 @@ GenTree* Lowering::LowerNode(GenTree* node)
                 return next;
             }
 
-#if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
             LowerShift(node->AsOp());
-#else
-            ContainCheckShiftRotate(node->AsOp());
-#endif
             break;
         }
 
@@ -8818,13 +8814,19 @@ bool Lowering::TryFoldBinop(GenTreeOp* node)
 //    shift - the shift node (GT_LSH, GT_RSH or GT_RSZ)
 //
 // Notes:
-//    Remove unnecessary shift count masking, xarch shift instructions
-//    mask the shift count to 5 bits (or 6 bits for 64 bit operations).
+//    On targets where hardware masks the shift count to the operand width (XARCH, ARM64,
+//    LOONGARCH64, RISCV64), nothing to do -- gtFoldExprShiftCountMask strips the redundant
+//    AND earlier.
+//
+//    On ARM32 the hardware uses Rs[7:0] without masking mod 32, so counts >= 32 give 0
+//    rather than wrapping. Insert AND(count, 31) for variable-count shifts to restore the
+//    masking semantics that were stripped in gtFoldExprShiftCountMask.
 //
 void Lowering::LowerShift(GenTreeOp* shift)
 {
     assert(shift->OperIs(GT_LSH, GT_RSH, GT_RSZ));
 
+#if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
     size_t mask = 0x1f;
 #ifdef TARGET_64BIT
     if (varTypeIsLong(shift->TypeGet()))
@@ -8852,9 +8854,44 @@ void Lowering::LowerShift(GenTreeOp* shift)
         shift->gtOp2 = andOp->gtGetOp1();
         BlockRange().Remove(andOp);
         BlockRange().Remove(maskOp);
-        // The parent was replaced, clear contain and regOpt flag.
         shift->gtOp2->ClearContained();
     }
+#elif defined(TARGET_ARM)
+    // ARM32 uses Rs[7:0] as the shift count; counts in [32, 255] give 0 (LSL/LSR) or
+    // replicated sign (ASR) rather than masking mod 32. Insert AND(count, 31) so the
+    // hardware sees a value in [0, 31]. Skip when the count is already provably in
+    // [0, 31]: a constant (handled above) or AND(x, C) where C fits in 5 bits.
+    {
+        assert(!varTypeIsLong(shift->TypeGet()));
+        constexpr ssize_t mask = 0x1f;
+
+        GenTree* shiftBy = shift->gtGetOp2();
+        if (!shiftBy->IsCnsIntOrI())
+        {
+            // Skip insertion when the count is already AND(x, C) where C <= 31,
+            // which guarantees the result is in [0, 31]. A bare GT_AND is not
+            // sufficient: user-written masks like (y & 0xFF) don't bound to [0, 31].
+            bool alreadyMasked = false;
+            if (shiftBy->OperIs(GT_AND))
+            {
+                GenTree* andOp2 = shiftBy->gtGetOp2();
+                if (andOp2->IsCnsIntOrI() && ((static_cast<size_t>(andOp2->AsIntCon()->IconValue()) & ~mask) == 0))
+                {
+                    alreadyMasked = true;
+                }
+            }
+
+            if (!alreadyMasked)
+            {
+                GenTree* maskCns = m_compiler->gtNewIconNode(mask);
+                GenTree* andNode = m_compiler->gtNewOperNode(GT_AND, TYP_INT, shiftBy, maskCns);
+                BlockRange().InsertBefore(shift, maskCns);
+                BlockRange().InsertBefore(shift, andNode);
+                shift->AsOp()->gtOp2 = andNode;
+            }
+        }
+    }
+#endif
 
     ContainCheckShiftRotate(shift);
 

src/coreclr/jit/morph.cpp

@@ -12342,9 +12342,7 @@ GenTree* Compiler::fgRecognizeAndMorphBitwiseRotation(GenTree* tree)
     //                      /      \                   / \.
     //                    LSH      RSZ      ->        x   y
     //                    / \      / \.
-    //                   x  AND   x  AND
-    //                      / \      / \.
-    //                     y  31   ADD  31
+    //                   x   y    x  ADD
     //                             / \.
     //                            NEG 32
     //                             |
@@ -12354,7 +12352,7 @@ GenTree* Compiler::fgRecognizeAndMorphBitwiseRotation(GenTree* tree)
     // (x >>> ((-y + N) & M)) op (x << (y & M))
     //
     // (x << y) op (x >>> (-y + N))
-    // (x >> > (-y + N)) op (x << y)
+    // (x >>> (-y + N)) op (x << y)
     //
     // (x >>> (y & M)) op (x << ((-y + N) & M))
     // (x << ((-y + N) & M)) op (x >>> (y & M))
@@ -12416,46 +12414,69 @@ GenTree* Compiler::fgRecognizeAndMorphBitwiseRotation(GenTree* tree)
         GenTree* rightShiftIndex = rightShiftTree->gtGetOp2();
 
         // The shift index may be masked. At least (rotatedValueBitSize - 1) lower bits
-        // shouldn't be masked for the transformation to be valid. If additional
-        // higher bits are not masked, the transformation is still valid since the result
-        // of MSIL shift instructions is unspecified if the shift amount is greater or equal
-        // than the width of the value being shifted.
+        // must be unmasked for the transformation to be valid.
         ssize_t minimalMask    = rotatedValueBitSize - 1;
         ssize_t leftShiftMask  = -1;
         ssize_t rightShiftMask = -1;
 
         if (leftShiftIndex->OperIs(GT_AND))
         {
-            if (leftShiftIndex->gtGetOp2()->IsCnsIntOrI())
+            GenTree* andOp1 = leftShiftIndex->gtGetOp1();
+            GenTree* andOp2 = leftShiftIndex->gtGetOp2();
+
+            GenTree* maskOp  = nullptr;
+            GenTree* indexOp = nullptr;
+
+            if (andOp2->IsCnsIntOrI())
             {
-                leftShiftMask  = leftShiftIndex->gtGetOp2()->AsIntCon()->IconValue();
-                leftShiftIndex = leftShiftIndex->gtGetOp1();
+                maskOp  = andOp2;
+                indexOp = andOp1;
             }
-            else
+            else if (andOp1->IsCnsIntOrI())
+            {
+                maskOp  = andOp1;
+                indexOp = andOp2;
+            }
+
+            if (maskOp == nullptr)
             {
                 return nullptr;
             }
+
+            leftShiftMask  = maskOp->AsIntCon()->IconValue();
+            leftShiftIndex = indexOp;
         }
 
         if (rightShiftIndex->OperIs(GT_AND))
         {
-            if (rightShiftIndex->gtGetOp2()->IsCnsIntOrI())
+            GenTree* andOp1 = rightShiftIndex->gtGetOp1();
+            GenTree* andOp2 = rightShiftIndex->gtGetOp2();
+
+            GenTree* maskOp  = nullptr;
+            GenTree* indexOp = nullptr;
+
+            if (andOp2->IsCnsIntOrI())
             {
-                rightShiftMask  = rightShiftIndex->gtGetOp2()->AsIntCon()->IconValue();
-                rightShiftIndex = rightShiftIndex->gtGetOp1();
+                maskOp  = andOp2;
+                indexOp = andOp1;
             }
-            else
+            else if (andOp1->IsCnsIntOrI())
+            {
+                maskOp  = andOp1;
+                indexOp = andOp2;
+            }
+
+            if (maskOp == nullptr)
             {
                 return nullptr;
             }
+
+            rightShiftMask  = maskOp->AsIntCon()->IconValue();
+            rightShiftIndex = indexOp;
         }
 
         if (((minimalMask & leftShiftMask) != minimalMask) || ((minimalMask & rightShiftMask) != minimalMask))
         {
-            // The shift index is overmasked, e.g., we have
-            // something like (x << y & 15) or
-            // (x >> (32 - y) & 15 with 32 bit x.
-            // The transformation is not valid.
             return nullptr;
         }
 
@@ -12488,12 +12509,9 @@ GenTree* Compiler::fgRecognizeAndMorphBitwiseRotation(GenTree* tree)
                         if (GenTree::Compare(shiftIndexWithAdd->gtGetOp1()->gtGetOp1(), shiftIndexWithoutAdd))
                         {
                             // We found one of these patterns:
-                            // (x << (y & M)) | (x >>> ((-y + N) & M))
                             // (x << y) | (x >>> (-y + N))
-                            // (x >>> (y & M)) | (x << ((-y + N) & M))
                             // (x >>> y) | (x << (-y + N))
-                            // where N == bitsize(x), M is const, and
-                            // M & (N - 1) == N - 1
+                            // where N == bitsize(x)
 
 #ifndef TARGET_64BIT
                             if (!shiftIndexWithoutAdd->IsCnsIntOrI() && (rotatedValueBitSize == 64))

src/coreclr/jit/valuenum.cpp

@@ -733,7 +733,7 @@ T ValueNumStore::EvalOpSpecialized(VNFunc vnf, T v0, T v1)
                 }
                 else
                 {
-                    return v0 << v1;
+                    return v0 << (v1 & 0x1F);
                 }
             case GT_RSH:
                 if (sizeof(T) == 8)
@@ -742,7 +742,7 @@ T ValueNumStore::EvalOpSpecialized(VNFunc vnf, T v0, T v1)
                 }
                 else
                 {
-                    return v0 >> v1;
+                    return v0 >> (v1 & 0x1F);
                 }
             case GT_RSZ:
                 if (sizeof(T) == 8)
@@ -751,7 +751,7 @@ T ValueNumStore::EvalOpSpecialized(VNFunc vnf, T v0, T v1)
                 }
                 else
                 {
-                    return UINT32(v0) >> v1;
+                    return UINT32(v0) >> (v1 & 0x1F);
                 }
             case GT_ROL:
                 if (sizeof(T) == 8)
@@ -13695,6 +13695,7 @@ void Compiler::fgValueNumberTree(GenTree* tree)
                     ValueNumPair op2vnp;
                     ValueNumPair op2Xvnp;
                     vnStore->VNPUnpackExc(tree->AsOp()->gtOp2->gtVNPair, &op2vnp, &op2Xvnp);
+
                     ValueNumPair excSetPair = vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp);
 
                     ValueNum newVN = ValueNumStore::NoVN;