Skip to content

Commit

Permalink
Eliminate bound checks for "arr[index % arr.Length]" (#84231)
Browse files Browse the repository at this point in the history
  • Loading branch information
EgorBo authored Apr 15, 2023
1 parent 49af07c commit d0de955
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 8 deletions.
14 changes: 11 additions & 3 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ bool IntegralRange::Contains(int64_t value) const
ForNode(node->AsQmark()->ElseNode(), compiler));

case GT_CAST:
return ForCastOutput(node->AsCast());
return ForCastOutput(node->AsCast(), compiler);

#if defined(FEATURE_HW_INTRINSICS)
case GT_HWINTRINSIC:
Expand Down Expand Up @@ -369,12 +369,13 @@ bool IntegralRange::Contains(int64_t value) const
// Unlike ForCastInput, this method supports casts from floating point types.
//
// Arguments:
// cast - the cast node for which the range will be computed
// cast - the cast node for which the range will be computed
// compiler - Compiler object
//
// Return Value:
// The range this cast produces - see description.
//
/* static */ IntegralRange IntegralRange::ForCastOutput(GenTreeCast* cast)
/* static */ IntegralRange IntegralRange::ForCastOutput(GenTreeCast* cast, Compiler* compiler)
{
var_types fromType = genActualType(cast->CastOp());
var_types toType = cast->CastToType();
Expand Down Expand Up @@ -407,6 +408,13 @@ bool IntegralRange::Contains(int64_t value) const
return ForCastInput(cast);
}

// if we're upcasting and the cast op is a known non-negative - consider
// this cast unsigned
if (!fromUnsigned && (genTypeSize(toType) >= genTypeSize(fromType)))
{
fromUnsigned = cast->CastOp()->IsNeverNegative(compiler);
}

// CAST(uint/int <- ulong/long) - [INT_MIN..INT_MAX]
// CAST(ulong/long <- uint) - [0..UINT_MAX]
// CAST(ulong/long <- int) - [INT_MIN..INT_MAX]
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,7 @@ class IntegralRange

static IntegralRange ForNode(GenTree* node, Compiler* compiler);
static IntegralRange ForCastInput(GenTreeCast* cast);
static IntegralRange ForCastOutput(GenTreeCast* cast);
static IntegralRange ForCastOutput(GenTreeCast* cast, Compiler* compiler);
static IntegralRange Union(IntegralRange range1, IntegralRange range2);

#ifdef DEBUG
Expand Down
15 changes: 15 additions & 0 deletions src/coreclr/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,21 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
}
}

if (m_pCompiler->vnStore->GetVNFunc(idxVn, &funcApp) && (funcApp.m_func == (VNFunc)GT_UMOD))
{
// We can always omit bound checks for Arr[X u% Arr.Length] pattern (unsigned MOD).
//
// if arr.Length is 0 we technically should keep the bounds check, but since the expression
// has to throw DividedByZeroException anyway - no special handling needed.
if (funcApp.m_args[1] == arrLenVn)
{
JITDUMP("[RangeCheck::OptimizeRangeCheck] UMOD(X, ARR_LEN) is always between bounds\n");
m_pCompiler->optRemoveRangeCheck(bndsChk, comma, stmt);
m_updateStmt = true;
return;
}
}

// Get the range for this index.
Range range = GetRange(block, treeIndex, false DEBUGARG(0));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private bool Find(TKey key, bool add, bool set, [MaybeNullWhen(false)] ref TValu
{
int hashCode = GetKeyHashCode(key);

for (int i = buckets[hashCode % buckets.Length] - 1; i >= 0; i = slots[i].next)
for (int i = buckets[(uint)hashCode % buckets.Length] - 1; i >= 0; i = slots[i].next)
{
if (slots[i].hashCode == hashCode && AreKeysEqual(slots[i].key, key))
{
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq/src/System/Linq/Lookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ private int InternalGetHashCode(TKey key)
internal Grouping<TKey, TElement>? GetGrouping(TKey key, bool create)
{
int hashCode = InternalGetHashCode(key);
for (Grouping<TKey, TElement>? g = _groupings[hashCode % _groupings.Length]; g != null; g = g._hashNext)
for (Grouping<TKey, TElement>? g = _groupings[(uint)hashCode % _groupings.Length]; g != null; g = g._hashNext)
{
if (g._hashCode == hashCode && _comparer.Equals(g._key, key))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1322,7 +1322,7 @@ private ref int GetBucket(uint hashCode)
#if TARGET_64BIT
return ref buckets[HashHelpers.FastMod(hashCode, (uint)buckets.Length, _fastModMultiplier)];
#else
return ref buckets[hashCode % (uint)buckets.Length];
return ref buckets[(uint)hashCode % buckets.Length];
#endif
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ internal TimerQueueTimer(TimerCallback timerCallback, object? state, uint dueTim
{
_executionContext = ExecutionContext.Capture();
}
_associatedTimerQueue = TimerQueue.Instances[Thread.GetCurrentProcessorId() % TimerQueue.Instances.Length];
_associatedTimerQueue = TimerQueue.Instances[(uint)Thread.GetCurrentProcessorId() % TimerQueue.Instances.Length];

// After the following statement, the timer may fire. No more manipulation of timer state outside of
// the lock is permitted beyond this point!
Expand Down
99 changes: 99 additions & 0 deletions src/tests/JIT/opt/RangeChecks/ModLength.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// 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;

public class ModLength
{
public static int Main()
{
Throws<DivideByZeroException>(() => Test1(new int[0], 0));
Throws<DivideByZeroException>(() => Test2(new int[0], 1));
Throws<DivideByZeroException>(() => Test3(new int[0], int.MaxValue));
Throws<DivideByZeroException>(() => Test4(new int[0], 0));
Throws<DivideByZeroException>(() => Test5(new int[0], 0));
Throws<DivideByZeroException>(() => Test6(new int[0], 0));
Throws<DivideByZeroException>(() => Test7(new int[0], 0));
Throws<DivideByZeroException>(() => Test8(new int[0], 0));
Throws<DivideByZeroException>(() => Test9(new int[0], 0));
Test1(new int[1], 1);
Test2(new int[1], 1);
Throws<IndexOutOfRangeException>(() => Test9(new int[1], 2));
Test3(new int[1], int.MaxValue);
Throws<IndexOutOfRangeException>(() => Test4(new int[1], int.MinValue));
Test5(new int[1], -1);
Test6(new int[1], 1);
Test7(new int[1], 1);
Test8(new int[1], 1);
Test1(new int[10], 10);
Test2(new int[10], 11);
Test3(new int[10], int.MaxValue);
Throws<IndexOutOfRangeException>(() => Test4(new int[10], int.MinValue));
Throws<IndexOutOfRangeException>(() => Test5(new int[10], -1));
Test6(new int[10], 0);
Test7(new int[10], 0);
Throws<DivideByZeroException>(() => Test8(new int[10], 0));
return 100;
}

static void Throws<T>(Action action, [CallerLineNumber] int line = 0)
{
try
{
action();
}
catch (Exception e)
{
if (e is T)
{
return;
}
throw new InvalidOperationException($"{typeof(T)} exception was expected, actual: {e.GetType()}");
}
throw new InvalidOperationException($"{typeof(T)} exception was expected");
}

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test1(int[] arr, int index) => arr[index % arr.Length];

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test2(int[] arr, int index) => arr[(int)index % (int)arr.Length];

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test3(int[] arr, int index) => arr[(int)((uint)index % (uint)arr.Length)];

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test4(int[] arr, int index) => arr[arr.Length % index];

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test5(int[] arr, int index)
{
var span = arr.AsSpan();
return arr[index % arr.Length];
}

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test6(int[] arr, int index)
{
var span = arr.AsSpan();
return span[(int)index % (int)span.Length];
}

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test7(int[] arr, int index)
{
var span = arr.AsSpan();
return span[(int)((uint)index % (uint)span.Length)];
}

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test8(int[] arr, int index)
{
var span = arr.AsSpan();
return span[span.Length % index];
}

[MethodImpl(MethodImplOptions.NoInlining)]
static int Test9(int[] arr, int index) => arr[index / arr.Length];
}
9 changes: 9 additions & 0 deletions src/tests/JIT/opt/RangeChecks/ModLength.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit d0de955

Please sign in to comment.