Skip to content

Commit 910fb04

Browse files
EgorBoEgorBo
authored
Fold "X >= 0 && X < NN" to "X u< NN" (#93834)
* Fold "X >= 0 && X < NN" to "X u< NN" * Relax to GTF_SIDE_EFFECT
1 parent aae9497 commit 910fb04

File tree

3 files changed

+185
-3
lines changed

3 files changed

+185
-3
lines changed

‎src/coreclr/jit/optimizebools.cpp

+92-3
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,96 @@ bool IsConstantRangeTest(GenTreeOp* tree, GenTree** varNode, GenTreeIntCon** cns
532532
return false;
533533
}
534534

535+
//------------------------------------------------------------------------------
536+
// FoldRangeTests: Given two compare nodes (cmp1 && cmp2) where cmp1 is X >= 0
537+
// and cmp2 is X < NN (NeverNegative), try to fold the range test into a single
538+
// X u< NN (unsigned) compare node.
539+
//
540+
// Arguments:
541+
// compiler - compiler instance
542+
// cmp1 - first compare node
543+
// cmp1IsReversed - true if cmp1 is in fact reversed
544+
// cmp2 - second compare node
545+
// cmp2IsReversed - true if cmp2 is in fact reversed
546+
//
547+
// Returns:
548+
// true if cmp1 now represents the folded range check and cmp2 can be removed.
549+
//
550+
bool FoldNeverNegativeRangeTest(
551+
Compiler* comp, GenTreeOp* cmp1, bool cmp1IsReversed, GenTreeOp* cmp2, bool cmp2IsReversed)
552+
{
553+
GenTree* var1Node;
554+
GenTreeIntCon* cns1Node;
555+
genTreeOps cmp1Op;
556+
557+
// First cmp has to be "X >= 0" (or "0 <= X")
558+
// TODO: handle "X < NN && X >= 0" (where the 2nd comparison is the lower bound)
559+
// It seems to be a rare case, so we don't handle it for now.
560+
if (!IsConstantRangeTest(cmp1, &var1Node, &cns1Node, &cmp1Op))
561+
{
562+
return false;
563+
}
564+
565+
// Now, reverse the comparison if necessary depending on cmp1IsReversed and cmp2IsReversed
566+
// so we'll get a canonical form of "X >= 0 && X </<= NN"
567+
cmp1Op = cmp1IsReversed ? GenTree::ReverseRelop(cmp1Op) : cmp1Op;
568+
genTreeOps cmp2Op = cmp2IsReversed ? GenTree::ReverseRelop(cmp2->OperGet()) : cmp2->OperGet();
569+
570+
if ((cmp1Op != GT_GE) || (!cns1Node->IsIntegralConst(0)))
571+
{
572+
// Lower bound check has to be "X >= 0".
573+
// We already re-ordered the comparison so that the constant is always on the right side.
574+
return false;
575+
}
576+
577+
// Upper bound check has to be "X relop NN" or "NN relop X" (NN = NeverNegative)
578+
// We allow var1Node to be a GT_COMMA node, so we need to call gtEffectiveVal() to get the actual variable
579+
// since it's guaranteed to be evaluated first.
580+
GenTree* upperBound;
581+
if (cmp2->gtGetOp1()->OperIs(GT_LCL_VAR, GT_LCL_FLD) &&
582+
GenTree::Compare(var1Node->gtEffectiveVal(), cmp2->gtGetOp1()))
583+
{
584+
// "X relop NN"
585+
upperBound = cmp2->gtGetOp2();
586+
}
587+
else if (cmp2->gtGetOp2()->OperIs(GT_LCL_VAR, GT_LCL_FLD) &&
588+
GenTree::Compare(var1Node->gtEffectiveVal(), cmp2->gtGetOp2()))
589+
{
590+
// "NN relop X"
591+
upperBound = cmp2->gtGetOp1();
592+
// Normalize to "X relop NN"
593+
cmp2Op = GenTree::SwapRelop(cmp2Op);
594+
}
595+
else
596+
{
597+
return false;
598+
}
599+
600+
// Check that our upper bound is known to be never negative (e.g. GT_ARR_LENGTH or Span.Length, etc.)
601+
if (!upperBound->IsNeverNegative(comp) || !upperBound->TypeIs(var1Node->TypeGet()))
602+
{
603+
return false;
604+
}
605+
606+
if ((upperBound->gtFlags & GTF_SIDE_EFFECT) != 0)
607+
{
608+
// We can't fold "X >= 0 && X < NN" to "X u< NN" if NN has side effects.
609+
return false;
610+
}
611+
612+
if ((cmp2Op != GT_LT) && (cmp2Op != GT_LE))
613+
{
614+
// Upper bound check has to be "X < NN" or "X <= NN" (normalized form).
615+
return false;
616+
}
617+
618+
cmp1->gtOp1 = var1Node;
619+
cmp1->gtOp2 = upperBound;
620+
cmp1->SetOper(cmp2IsReversed ? GenTree::ReverseRelop(cmp2Op) : cmp2Op);
621+
cmp1->SetUnsigned();
622+
return true;
623+
}
624+
535625
//------------------------------------------------------------------------------
536626
// FoldRangeTests: Given two compare nodes (cmp1 && cmp2) that represent a range check,
537627
// fold them into a single compare node if possible, e.g.:
@@ -560,12 +650,11 @@ bool FoldRangeTests(Compiler* comp, GenTreeOp* cmp1, bool cmp1IsReversed, GenTre
560650
genTreeOps cmp2Op;
561651

562652
// Make sure both conditions are constant range checks, e.g. "X > CNS"
563-
// TODO: support more cases, e.g. "X >= 0 && X < array.Length" -> "(uint)X < array.Length"
564-
// Basically, we can use GenTree::IsNeverNegative() for it.
565653
if (!IsConstantRangeTest(cmp1, &var1Node, &cns1Node, &cmp1Op) ||
566654
!IsConstantRangeTest(cmp2, &var2Node, &cns2Node, &cmp2Op))
567655
{
568-
return false;
656+
// Give FoldNeverNegativeRangeTest a try if both conditions are not constant range checks.
657+
return FoldNeverNegativeRangeTest(comp, cmp1, cmp1IsReversed, cmp2, cmp2IsReversed);
569658
}
570659

571660
// Reverse the comparisons if necessary so we'll get a canonical form "cond1 == true && cond2 == true" -> InRange.

‎src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned2.cs

+85
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Linq;
6+
using System.Runtime.CompilerServices;
7+
using Xunit;
8+
9+
public class RedundantBranchUnsigned2
10+
{
11+
[Fact]
12+
public static int TestEntryPoint()
13+
{
14+
int[] arr1 = new int[2];
15+
Test_Span1(arr1, -1);
16+
Test_Span1(arr1, 0);
17+
Test_Span1(arr1, 1);
18+
Test_Span1(arr1, 2);
19+
if (!arr1.SequenceEqual(new[] {42, 42}))
20+
throw new Exception("Test_Span1 failed");
21+
22+
int[] arr2 = new int[2];
23+
Test_Span2(arr2, -1);
24+
Test_Span2(arr2, 0);
25+
Test_Span2(arr2, 1);
26+
Test_Span2(arr2, 2);
27+
if (!arr2.SequenceEqual(new[] { 0, 42 }))
28+
throw new Exception("Test_Span1 failed");
29+
30+
int[] arr3 = new int[2];
31+
Test_Span3(arr3, -1);
32+
Test_Span3(arr3, 0);
33+
Test_Span3(arr3, 1);
34+
Throws<IndexOutOfRangeException>(() => Test_Span3(arr3, 2));
35+
if (!arr3.SequenceEqual(new[] { 0, 42 }))
36+
throw new Exception("Test_Span1 failed");
37+
38+
// Should not throw NRE
39+
Test_Array(arr3, -1);
40+
return 100;
41+
}
42+
43+
private static void Throws<T>(Action action) where T : Exception
44+
{
45+
try
46+
{
47+
action();
48+
}
49+
catch (T)
50+
{
51+
return;
52+
}
53+
throw new Exception($"Expected {typeof(T)} to be thrown");
54+
}
55+
56+
[MethodImpl(MethodImplOptions.NoInlining)]
57+
private static void Test_Span1(Span<int> arr, int i)
58+
{
59+
if (i >= 0 && i < arr.Length)
60+
arr[i] = 42;
61+
}
62+
63+
[MethodImpl(MethodImplOptions.NoInlining)]
64+
private static void Test_Span2(Span<int> arr, int i)
65+
{
66+
if (i > 0 && i < arr.Length)
67+
arr[i] = 42;
68+
}
69+
70+
[MethodImpl(MethodImplOptions.NoInlining)]
71+
private static void Test_Span3(Span<int> arr, int i)
72+
{
73+
if (i > 0 && i <= arr.Length)
74+
arr[i] = 42;
75+
}
76+
77+
[MethodImpl(MethodImplOptions.NoInlining)]
78+
private static void Test_Array(Span<int> arr, int i)
79+
{
80+
// Can't be folded into "(uint)i < arr.Length"
81+
// because arr can be null
82+
if (i >= 0 && i < arr.Length)
83+
arr[i] = 42;
84+
}
85+
}

‎src/tests/JIT/opt/RedundantBranch/RedundantBranchUnsigned2.csproj

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<Optimize>True</Optimize>
4+
</PropertyGroup>
5+
<ItemGroup>
6+
<Compile Include="$(MSBuildProjectName).cs" />
7+
</ItemGroup>
8+
</Project>

0 commit comments

Comments
 (0)