Implement Unsafe.IsAddressLessThanOrEqualTo & Unsafe.IsAddressGreaterThanOrEqualTo (#117800)

* Implement Unsafe. IsAddressLessThanOrEqualTo & Unsafe.IsAddressGreaterThanOrEqualTo

* Update vectorization-guidelines.md

Co-authored-by: Jan Kotas <[email protected]>

Author: hamarb123

docs/coding-guidelines/vectorization-guidelines.md

@@ -523,7 +523,7 @@ int ManagedReferencesSum(int[] buffer)
 
     Vector128<int> sum = Vector128<int>.Zero;
 
-    while (!Unsafe.IsAddressGreaterThan(ref current, ref oneVectorAwayFromEnd))
+    while (Unsafe.IsAddressLessThanOrEqualTo(ref current, ref oneVectorAwayFromEnd))
     {
         sum += Vector128.LoadUnsafe(ref current);
 
@@ -561,7 +561,7 @@ do
 
     return ...;
 }
-while (!Unsafe.IsAddressLessThan(ref currentSearchSpace, ref searchSpace));
+while (Unsafe.IsAddressGreaterThanOrEqualTo(ref currentSearchSpace, ref searchSpace));
 ```
 
 It was part of `LastIndexOf` implementation, where we were iterating from the end to the beginning of the buffer. In the last iteration of the loop, `currentSearchSpace` could become a pointer to unknown memory that lied before the beginning of the buffer:
@@ -573,7 +573,7 @@ currentSearchSpace = ref Unsafe.Subtract(ref currentSearchSpace, Vector128<TValu
 And it was fine until GC kicked right after that, moved objects in memory, updated all valid managed references and resumed the execution, which run following condition:
 
 ```csharp
-while (!Unsafe.IsAddressLessThan(ref currentSearchSpace, ref searchSpace));
+while (Unsafe.IsAddressGreaterThanOrEqualTo(ref currentSearchSpace, ref searchSpace));
 ```
 
 Which could return true because `currentSearchSpace` was invalid and not updated. If you are interested in more details, you can check the [issue](https://github.com/dotnet/runtime/issues/75792#issuecomment-1249973858) and the [fix](https://github.com/dotnet/runtime/pull/75857).

src/coreclr/jit/fgbasic.cpp

@@ -1245,7 +1245,9 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                             case NI_SRCS_UNSAFE_AreSame:
                             case NI_SRCS_UNSAFE_ByteOffset:
                             case NI_SRCS_UNSAFE_IsAddressGreaterThan:
+                            case NI_SRCS_UNSAFE_IsAddressGreaterThanOrEqualTo:
                             case NI_SRCS_UNSAFE_IsAddressLessThan:
+                            case NI_SRCS_UNSAFE_IsAddressLessThanOrEqualTo:
                             case NI_SRCS_UNSAFE_IsNullRef:
                             case NI_SRCS_UNSAFE_Subtract:
                             case NI_SRCS_UNSAFE_SubtractByteOffset:
@@ -1260,7 +1262,9 @@ void Compiler::fgFindJumpTargets(const BYTE* codeAddr, IL_OFFSET codeSize, Fixed
                                     {
                                         case NI_SRCS_UNSAFE_AreSame:
                                         case NI_SRCS_UNSAFE_IsAddressGreaterThan:
+                                        case NI_SRCS_UNSAFE_IsAddressGreaterThanOrEqualTo:
                                         case NI_SRCS_UNSAFE_IsAddressLessThan:
+                                        case NI_SRCS_UNSAFE_IsAddressLessThanOrEqualTo:
                                         case NI_SRCS_UNSAFE_IsNullRef:
                                         {
                                             fgObserveInlineConstants(opcode, pushedStack, isInlining);

src/coreclr/jit/importercalls.cpp

@@ -5503,6 +5503,25 @@ GenTree* Compiler::impSRCSUnsafeIntrinsic(NamedIntrinsic          intrinsic,
             return gtFoldExpr(tmp);
         }
 
+        case NI_SRCS_UNSAFE_IsAddressGreaterThanOrEqualTo:
+        {
+            assert(sig->sigInst.methInstCount == 1);
+
+            // ldarg.0
+            // ldarg.1
+            // clt.un
+            // ldc.i4.0
+            // ceq
+            // ret
+
+            GenTree* op2 = impPopStack().val;
+            GenTree* op1 = impPopStack().val;
+
+            GenTree* tmp = gtNewOperNode(GT_GE, TYP_INT, op1, op2);
+            tmp->gtFlags |= GTF_UNSIGNED;
+            return gtFoldExpr(tmp);
+        }
+
         case NI_SRCS_UNSAFE_IsAddressLessThan:
         {
             assert(sig->sigInst.methInstCount == 1);
@@ -5520,6 +5539,25 @@ GenTree* Compiler::impSRCSUnsafeIntrinsic(NamedIntrinsic          intrinsic,
             return gtFoldExpr(tmp);
         }
 
+        case NI_SRCS_UNSAFE_IsAddressLessThanOrEqualTo:
+        {
+            assert(sig->sigInst.methInstCount == 1);
+
+            // ldarg.0
+            // ldarg.1
+            // cgt.un
+            // ldc.i4.0
+            // ceq
+            // ret
+
+            GenTree* op2 = impPopStack().val;
+            GenTree* op1 = impPopStack().val;
+
+            GenTree* tmp = gtNewOperNode(GT_LE, TYP_INT, op1, op2);
+            tmp->gtFlags |= GTF_UNSIGNED;
+            return gtFoldExpr(tmp);
+        }
+
         case NI_SRCS_UNSAFE_IsNullRef:
         {
             assert(sig->sigInst.methInstCount == 1);
@@ -10722,10 +10760,18 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
                             {
                                 result = NI_SRCS_UNSAFE_IsAddressGreaterThan;
                             }
+                            else if (strcmp(methodName, "IsAddressGreaterThanOrEqualTo") == 0)
+                            {
+                                result = NI_SRCS_UNSAFE_IsAddressGreaterThanOrEqualTo;
+                            }
                             else if (strcmp(methodName, "IsAddressLessThan") == 0)
                             {
                                 result = NI_SRCS_UNSAFE_IsAddressLessThan;
                             }
+                            else if (strcmp(methodName, "IsAddressLessThanOrEqualTo") == 0)
+                            {
+                                result = NI_SRCS_UNSAFE_IsAddressLessThanOrEqualTo;
+                            }
                             else if (strcmp(methodName, "IsNullRef") == 0)
                             {
                                 result = NI_SRCS_UNSAFE_IsNullRef;

src/coreclr/jit/namedintrinsiclist.h

@@ -221,7 +221,9 @@ enum NamedIntrinsic : unsigned short
     NI_SRCS_UNSAFE_InitBlock,
     NI_SRCS_UNSAFE_InitBlockUnaligned,
     NI_SRCS_UNSAFE_IsAddressGreaterThan,
+    NI_SRCS_UNSAFE_IsAddressGreaterThanOrEqualTo,
     NI_SRCS_UNSAFE_IsAddressLessThan,
+    NI_SRCS_UNSAFE_IsAddressLessThanOrEqualTo,
     NI_SRCS_UNSAFE_IsNullRef,
     NI_SRCS_UNSAFE_NullRef,
     NI_SRCS_UNSAFE_Read,

src/libraries/System.Linq/src/System/Linq/Range.cs

@@ -97,7 +97,7 @@ private static void FillIncrementing<T>(Span<T> destination, T value) where T :
                     current += increment;
                     pos = ref Unsafe.Add(ref pos, Vector<T>.Count);
                 }
-                while (!Unsafe.IsAddressGreaterThan(ref pos, ref oneVectorFromEnd));
+                while (Unsafe.IsAddressLessThanOrEqualTo(ref pos, ref oneVectorFromEnd));
 
                 value = current[0];
             }

src/libraries/System.Private.CoreLib/src/System/Collections/Generic/ArraySortHelper.cs

@@ -487,7 +487,7 @@ private static unsafe int PickPivotAndPartition(Span<T> keys)
                     while (Unsafe.IsAddressGreaterThan(ref rightRef, ref zeroRef) && LessThan(ref pivot, ref rightRef = ref Unsafe.Add(ref rightRef, -1))) ;
                 }
 
-                if (!Unsafe.IsAddressLessThan(ref leftRef, ref rightRef))
+                if (Unsafe.IsAddressGreaterThanOrEqualTo(ref leftRef, ref rightRef))
                 {
                     break;
                 }

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/Unsafe.cs

@@ -404,6 +404,22 @@ public static bool IsAddressGreaterThan<T>([AllowNull] ref readonly T left, [All
             // ret
         }
 
+        /// <summary>
+        /// Determines whether the memory address referenced by <paramref name="left"/> is greater than
+        /// or equal to the memory address referenced by <paramref name="right"/>.
+        /// </summary>
+        /// <remarks>
+        /// This check is conceptually similar to "(void*)(&amp;left) &gt;= (void*)(&amp;right)".
+        /// </remarks>
+        [Intrinsic]
+        [NonVersionable]
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static bool IsAddressGreaterThanOrEqualTo<T>([AllowNull] ref readonly T left, [AllowNull] ref readonly T right)
+            where T : allows ref struct
+        {
+            return !IsAddressLessThan(in left, in right);
+        }
+
         /// <summary>
         /// Determines whether the memory address referenced by <paramref name="left"/> is less than
         /// the memory address referenced by <paramref name="right"/>.
@@ -428,6 +444,22 @@ public static bool IsAddressLessThan<T>([AllowNull] ref readonly T left, [AllowN
             // ret
         }
 
+        /// <summary>
+        /// Determines whether the memory address referenced by <paramref name="left"/> is less than
+        /// or equal to the memory address referenced by <paramref name="right"/>.
+        /// </summary>
+        /// <remarks>
+        /// This check is conceptually similar to "(void*)(&amp;left) &lt;= (void*)(&amp;right)".
+        /// </remarks>
+        [Intrinsic]
+        [NonVersionable]
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static bool IsAddressLessThanOrEqualTo<T>([AllowNull] ref readonly T left, [AllowNull] ref readonly T right)
+            where T : allows ref struct
+        {
+            return !IsAddressGreaterThan(in left, in right);
+        }
+
         /// <summary>
         /// Initializes a block of memory at the given location with a given initial value.
         /// </summary>

src/libraries/System.Private.CoreLib/src/System/SearchValues/IndexOfAnyAsciiSearcher.cs

@@ -321,7 +321,7 @@ private static TResult IndexOfAnyCore<TResult, TNegator, TOptimizations, TUnique
                     // As packing two Vector256<short>s into a Vector256<byte> is cheap compared to the lookup, we can effectively double the throughput.
                     // If the input length is a multiple of 32, don't consume the last 32 characters in this loop.
                     // Let the fallback below handle it instead. This is why the condition is
-                    // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "!IsAddressGreaterThan".
+                    // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "IsAddressLessThanOrEqualTo".
                     ref short twoVectorsAwayFromEnd = ref Unsafe.Add(ref searchSpace, searchSpaceLength - (2 * Vector256<short>.Count));
 
                     do
@@ -374,7 +374,7 @@ private static TResult IndexOfAnyCore<TResult, TNegator, TOptimizations, TUnique
                 // As packing two Vector128<short>s into a Vector128<byte> is cheap compared to the lookup, we can effectively double the throughput.
                 // If the input length is a multiple of 16, don't consume the last 16 characters in this loop.
                 // Let the fallback below handle it instead. This is why the condition is
-                // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "!IsAddressGreaterThan".
+                // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "IsAddressLessThanOrEqualTo".
                 ref short twoVectorsAwayFromEnd = ref Unsafe.Add(ref searchSpace, searchSpaceLength - (2 * Vector128<short>.Count));
 
                 do
@@ -453,7 +453,7 @@ public static int LastIndexOfAny<TNegator, TOptimizations, TUniqueLowNibble>(ref
                     // As packing two Vector256<short>s into a Vector256<byte> is cheap compared to the lookup, we can effectively double the throughput.
                     // If the input length is a multiple of 32, don't consume the last 32 characters in this loop.
                     // Let the fallback below handle it instead. This is why the condition is
-                    // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "!IsAddressLessThan".
+                    // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "IsAddressGreaterThanOrEqualTo".
                     ref short twoVectorsAfterStart = ref Unsafe.Add(ref searchSpace, 2 * Vector256<short>.Count);
 
                     do
@@ -504,7 +504,7 @@ public static int LastIndexOfAny<TNegator, TOptimizations, TUniqueLowNibble>(ref
                 // As packing two Vector128<short>s into a Vector128<byte> is cheap compared to the lookup, we can effectively double the throughput.
                 // If the input length is a multiple of 16, don't consume the last 16 characters in this loop.
                 // Let the fallback below handle it instead. This is why the condition is
-                // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "!IsAddressLessThan".
+                // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "IsAddressGreaterThanOrEqualTo".
                 ref short twoVectorsAfterStart = ref Unsafe.Add(ref searchSpace, 2 * Vector128<short>.Count);
 
                 do
@@ -604,7 +604,7 @@ private static TResult IndexOfAnyCore<TResult, TNegator, TUniqueLowNibble, TResu
                     // Process the input in chunks of 32 bytes.
                     // If the input length is a multiple of 32, don't consume the last 32 characters in this loop.
                     // Let the fallback below handle it instead. This is why the condition is
-                    // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "!IsAddressGreaterThan".
+                    // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "IsAddressLessThanOrEqualTo".
                     ref byte vectorAwayFromEnd = ref Unsafe.Add(ref searchSpace, searchSpaceLength - Vector256<byte>.Count);
 
                     do
@@ -653,7 +653,7 @@ private static TResult IndexOfAnyCore<TResult, TNegator, TUniqueLowNibble, TResu
                 // Process the input in chunks of 16 bytes.
                 // If the input length is a multiple of 16, don't consume the last 16 characters in this loop.
                 // Let the fallback below handle it instead. This is why the condition is
-                // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "!IsAddressGreaterThan".
+                // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "IsAddressLessThanOrEqualTo".
                 ref byte vectorAwayFromEnd = ref Unsafe.Add(ref searchSpace, searchSpaceLength - Vector128<byte>.Count);
 
                 do
@@ -729,7 +729,7 @@ public static int LastIndexOfAny<TNegator, TUniqueLowNibble>(ref byte searchSpac
                     // Process the input in chunks of 32 bytes.
                     // If the input length is a multiple of 32, don't consume the last 32 characters in this loop.
                     // Let the fallback below handle it instead. This is why the condition is
-                    // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "!IsAddressLessThan".
+                    // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "IsAddressGreaterThanOrEqualTo".
                     ref byte vectorAfterStart = ref Unsafe.Add(ref searchSpace, Vector256<byte>.Count);
 
                     do
@@ -778,7 +778,7 @@ public static int LastIndexOfAny<TNegator, TUniqueLowNibble>(ref byte searchSpac
                 // Process the input in chunks of 16 bytes.
                 // If the input length is a multiple of 16, don't consume the last 16 characters in this loop.
                 // Let the fallback below handle it instead. This is why the condition is
-                // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "!IsAddressLessThan".
+                // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "IsAddressGreaterThanOrEqualTo".
                 ref byte vectorAfterStart = ref Unsafe.Add(ref searchSpace, Vector128<byte>.Count);
 
                 do
@@ -876,7 +876,7 @@ private static TResult IndexOfAnyCore<TResult, TNegator, TResultMapper>(ref byte
                     // Process the input in chunks of 32 bytes.
                     // If the input length is a multiple of 32, don't consume the last 32 characters in this loop.
                     // Let the fallback below handle it instead. This is why the condition is
-                    // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "!IsAddressGreaterThan".
+                    // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "IsAddressLessThanOrEqualTo".
                     ref byte vectorAwayFromEnd = ref Unsafe.Add(ref searchSpace, searchSpaceLength - Vector256<byte>.Count);
 
                     do
@@ -928,7 +928,7 @@ private static TResult IndexOfAnyCore<TResult, TNegator, TResultMapper>(ref byte
                 // Process the input in chunks of 16 bytes.
                 // If the input length is a multiple of 16, don't consume the last 16 characters in this loop.
                 // Let the fallback below handle it instead. This is why the condition is
-                // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "!IsAddressGreaterThan".
+                // ">" instead of ">=" above, and why "IsAddressLessThan" is used instead of "IsAddressLessThanOrEqualTo".
                 ref byte vectorAwayFromEnd = ref Unsafe.Add(ref searchSpace, searchSpaceLength - Vector128<byte>.Count);
 
                 do
@@ -1004,7 +1004,7 @@ public static int LastIndexOfAny<TNegator>(ref byte searchSpace, int searchSpace
                     // Process the input in chunks of 32 bytes.
                     // If the input length is a multiple of 32, don't consume the last 32 characters in this loop.
                     // Let the fallback below handle it instead. This is why the condition is
-                    // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "!IsAddressLessThan".
+                    // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "IsAddressGreaterThanOrEqualTo".
                     ref byte vectorAfterStart = ref Unsafe.Add(ref searchSpace, Vector256<byte>.Count);
 
                     do
@@ -1056,7 +1056,7 @@ public static int LastIndexOfAny<TNegator>(ref byte searchSpace, int searchSpace
                 // Process the input in chunks of 16 bytes.
                 // If the input length is a multiple of 16, don't consume the last 16 characters in this loop.
                 // Let the fallback below handle it instead. This is why the condition is
-                // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "!IsAddressLessThan".
+                // ">" instead of ">=" above, and why "IsAddressGreaterThan" is used instead of "IsAddressGreaterThanOrEqualTo".
                 ref byte vectorAfterStart = ref Unsafe.Add(ref searchSpace, Vector128<byte>.Count);
 
                 do

src/libraries/System.Private.CoreLib/src/System/SearchValues/ProbabilisticMap.cs

@@ -626,7 +626,7 @@ private static int LastIndexOfAnyVectorizedAvx512<TUseFastContains>(ref char sea
                             }
                         }
 
-                        if (!Unsafe.IsAddressGreaterThan(ref cur, ref lastStartVector))
+                        if (Unsafe.IsAddressLessThanOrEqualTo(ref cur, ref lastStartVector))
                         {
                             if (Unsafe.AreSame(ref cur, ref searchSpace))
                             {
@@ -715,7 +715,7 @@ private static int LastIndexOfAnyVectorized<TUseFastContains>(ref char searchSpa
                         }
                     }
 
-                    if (!Unsafe.IsAddressGreaterThan(ref cur, ref lastStartVectorAvx2))
+                    if (Unsafe.IsAddressLessThanOrEqualTo(ref cur, ref lastStartVectorAvx2))
                     {
                         if (Unsafe.AreSame(ref cur, ref searchSpace))
                         {
@@ -757,7 +757,7 @@ private static int LastIndexOfAnyVectorized<TUseFastContains>(ref char searchSpa
                     }
                 }
 
-                if (!Unsafe.IsAddressGreaterThan(ref cur, ref lastStartVector))
+                if (Unsafe.IsAddressLessThanOrEqualTo(ref cur, ref lastStartVector))
                 {
                     if (Unsafe.AreSame(ref cur, ref searchSpace))
                     {

src/libraries/System.Private.CoreLib/src/System/SearchValues/Strings/Helpers/RabinKarp.cs

@@ -142,7 +142,7 @@ private readonly int IndexOfAnyCore<TCaseSensitivity>(ReadOnlySpan<char> span)
                         }
                     }
 
-                    if (!Unsafe.IsAddressLessThan(ref current, ref end))
+                    if (Unsafe.IsAddressGreaterThanOrEqualTo(ref current, ref end))
                     {
                         break;
                     }