Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/coding-guidelines/vectorization-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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:
Expand All @@ -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).
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/fgbasic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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);
Expand Down
46 changes: 46 additions & 0 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5500,6 +5500,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);
Expand All @@ -5517,6 +5536,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);
Expand Down Expand Up @@ -10720,10 +10758,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;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/namedintrinsiclist.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq/src/System/Linq/Range.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>.
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Loading
Loading