Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5626,8 +5626,13 @@ Compiler::AssertVisit Compiler::optVisitReachingAssertions(ValueNum vn, TAssertV
return AssertVisit::Abort;
}

const ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArg->gtVNPair);
ASSERT_TP assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB);
// fgValueNumberPhiDef may leave gtVNPair as NoVN when it refuses to back-patch
// a loop-varying SSA-def VN into the phi-arg slot (a hygiene constraint for general
// gtVNPair consumers that does not apply here, since we hand the VN to the visitor
// together with this edge's assertion set and never write it back into any tree).
LclSsaVarDsc* phiArgSsaDef = lvaGetDesc(phiArg)->GetPerSsaData(phiArg->GetSsaNum());
ValueNum phiArgVN = vnStore->VNConservativeNormalValue(phiArgSsaDef->m_vnPair);
ASSERT_TP assertions = optGetEdgeAssertions(ssaDef->GetBlock(), phiArg->gtPredBB);
if (argVisitor(phiArgVN, assertions) == AssertVisit::Abort)
{
// The visitor wants to abort the walk.
Expand Down
79 changes: 63 additions & 16 deletions src/coreclr/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -650,11 +650,34 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP
// comp - the compiler instance
// num - the value number to analyze range for
// assertions - the assertions to use
// budget - the remaining budget for recursive analysis
//
// Return Value:
// The computed range
//
Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int budget)
{
ValueNumStore::SmallValueNumSet set;
return GetRangeFromAssertionsWorker(comp, num, assertions, budget, &set);
}

//------------------------------------------------------------------------
// GetRangeFromAssertionsWorker: Cheaper version of TryGetRange that is based purely on assertions
// and does not require a full range analysis based on SSA.
//
// Arguments:
// comp - the compiler instance
// num - the value number to analyze range for
// assertions - the assertions to use
// budget - the remaining budget for recursive analysis
// visited - the set of value numbers already visited in the current search
// path to prevent infinite recursion
//
// Return Value:
// The computed range
//
Range RangeCheck::GetRangeFromAssertionsWorker(
Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, int budget, ValueNumStore::SmallValueNumSet* visited)
{
// Start with the widest possible constant range.
Range result = Range(Limit(Limit::keConstant, INT32_MIN), Limit(Limit::keConstant, INT32_MAX));
Expand Down Expand Up @@ -699,7 +722,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
// if its range is within the castTo range, we can use that (and the cast is basically a no-op).
if (comp->vnStore->TypeOfVN(funcApp.m_args[0]) == TYP_INT)
{
Range castOpRange = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
Range castOpRange =
GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited);
if (castOpRange.IsConstantRange() &&
(castOpRange.LowerLimit().GetConstant() >= castToTypeRange.LowerLimit().GetConstant()) &&
(castOpRange.UpperLimit().GetConstant() <= castToTypeRange.UpperLimit().GetConstant()))
Expand All @@ -713,7 +737,7 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA

case VNF_NEG:
{
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited);
Range unaryOpResult = RangeOps::Negate(r1);

// We can use the result only if it never overflows.
Expand All @@ -732,8 +756,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
case VNF_UMOD:
{
// Get ranges of both operands and perform the same operation on the ranges.
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
Range r2 = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, --budget);
Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited);
Range r2 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[1], assertions, --budget, visited);
Range binOpResult = Range(Limit(Limit::keUnknown));
switch (funcApp.m_func)
{
Expand Down Expand Up @@ -799,8 +823,8 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
if ((genActualType(comp->vnStore->TypeOfVN(funcApp.m_args[0])) == TYP_INT) &&
(genActualType(comp->vnStore->TypeOfVN(funcApp.m_args[1])) == TYP_INT))
{
Range r1 = GetRangeFromAssertions(comp, funcApp.m_args[0], assertions, --budget);
Range r2 = GetRangeFromAssertions(comp, funcApp.m_args[1], assertions, --budget);
Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited);
Range r2 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[1], assertions, --budget, visited);

bool isUnsigned = true;
genTreeOps cmpOper;
Expand Down Expand Up @@ -846,9 +870,9 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
}

Range phiRange = Range(Limit(Limit::keUndef));
auto visitor = [comp, &phiRange, &budget](ValueNum reachingVN, ASSERT_TP reachingAssertions) {
// call GetRangeFromAssertions for each reaching VN using reachingAssertions
Range edgeRange = GetRangeFromAssertions(comp, reachingVN, reachingAssertions, --budget);
auto visitor = [comp, &phiRange, &budget, visited](ValueNum reachingVN, ASSERT_TP reachingAssertions) {
// call GetRangeFromAssertionsWorker for each reaching VN using reachingAssertions
Range edgeRange = GetRangeFromAssertionsWorker(comp, reachingVN, reachingAssertions, --budget, visited);

// If phiRange is not yet set, set it to the first edgeRange
// else merge it with the new edgeRange. Example: [10..100] U [50..150] = [10..150]
Expand All @@ -864,7 +888,9 @@ Range RangeCheck::GetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VA
return Compiler::AssertVisit::Abort;
};

if (comp->optVisitReachingAssertions(num, visitor) == Compiler::AssertVisit::Continue && !phiRange.IsUndef())
// If it's a phi, we need to look at the reaching assertions for each of the phi args and merge them together.
if (comp->vnStore->IsPhiDef(num) && visited->Add(comp, num) &&
(comp->optVisitReachingAssertions(num, visitor) == Compiler::AssertVisit::Continue) && !phiRange.IsUndef())
{
Comment thread
EgorBo marked this conversation as resolved.
assert(phiRange.IsConstantRange());
result = phiRange;
Expand Down Expand Up @@ -1082,17 +1108,38 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
else if (curAssertion.KindIs(Compiler::OAK_GE, Compiler::OAK_GT, Compiler::OAK_LE, Compiler::OAK_LT) &&
curAssertion.GetOp2().KindIs(Compiler::O2K_CHECKED_BOUND_ADD_CNS))
{
const ValueNum boundVN = curAssertion.GetOp2().GetCheckedBound();
const int boundCns = curAssertion.GetOp2().GetCheckedBoundConstant();

Comment thread
EgorBo marked this conversation as resolved.
// Check whether normalLclVN VN-equals the op2 expression "(boundVN + boundCns)":
// - trivially, when boundCns is 0 and normalLclVN == boundVN, or
// - when normalLclVN itself is the value-number "ADD(boundVN, boundCns)".
//
// The general case commonly arises on a loop back-edge where the induction variable
// is e.g. V = phi(V0, V - 8), and the back-edge JTRUE generates an assertion of the
// form "8 <= (V + -8)" against the phi's VN. We want to apply that assertion to the
// value of the back-edge phi-arg "(V - 8)" itself.
//
Comment thread
EgorBo marked this conversation as resolved.
// Note: when boundCns == 0, the trivial check above is exhaustive -- VN normalizes
// ADD(x, 0) to x, so any IsVNBinFuncWithConst match would also have to satisfy
// normalLclVN == boundVN, which the trivial check already covers. Skip the lookup
// in that case to keep this fast-path cheap.
ValueNum addOp;
int addCns;
const bool normalLclVNMatchesOp2 =
((normalLclVN == boundVN) && (boundCns == 0)) ||
((boundCns != 0) && comp->vnStore->IsVNBinFuncWithConst(normalLclVN, VNF_ADD, &addOp, &addCns) &&
(addOp == boundVN) && (addCns == boundCns));
Comment thread
EgorBo marked this conversation as resolved.

if (canUseCheckedBounds && (normalLclVN == curAssertion.GetOp1().GetVN()))
{
cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned);
limit = Limit(Limit::keBinOpArray, curAssertion.GetOp2().GetCheckedBound(),
curAssertion.GetOp2().GetCheckedBoundConstant());
limit = Limit(Limit::keBinOpArray, boundVN, boundCns);
}
else if ((normalLclVN == curAssertion.GetOp2().GetCheckedBound()) &&
(curAssertion.GetOp2().GetCheckedBoundConstant() == 0))
else if (normalLclVNMatchesOp2)
{
// Check if it's "Const <relop> (checkedBndVN + 0)" or in other words "Const <relop> checkedBndVN"
// If normalLclVN == checkedBndVN, then we can deduce "normalLclVN <relop> Const"
// Check if it's "Const <relop> (checkedBndVN + cns)" or in other words "Const <relop> normalLclVN"
// If normalLclVN VN-equals (checkedBndVN + cns), we can deduce "normalLclVN <relop> Const"
//
// Example: We created "O1K_VN(vn1) - OAK_GT - O2K_CHECKED_BOUND_ADD_CNS(vn2, 0)"
// if vn1 is a constant (say, 100) and vn2 is our normalLclVN, we can deduce "normalLclVN < 100"
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/jit/rangecheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,13 @@ class RangeCheck
typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, Range*> RangeMap;
typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, BasicBlock*> SearchPath;

// Cheaper version of TryGetRange that is based only on incoming assertions.
static Range GetRangeFromAssertionsWorker(Compiler* comp,
ValueNum num,
ASSERT_VALARG_TP assertions,
int budget,
ValueNumStore::SmallValueNumSet* visited);

int GetArrLength(ValueNum vn);

// Check whether the computed range is within 0 and upper bounds. This function
Expand Down
Loading