Optimize better with maximum array length #69735
Conversation
The maximum array length is 0x7FFFFFC7 (see dotnet#43301), which is slightly less than the largest 32-bit signed integer. Use this value in range check and assertion prop to optimize better. This is due do knowing that a value smaller than the maximum array length plus a small constant will not overflow. This leads to being able to remove some bounds checks.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsThe maximum array length is 0x7FFFFFC7 (see
|
|
/azp run runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@AndyAyersMS PTAL |
src/coreclr/jit/jit.h
Outdated
| // See https://github.com/dotnet/runtime/pull/43301, https://msdn.microsoft.com/en-us/windows/apps/hh285054.aspx. | ||
| // CLR throws IDS_EE_ARRAY_DIMENSIONS_EXCEEDED if array length is too large. The practical limit is likely smaller | ||
| // due to memory fragmentation. | ||
| #define ARRLEN_MAX (0x7FFFFFC7) |
There was a problem hiding this comment.
A thing to watch out for with this value is that range check works on arbitrary BOUNDS_CHECK nodes, and in case of spans (and HWIs/SIMDs), the limit is int.MaxValue.
There was a problem hiding this comment.
@SingleAccretion Does this observation imply an issue with this change, or you're just pointing out to be careful with range bounds?
There was a problem hiding this comment.
Does this observation imply an issue with this change
Yes, it does.
For example, we must not remove a range check in a loop such as below:
private static int Problem(Span<int> a)
{
int sum = 0;
for (int i = 0; i < a.Length; i += 2) // 'i' will overflow for "a.Length == int.MaxValue".
{
sum += a[i];
}
return sum;
}And I see we do with this change.
There was a problem hiding this comment.
Maybe I'm missing something. When I compile your test (specifically:
using System;
using System.Runtime.CompilerServices;
namespace testns
{
class testclass
{
[MethodImpl(MethodImplOptions.NoInlining)]
private static int Problem(Span<int> a)
{
int sum = 0;
for (int i = 0; i < a.Length; i += 2) // 'i' will overflow for "a.Length == int.MaxValue".
{
sum += a[i];
}
return sum;
}
static void Main(string[] args)
{
int[] a = new int[10] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
int s = Problem(a);
Console.WriteLine(s);
}
}
}
), I see a.Length expanded as
CALL int System.Span`1[Int32][System.Int32].get_Length
whereas this PR only affects GT_ARR_LENGTH nodes, and there are none in the test, so I don't see any diff for this case.
There was a problem hiding this comment.
so I don't see any diff for this case
That is very surprising to me, there should be a diff with the removed check.
As a reference, I have checked out the change locally and obtained two dumps for Problem, one for the baseline and one for the diff: baselog.txt, difflog.txt, and I do see we remove the check in the diff, which corresponds to this change in range check's logic:
int tmp = GetArrLength(limit.vn);
if (tmp <= 0)
{
- tmp = ARRLEN_MAX;
+ tmp = CORINFO_Array_MaxLength;
}There was a problem hiding this comment.
Ok, it was operator error: using a Debug S.P.C.dll prevents cross-module inlining, and I normally use a Debug build (if at all possible). Running with Checked (and a Debug JIT) repros the problem.
Seems like VN/AssertionProp doesn't keep array and non-array GT_BOUNDS_CHECK usage distinct. Maybe I can revert this line for now and come back to it later.
|
Any further comments? |
|
@SingleAccretion Take a look at the latest update: it's only using the array max length if we've got a GT_ARR_LENGTH. This preserves most of the wins, but your test case is zero diff. |
|
@SingleAccretion Thanks for the great feedback! |
ghost
locked as resolved and limited conversation to collaborators
|
Improvements on x64: dotnet/perf-autofiling-issues#5618 |
I suspect most of those improvements are actually due to #69839. |
Oops 🙂 |
The maximum array length is 0x7FFFFFC7 (see
#43301), which is slightly less than
the largest 32-bit signed integer. Use this value in range check and assertion
prop to optimize better. This is due do knowing that a value smaller than
the maximum array length plus a small constant will not overflow. This leads
to being able to remove some bounds checks.