Skip to content

Commit a9616d3

Browse files
EgorBoEgorBo
committed
clean up
1 parent 618a00d commit a9616d3

File tree

3 files changed

+42
-67
lines changed

3 files changed

+42
-67
lines changed

‎src/coreclr/jit/assertionprop.cpp‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3971,18 +3971,13 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions,
39713971
if (tree->TypeIs(TYP_INT))
39723972
{
39733973
Range rng = RangeCheck::GetRangeFromAssertions(this, treeVN, assertions);
3974-
if (rng.IsConstantRange())
3974+
if (rng.LowerLimit().GetConstant() >= 0)
39753975
{
3976-
Limit lowerBound = rng.LowerLimit();
3977-
assert(lowerBound.IsConstant());
3978-
if (lowerBound.GetConstant() >= 0)
3979-
{
3980-
*isKnownNonNegative = true;
3981-
}
3982-
if (lowerBound.GetConstant() > 0)
3983-
{
3984-
*isKnownNonZero = true;
3985-
}
3976+
*isKnownNonNegative = true;
3977+
}
3978+
if (rng.LowerLimit().GetConstant() > 0)
3979+
{
3980+
*isKnownNonZero = true;
39863981
}
39873982
}
39883983
}
@@ -4338,9 +4333,7 @@ GenTree* Compiler::optAssertionPropGlobal_RelOp(ASSERT_VALARG_TP assertions,
43384333
ValueNum op1VN = vnStore->VNConservativeNormalValue(op1->gtVNPair);
43394334
ValueNum op2VN = vnStore->VNConservativeNormalValue(op2->gtVNPair);
43404335

4341-
// IsVNIntegralConstant for op2VN is for reducing the TP regression, can be removed.
4342-
int op2cns;
4343-
if (op1->TypeIs(TYP_INT) && op2->TypeIs(TYP_INT) && vnStore->IsVNIntegralConstant(op2VN, &op2cns))
4336+
if (op1->TypeIs(TYP_INT) && op2->TypeIs(TYP_INT))
43444337
{
43454338
Range rng1 = RangeCheck::GetRangeFromAssertions(this, op1VN, assertions);
43464339
Range rng2 = RangeCheck::GetRangeFromAssertions(this, op2VN, assertions);
@@ -5285,17 +5278,14 @@ GenTree* Compiler::optAssertionProp_BndsChk(ASSERT_VALARG_TP assertions, GenTree
52855278
if ((funcApp.m_args[0] == vnCurLen) && vnStore->IsVNInt32Constant(funcApp.m_args[1]))
52865279
{
52875280
Range rng = RangeCheck::GetRangeFromAssertions(this, vnCurLen, assertions);
5288-
if (rng.IsConstantRange())
5289-
{
5290-
// Lower known limit of ArrLen:
5291-
const int lenLowerLimit = rng.LowerLimit().GetConstant();
5281+
// Lower known limit of ArrLen:
5282+
const int lenLowerLimit = rng.LowerLimit().GetConstant();
52925283

5293-
// Negative delta in the array access (ArrLen + -CNS)
5294-
const int delta = vnStore->GetConstantInt32(funcApp.m_args[1]);
5295-
if ((lenLowerLimit > 0) && (delta < 0) && (delta > INT_MIN) && (lenLowerLimit >= -delta))
5296-
{
5297-
return dropBoundsCheck(INDEBUG("a[a.Length-cns] when a.Length is known to be >= cns"));
5298-
}
5284+
// Negative delta in the array access (ArrLen + -CNS)
5285+
const int delta = vnStore->GetConstantInt32(funcApp.m_args[1]);
5286+
if ((lenLowerLimit > 0) && (delta < 0) && (delta > INT_MIN) && (lenLowerLimit >= -delta))
5287+
{
5288+
return dropBoundsCheck(INDEBUG("a[a.Length-cns] when a.Length is known to be >= cns"));
52995289
}
53005290
}
53015291
}

‎src/coreclr/jit/rangecheck.cpp‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -648,13 +648,19 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
648648
// Return Value:
649649
// The computed range
650650
//
651-
Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int maxDepth)
651+
Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int budget)
652652
{
653-
if (num == ValueNumStore::NoVN || maxDepth <= 0)
653+
// Start with the widest possible constant range.
654+
Range result = Range(Limit(Limit::keConstant, INT32_MIN), Limit(Limit::keConstant, INT32_MAX));
655+
656+
if ((num == ValueNumStore::NoVN) || (budget <= 0))
654657
{
655-
return Range(Limit(Limit::keUnknown));
658+
return result;
656659
}
657660

661+
// Currently, we only handle int32 and smaller integer types.
662+
assert(varTypeIsInt(comp->vnStore->TypeOfVN(num)) || varTypeIsSmall(comp->vnStore->TypeOfVN(num)));
663+
658664
//
659665
// First, let's see if we can tighten the range based on VN information.
660666
//
@@ -666,9 +672,6 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
666672
return Range(Limit(Limit::keConstant, cns));
667673
}
668674

669-
// Start with the widest possible constant range.
670-
Range result = Range(Limit(Limit::keConstant, INT32_MIN), Limit(Limit::keConstant, INT32_MAX));
671-
672675
VNFuncApp funcApp;
673676
if (comp->vnStore->GetVNFunc(num, &funcApp))
674677
{
@@ -713,12 +716,11 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
713716

714717
case VNF_NEG:
715718
{
716-
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, maxDepth - 1);
719+
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
717720
Range unaryOpResult = RangeOps::Negate(r1);
718-
if (unaryOpResult.IsConstantRange())
719-
{
720-
result = unaryOpResult;
721-
}
721+
722+
// We can use the result only if it never overflows.
723+
result = unaryOpResult.IsConstantRange() ? unaryOpResult : result;
722724
break;
723725
}
724726

@@ -732,8 +734,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
732734
case VNF_UMOD:
733735
{
734736
// Get ranges of both operands and perform the same operation on the ranges.
735-
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, maxDepth - 1);
736-
Range r2 = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, maxDepth - 1);
737+
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
738+
Range r2 = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, --budget);
737739
Range binOpResult = Range(Limit(Limit::keUnknown));
738740
switch (funcApp.m_func)
739741
{
@@ -765,12 +767,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
765767
unreached();
766768
}
767769

768-
if (binOpResult.IsConstantRange())
769-
{
770-
result = binOpResult;
771-
}
772-
// if the result is unknown (or may overflow), we'll just analyze the binop itself based on the
773-
// assertions
770+
// We can use the result only if it never overflows.
771+
result = binOpResult.IsConstantRange() ? binOpResult : result;
774772
break;
775773
}
776774

@@ -808,12 +806,17 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
808806
}
809807

810808
Range phiRange = Range(Limit(Limit::keUndef));
809+
if (optVisitReachingAssertions(comp, num,
810+
[comp, &phiRange, &budget](ValueNum reachingVN, ASSERT_TP reachingAssertions) {
811+
// call GetRangeFromAssertions for each reaching VN using reachingAssertions
812+
Range edgeRange = GetRangeFromAssertions(comp, reachingVN, reachingAssertions, --budget);
813+
814+
// If phiRange is not yet set, set it to the first edgeRange
815+
// else merge it with the new edgeRange. Example: [10..100] U [50..150] = [10..150]
816+
phiRange = phiRange.LowerLimit().IsUndef() ? edgeRange : RangeOps::Merge(phiRange, edgeRange, false);
811817

812-
if ((maxDepth > 1) &&
813-
optVisitReachingAssertions(comp, num,
814-
[comp, &phiRange, maxDepth](ValueNum reachingVN, ASSERT_TP reachingAssertions) {
815-
Range edgeRange = GetRangeFromAssertions(comp, reachingVN, reachingAssertions, maxDepth - 1);
816-
phiRange = phiRange.LowerLimit().IsUndef() ? edgeRange : RangeOps::Union(phiRange, edgeRange);
818+
// if any edge produces a non-constant range, we abort further processing
819+
// We also give up if the range is full, as it won't help tighten the result.
817820
return edgeRange.IsConstantRange() && !edgeRange.IsFullRange() ? AssertVisit::Continue : AssertVisit::Abort;
818821
}) == AssertVisit::Continue)
819822
{

‎src/coreclr/jit/rangecheck.h‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ struct RangeOps
602602
static Range Negate(const Range& range)
603603
{
604604
// Only constant ranges can be negated.
605-
if (!range.LowerLimit().IsConstant() || !range.UpperLimit().IsConstant())
605+
if (!range.IsConstantRange())
606606
{
607607
return Limit(Limit::keUnknown);
608608
}
@@ -623,24 +623,6 @@ struct RangeOps
623623
return result;
624624
}
625625

626-
static Range Union(const Range& r1, const Range& r2)
627-
{
628-
const Limit& r1lo = r1.LowerLimit();
629-
const Limit& r1hi = r1.UpperLimit();
630-
const Limit& r2lo = r2.LowerLimit();
631-
const Limit& r2hi = r2.UpperLimit();
632-
633-
if (r1.IsConstantRange() && r2.IsConstantRange())
634-
{
635-
// TODO-RangeCheck: handle BinOpArray limits too.
636-
Range result = Limit(Limit::keUnknown);
637-
result.lLimit = Limit(Limit::keConstant, min(r1lo.GetConstant(), r2lo.GetConstant()));
638-
result.uLimit = Limit(Limit::keConstant, max(r1hi.GetConstant(), r2hi.GetConstant()));
639-
return result;
640-
}
641-
return Limit(Limit::keUnknown);
642-
}
643-
644626
enum class RelationKind
645627
{
646628
AlwaysTrue,

0 commit comments

Comments
 (0)