Skip to content

Commit

Permalink
Introduce a concept of minimum array length into range check
Browse files Browse the repository at this point in the history
  • Loading branch information
@nathan-moore
nathan-moore committed Aug 21, 2020
1 parent 3873bf5 commit 9c4c520
Show file tree
Hide file tree
Showing 6 changed files with 410 additions and 48 deletions.
23 changes: 22 additions & 1 deletion src/coreclr/src/jit/assertionprop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,6 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion)
assert(assertion->op2.u1.iconFlags != 0);
break;
case O1K_LCLVAR:
case O1K_ARR_BND:
assert((lvaTable[assertion->op1.lcl.lclNum].lvType != TYP_REF) || (assertion->op2.u1.iconVal == 0));
break;
case O1K_VALUE_NUMBER:
Expand Down Expand Up @@ -1959,12 +1958,34 @@ AssertionInfo Compiler::optAssertionGenJtrue(GenTree* tree)
{
std::swap(op1, op2);
}

ValueNum op1VN = vnStore->VNConservativeNormalValue(op1->gtVNPair);
// If op1 is lcl and op2 is const or lcl, create assertion.
if ((op1->gtOper == GT_LCL_VAR) &&
((op2->OperKind() & GTK_CONST) || (op2->gtOper == GT_LCL_VAR))) // Fix for Dev10 851483
{
return optCreateJtrueAssertions(op1, op2, assertionKind);
}
else if (vnStore->IsVNCheckedBound(op1VN) && op2->OperIs(GT_CNS_INT))
{
AssertionDsc dsc;
dsc.assertionKind = OAK_EQUAL;
dsc.op1.vn = vnStore->VNConservativeNormalValue(relop->gtVNPair);
dsc.op1.kind = O1K_ARR_BND;
dsc.op1.bnd.vnIdx = vnStore->VNForIntCon(op2->AsIntCon()->IntegralValue() - 1);
dsc.op1.bnd.vnLen = op1VN;
dsc.op2.vn = vnStore->VNConservativeNormalValue(op2->gtVNPair);
dsc.op2.kind = O2K_CONST_INT;
dsc.op2.u1.iconFlags = 0;
dsc.op2.u1.iconVal = 0;

AssertionIndex index = optAddAssertion(&dsc);
if (relop->OperIs(GT_NE))
{
return AssertionInfo::ForNextEdge(index);
}
return index;
}

// Check op1 and op2 for an indirection of a GT_LCL_VAR and keep it in op1.
if (((op1->gtOper != GT_IND) || (op1->AsOp()->gtOp1->gtOper != GT_LCL_VAR)) &&
Expand Down
102 changes: 61 additions & 41 deletions src/coreclr/src/jit/rangecheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "jitpch.h"
#include "rangecheck.h"
#include "compiler.h"

// Max stack depth (path length) in walking the UD chain.
static const int MAX_SEARCH_DEPTH = 100;
Expand Down Expand Up @@ -60,7 +61,7 @@ int RangeCheck::GetArrLength(ValueNum vn)
}

// Check if the computed range is within bounds.
bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper)
bool RangeCheck::BetweenBounds(Range& range, int lower, ValueNum uLimitVN, int arrSize DEBUGARG(GenTree* upper))
{
#ifdef DEBUG
if (m_pCompiler->verbose)
Expand All @@ -73,10 +74,8 @@ bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper)

ValueNumStore* vnStore = m_pCompiler->vnStore;

// Get the VN for the upper limit.
ValueNum uLimitVN = vnStore->VNConservativeNormalValue(upper->gtVNPair);

#ifdef DEBUG
assert(vnStore->VNConservativeNormalValue(upper->gtVNPair));
JITDUMP(FMT_VN " upper bound is: ", uLimitVN);
if (m_pCompiler->verbose)
{
Expand All @@ -85,26 +84,7 @@ bool RangeCheck::BetweenBounds(Range& range, int lower, GenTree* upper)
JITDUMP("\n");
#endif

int arrSize = 0;

if (vnStore->IsVNConstant(uLimitVN))
{
ssize_t constVal = -1;
unsigned iconFlags = 0;

if (m_pCompiler->optIsTreeKnownIntValue(true, upper, &constVal, &iconFlags))
{
arrSize = (int)constVal;
}
}
else if (vnStore->IsVNArrLen(uLimitVN))
{
// Get the array reference from the length.
ValueNum arrRefVN = vnStore->GetArrForLenVn(uLimitVN);
// Check if array size can be obtained.
arrSize = vnStore->GetNewArrSize(arrRefVN);
}
else if (!vnStore->IsVNCheckedBound(uLimitVN))
if ((arrSize <= 0) && !vnStore->IsVNCheckedBound(uLimitVN))
{
// If the upper limit is not length, then bail.
return false;
Expand Down Expand Up @@ -231,6 +211,16 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
#endif // FEATURE_SIMD
{
arrSize = GetArrLength(arrLenVn);
if (arrSize <= 0)
{
// see if there are any assertions about the array size we can use
Range arrLength = Range(Limit(Limit::keDependent));
MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength);
if (arrLength.lLimit.IsConstant())
{
arrSize = arrLength.lLimit.GetConstant();
}
}
}

JITDUMP("ArrSize for lengthVN:%03X = %d\n", arrLenVn, arrSize);
Expand Down Expand Up @@ -286,7 +276,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
}

// Is the range between the lower and upper bound values.
if (BetweenBounds(range, 0, bndsChk->gtArrLen))
if (BetweenBounds(range, 0, arrLenVn, arrSize DEBUGARG(bndsChk->gtArrLen)))
{
JITDUMP("[RangeCheck::OptimizeRangeCheck] Between bounds\n");
m_pCompiler->optRemoveRangeCheck(treeParent, stmt);
Expand Down Expand Up @@ -532,7 +522,6 @@ void RangeCheck::SetDef(UINT64 hash, Location* loc)
}
#endif

// Merge assertions on the edge flowing into the block about a variable.
void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange)
{
if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions))
Expand All @@ -544,6 +533,20 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
{
return;
}

LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
{
varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
}
LclSsaVarDsc* ssaData = varDsc->GetPerSsaData(lcl->GetSsaNum());
ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
MergeEdgeAssertions(normalLclVN, assertions, pRange);
}

// Merge assertions on the edge flowing into the block about a variable.
void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange)
{
// Walk through the "assertions" to check if the apply.
BitVecOps::Iter iter(m_pCompiler->apTraits, assertions);
unsigned index = 0;
Expand All @@ -556,14 +559,6 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
Limit limit(Limit::keUndef);
genTreeOps cmpOper = GT_NONE;

LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lcl);
if (varDsc->CanBeReplacedWithItsField(m_pCompiler))
{
varDsc = m_pCompiler->lvaGetDesc(varDsc->lvFieldLclStart);
}
LclSsaVarDsc* ssaData = varDsc->GetPerSsaData(lcl->GetSsaNum());
ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);

// Current assertion is of the form (i < len - cns) != 0
if (curAssertion->IsCheckedBoundArithBound())
{
Expand Down Expand Up @@ -602,13 +597,20 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
m_pCompiler->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);

// If we don't have the same variable we are comparing against, bail.
if (normalLclVN != info.cmpOp)
if (normalLclVN == info.cmpOp)
{
cmpOper = (genTreeOps)info.cmpOper;
limit = Limit(Limit::keBinOpArray, info.vnBound, 0);
}
else if (info.vnBound == info.vnBound)
{
cmpOper = GenTree::SwapRelop((genTreeOps)info.cmpOper);
limit = Limit(Limit::keBinOpArray, info.cmpOp, 0);
}
else
{
continue;
}

limit = Limit(Limit::keBinOpArray, info.vnBound, 0);
cmpOper = (genTreeOps)info.cmpOper;
}
// Current assertion is of the form (i < 100) != 0
else if (curAssertion->IsConstantBound())
Expand All @@ -627,6 +629,11 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
limit = Limit(Limit::keConstant, info.constVal);
cmpOper = (genTreeOps)info.cmpOper;
}
else if (IsConstantAssertion(curAssertion, normalLclVN))
{
limit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue<int>(curAssertion->op2.vn));
cmpOper = GT_EQ;
}
// Current assertion is not supported, ignore it
else
{
Expand All @@ -636,7 +643,8 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
assert(limit.IsBinOpArray() || limit.IsConstant());

// Make sure the assertion is of the form != 0 or == 0.
if (curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT))
if ((curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)) &&
(cmpOper != GT_EQ))
{
continue;
}
Expand All @@ -647,6 +655,13 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
}
#endif

// Limits are sometimes made with the form vn + constant, where vn is a known constant
// see if we can simplify this to just a constant
if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn))
{
limit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue<int>(limit.vn) + limit.cns);
}

ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->gtArrLen->gtVNPair);

if (m_pCompiler->vnStore->IsVNConstant(arrLenVN))
Expand All @@ -663,6 +678,7 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
// (i < length) != 0
// (i < 100) == 0
// (i < 100) != 0
// i == 100
//
// At this point, we have detected that op1.vn is (i < length) or (i < length + cns) or
// (i < 100) and the op2.vn is 0.
Expand All @@ -673,12 +689,12 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
// If we have an assertion of the form == 0 (i.e., equals false), then reverse relop.
// The relop has to be reversed because we have: (i < length) is false which is the same
// as (i >= length).
if (curAssertion->assertionKind == Compiler::OAK_EQUAL)
if ((curAssertion->assertionKind == Compiler::OAK_EQUAL) && (cmpOper != GT_EQ))
{
cmpOper = GenTree::ReverseRelop(cmpOper);
}

// Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't overflow.
// Bounds are inclusive, so add -1 for upper bound when "<". But make sure we won't underflow.
if (cmpOper == GT_LT && !limit.AddConstant(-1))
{
continue;
Expand Down Expand Up @@ -744,6 +760,10 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
pRange->lLimit = limit;
break;

case GT_EQ:
pRange->uLimit = limit;
pRange->lLimit = limit;

default:
// All other 'cmpOper' kinds leave lLimit/uLimit unchanged
break;
Expand Down
27 changes: 26 additions & 1 deletion src/coreclr/src/jit/rangecheck.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ struct Limit
}
return false;
}

#ifdef DEBUG
const char* ToString(CompAllocator alloc)
{
Expand Down Expand Up @@ -376,6 +377,21 @@ struct RangeOps
result.uLimit = r2hi;
}
}
if (r1lo.IsBinOpArray() && r2lo.IsBinOpArray() && r1lo.vn == r2lo.vn)
{
result.lLimit = r2lo.GetConstant() < r1lo.GetConstant() ? r2lo : r1lo;
}

if (r1lo.IsConstant() && r1lo.GetConstant() >= 0 && r2lo.IsBinOpArray() &&
r2lo.GetConstant() >= r1lo.GetConstant())
{
result.lLimit = r1lo;
}
else if (r2lo.IsConstant() && r2lo.GetConstant() >= 0 && r1lo.IsBinOpArray() &&
r1lo.GetConstant() >= r2lo.GetConstant())
{
result.lLimit = r2lo;
}
return result;
}
};
Expand Down Expand Up @@ -438,7 +454,7 @@ class RangeCheck
// assumes that the lower range is resolved and upper range is symbolic as in an
// increasing loop.
// TODO-CQ: This is not general enough.
bool BetweenBounds(Range& range, int lower, GenTree* upper);
bool BetweenBounds(Range& range, int lower, ValueNum uLimitVN, int arrSize DEBUGARG(GenTree* upper));

// Entry point to optimize range checks in the block. Assumes value numbering
// and assertion prop phases are completed.
Expand Down Expand Up @@ -474,6 +490,8 @@ class RangeCheck
// refine the "pRange" value.
void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange);

void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP, Range* pRange);

// The maximum possible value of the given "limit." If such a value could not be determined
// return "false." For example: ARRLEN_MAX for array length.
bool GetLimitMax(Limit& limit, int* pMax);
Expand Down Expand Up @@ -519,6 +537,13 @@ class RangeCheck

GenTreeBoundsChk* m_pCurBndsChk;

// Is the given assertion a constant assertion with the given vn
bool IsConstantAssertion(Compiler::AssertionDsc* dsc, ValueNum vn)
{
return (dsc->assertionKind == Compiler::OAK_EQUAL) && (dsc->op1.vn == vn)
&& (m_pCompiler->vnStore->IsVNInt32Constant(dsc->op2.vn));
}

// Get the cached overflow values.
OverflowMap* GetOverflowMap();
OverflowMap* m_pOverflowMap;
Expand Down
6 changes: 1 addition & 5 deletions src/libraries/System.Private.CoreLib/src/System/String.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,9 @@ public char[] ToCharArray(int startIndex, int length)
[NonVersionable]
public static bool IsNullOrEmpty([NotNullWhen(false)] string? value)
{
// Using 0u >= (uint)value.Length rather than
// value.Length == 0 as it will elide the bounds check to
// the first char: value[0] if that is performed following the test
// for the same test cost.
// Ternary operator returning true/false prevents redundant asm generation:
// https://github.com/dotnet/runtime/issues/4207
return (value == null || 0u >= (uint)value.Length) ? true : false;
return (value == null || 0 == value.Length) ? true : false;
}

public static bool IsNullOrWhiteSpace([NotNullWhen(false)] string? value)
Expand Down
Loading

0 comments on commit 9c4c520

Please sign in to comment.