diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 83896681ef27fd..e6417673555749 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12778,6 +12778,43 @@ void Compiler::fgValueNumberTree(GenTree* tree) vnStore->VNPUnpackExc(tree->AsOp()->gtOp2->gtVNPair, &op2vnp, &op2Xvnp); ValueNumPair excSetPair = vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp); +#if defined(TARGET_XARCH) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) + // Shift instructions on these targets implicitly mask the shift amount + // to the operand width, so AND(shiftAmount, 31) or AND(shiftAmount, 63) + // is redundant. Strip the AND at VN level so that CSE sees through it + // and doesn't hoist the mask into a temp that Lowering can't remove. + if (tree->OperIs(GT_LSH, GT_RSH, GT_RSZ)) + { + size_t mask = 0x1f; +#ifdef TARGET_64BIT + if (varTypeIsLong(tree->TypeGet())) + { + mask = 0x3f; + } +#endif + GenTree* shiftAmount = tree->AsOp()->gtOp2; + while (shiftAmount->OperIs(GT_AND)) + { + GenTree* maskOp = shiftAmount->gtGetOp2(); + if (!maskOp->IsCnsIntOrI()) + { + break; + } + if ((static_cast(maskOp->AsIntCon()->IconValue()) & mask) != mask) + { + break; + } + shiftAmount = shiftAmount->gtGetOp1(); + } + + if (shiftAmount != tree->AsOp()->gtOp2) + { + vnStore->VNPUnpackExc(shiftAmount->gtVNPair, &op2vnp, &op2Xvnp); + excSetPair = vnStore->VNPExcSetUnion(op1Xvnp, op2Xvnp); + } + } +#endif + ValueNum newVN = ValueNumStore::NoVN; // Check for the addition of a field offset constant diff --git a/src/tests/JIT/opt/Shifts/ShiftMaskCSE.cs b/src/tests/JIT/opt/Shifts/ShiftMaskCSE.cs new file mode 100644 index 00000000000000..d7a8b8d1623ce2 --- /dev/null +++ b/src/tests/JIT/opt/Shifts/ShiftMaskCSE.cs @@ -0,0 +1,102 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using Xunit; + +namespace ShiftMaskCSE +{ + // Verify that the redundant AND mask on shift amounts (emitted by Roslyn) + // does not interfere with CSE. When two shifts share the same variable + // shift amount, CSE can hoist the (shift & 31) expression. By stripping + // the mask at VN level, shifts see through the redundant AND, preventing + // it from being CSE'd and avoiding the extra AND instruction at runtime. + public class ShiftMaskCSETests + { + [Fact] + public static int TestEntryPoint() + { + bool fail = false; + + // 32-bit unsigned right shift then left shift + if (ShiftAndCSE(0xDEADBEEF, 16) != 0xDEAD0000) + { + Console.WriteLine($"ShiftAndCSE failed: expected 0xDEAD0000, got 0x{ShiftAndCSE(0xDEADBEEF, 16):X}"); + fail = true; + } + + if (ShiftAndCSE(0x12345678, 0) != 0x12345678) + { + Console.WriteLine($"ShiftAndCSE with shift=0 failed"); + fail = true; + } + + if (ShiftAndCSE(0x12345678, 31) != 0) + { + Console.WriteLine($"ShiftAndCSE with shift=31 failed"); + fail = true; + } + + // 32-bit signed left shift then right shift + if (ShiftAndCSESigned(0x12345678, 8) != 0x00345678) + { + Console.WriteLine($"ShiftAndCSESigned failed: expected 0x345678, got 0x{ShiftAndCSESigned(0x12345678, 8):X}"); + fail = true; + } + + // 64-bit unsigned right shift then left shift + if (ShiftAndCSE64(0xDEADBEEFCAFEBABE, 32) != 0xDEADBEEF00000000) + { + Console.WriteLine($"ShiftAndCSE64 failed: expected 0xDEADBEEF00000000, got 0x{ShiftAndCSE64(0xDEADBEEFCAFEBABE, 32):X}"); + fail = true; + } + + // Three shifts sharing the same amount + if (TripleShift(0xFFFF0000, 8) != 0) + { + Console.WriteLine($"TripleShift failed: expected 0x0, got 0x{TripleShift(0xFFFF0000, 8):X}"); + fail = true; + } + + return fail ? -1 : 100; + } + + // Pattern from the issue: the two shifts share the same variable shift amount. + // Roslyn emits (shift & 31) for both, which CSE can hoist. + // VN sees through the redundant AND mask, so no AND instruction should be generated. + [MethodImpl(MethodImplOptions.NoInlining)] + static uint ShiftAndCSE(uint foo, int shift) + { + uint res = (foo >> shift); + res <<= shift; + return res; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int ShiftAndCSESigned(int foo, int shift) + { + int res = (foo << shift); + res >>= shift; + return res; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static ulong ShiftAndCSE64(ulong foo, int shift) + { + ulong res = (foo >> shift); + res <<= shift; + return res; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static uint TripleShift(uint foo, int shift) + { + uint a = foo >> shift; + uint b = foo << shift; + uint c = a & b; + c >>= shift; + return c; + } + } +} diff --git a/src/tests/JIT/opt/Shifts/ShiftMaskCSE.csproj b/src/tests/JIT/opt/Shifts/ShiftMaskCSE.csproj new file mode 100644 index 00000000000000..2328a56e4669da --- /dev/null +++ b/src/tests/JIT/opt/Shifts/ShiftMaskCSE.csproj @@ -0,0 +1,9 @@ + + + None + True + + + + +