Skip to content

Revert "Use rangecheck in assertprop (#112766)" #112872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 24, 2025
Merged

Revert "Use rangecheck in assertprop (#112766)" #112872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 6 additions & 70 deletions src/coreclr/jit/assertionprop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
*/

#include "jitpch.h"
#include "rangecheck.h"
#ifdef _MSC_VER
#pragma hdrstop
#endif
Expand Down Expand Up @@ -4126,69 +4125,6 @@ void Compiler::optAssertionProp_RangeProperties(ASSERT_VALARG_TP assertions,
}
}
}

if (*isKnownNonZero && *isKnownNonNegative)
{
return;
}

// Let's see if MergeEdgeAssertions can help us:
if (tree->TypeIs(TYP_INT))
{
// See if (X + CNS) is known to be non-negative
if (tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsIntCnsFitsInI32())
{
Range rng = Range(Limit(Limit::keDependent));
ValueNum vn = vnStore->VNConservativeNormalValue(tree->gtGetOp1()->gtVNPair);
RangeCheck::MergeEdgeAssertions(this, vn, ValueNumStore::NoVN, assertions, &rng, false);

int cns = static_cast<int>(tree->gtGetOp2()->AsIntCon()->IconValue());
rng.LowerLimit().AddConstant(cns);

if ((rng.LowerLimit().IsConstant() && !rng.LowerLimit().AddConstant(cns)) ||
(rng.UpperLimit().IsConstant() && !rng.UpperLimit().AddConstant(cns)))
{
// Add cns to both bounds if they are constants. Make sure the addition doesn't overflow.
return;
}

if (rng.LowerLimit().IsConstant())
{
// E.g. "X + -8" when X's range is [8..unknown]
// it's safe to say "X + -8" is non-negative
if ((rng.LowerLimit().GetConstant() == 0))
{
*isKnownNonNegative = true;
}

// E.g. "X + 8" when X's range is [0..CNS]
// Here we have to check the upper bound as well to avoid overflow
if ((rng.LowerLimit().GetConstant() > 0) && rng.UpperLimit().IsConstant() &&
rng.UpperLimit().GetConstant() > rng.LowerLimit().GetConstant())
{
*isKnownNonNegative = true;
*isKnownNonZero = true;
}
}
}
else
{
Range rng = Range(Limit(Limit::keDependent));
RangeCheck::MergeEdgeAssertions(this, treeVN, ValueNumStore::NoVN, assertions, &rng, false);
Limit lowerBound = rng.LowerLimit();
if (lowerBound.IsConstant())
{
if (lowerBound.GetConstant() >= 0)
{
*isKnownNonNegative = true;
}
if (lowerBound.GetConstant() > 0)
{
*isKnownNonZero = true;
}
}
}
}
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -4915,6 +4851,12 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCas
// Skip over a GT_COMMA node(s), if necessary to get to the lcl.
GenTree* lcl = op1->gtEffectiveVal();

// If we don't have a cast of a LCL_VAR then bail.
if (!lcl->OperIs(GT_LCL_VAR))
{
return nullptr;
}

// Try and see if we can make this cast into a cheaper zero-extending version
// if the input is known to be non-negative.
if (!cast->IsUnsigned() && genActualTypeIsInt(lcl) && cast->TypeIs(TYP_LONG) && (TARGET_POINTER_SIZE == 8))
Expand All @@ -4928,12 +4870,6 @@ GenTree* Compiler::optAssertionProp_Cast(ASSERT_VALARG_TP assertions, GenTreeCas
}
}

// If we don't have a cast of a LCL_VAR then bail.
if (!lcl->OperIs(GT_LCL_VAR))
{
return nullptr;
}

IntegralRange range = IntegralRange::ForCastInput(cast);
AssertionIndex index = optAssertionIsSubrange(lcl, range, assertions);
if (index != NO_ASSERTION_INDEX)
Expand Down
84 changes: 31 additions & 53 deletions src/coreclr/jit/rangecheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ void RangeCheck::OptimizeRangeCheck(BasicBlock* block, Statement* stmt, GenTree*
{
JITDUMP("Looking for array size assertions for: " FMT_VN "\n", arrLenVn);
Range arrLength = Range(Limit(Limit::keDependent));
MergeEdgeAssertions(m_pCompiler, arrLenVn, arrLenVn, block->bbAssertionIn, &arrLength);
MergeEdgeAssertions(arrLenVn, block->bbAssertionIn, &arrLength);
if (arrLength.lLimit.IsConstant())
{
arrSize = arrLength.lLimit.GetConstant();
Expand Down Expand Up @@ -640,28 +640,20 @@ void RangeCheck::MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP

LclSsaVarDsc* ssaData = m_pCompiler->lvaGetDesc(lcl)->GetPerSsaData(lcl->GetSsaNum());
ValueNum normalLclVN = m_pCompiler->vnStore->VNConservativeNormalValue(ssaData->m_vnPair);
ValueNum arrLenVN = m_pCompiler->vnStore->VNConservativeNormalValue(m_pCurBndsChk->GetArrayLength()->gtVNPair);
MergeEdgeAssertions(m_pCompiler, normalLclVN, arrLenVN, assertions, pRange);
MergeEdgeAssertions(normalLclVN, assertions, pRange);
}

//------------------------------------------------------------------------
// MergeEdgeAssertions: Merge assertions on the edge flowing into the block about a variable
//
// Arguments:
// comp - the compiler instance
// normalLclVN - the value number to look for assertions for
// preferredBoundVN - when this VN is set, it will be given preference over constant limits
// assertions - the assertions to use
// pRange - the range to tighten with assertions
// normalLclVN - the value number to look for assertions for
// assertions - the assertions to use
// pRange - the range to tighten with assertions
//
void RangeCheck::MergeEdgeAssertions(Compiler* comp,
ValueNum normalLclVN,
ValueNum preferredBoundVN,
ASSERT_VALARG_TP assertions,
Range* pRange,
bool log)
void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP assertions, Range* pRange)
{
if (BitVecOps::IsEmpty(comp->apTraits, assertions))
if (BitVecOps::IsEmpty(m_pCompiler->apTraits, assertions))
{
return;
}
Expand All @@ -671,14 +663,14 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
return;
}

// Walk through the "assertions" to check if they apply.
BitVecOps::Iter iter(comp->apTraits, assertions);
// Walk through the "assertions" to check if the apply.
BitVecOps::Iter iter(m_pCompiler->apTraits, assertions);
unsigned index = 0;
while (iter.NextElem(&index))
{
AssertionIndex assertionIndex = GetAssertionIndex(index);

Compiler::AssertionDsc* curAssertion = comp->optGetAssertion(assertionIndex);
Compiler::AssertionDsc* curAssertion = m_pCompiler->optGetAssertion(assertionIndex);

Limit limit(Limit::keUndef);
genTreeOps cmpOper = GT_NONE;
Expand All @@ -691,7 +683,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
ValueNumStore::CompareCheckedBoundArithInfo info;

// Get i, len, cns and < as "info."
comp->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info);
m_pCompiler->vnStore->GetCompareCheckedBoundArithInfo(curAssertion->op1.vn, &info);

// If we don't have the same variable we are comparing against, bail.
if (normalLclVN != info.cmpOp)
Expand All @@ -705,12 +697,12 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
}

// If the operand that operates on the bound is not constant, then done.
if (!comp->vnStore->IsVNInt32Constant(info.arrOp))
if (!m_pCompiler->vnStore->IsVNInt32Constant(info.arrOp))
{
continue;
}

int cons = comp->vnStore->ConstantValue<int>(info.arrOp);
int cons = m_pCompiler->vnStore->ConstantValue<int>(info.arrOp);
limit = Limit(Limit::keBinOpArray, info.vnBound, info.arrOper == GT_SUB ? -cons : cons);
cmpOper = (genTreeOps)info.cmpOper;
}
Expand All @@ -720,7 +712,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
ValueNumStore::CompareCheckedBoundArithInfo info;

// Get the info as "i", "<" and "len"
comp->vnStore->GetCompareCheckedBound(curAssertion->op1.vn, &info);
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)
Expand All @@ -744,7 +736,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
ValueNumStore::ConstantBoundInfo info;

// Get the info as "i", "<" and "100"
comp->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info);
m_pCompiler->vnStore->GetConstantBoundInfo(curAssertion->op1.vn, &info);

// If we don't have the same variable we are comparing against, bail.
if (normalLclVN != info.cmpOpVN)
Expand All @@ -764,10 +756,10 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
continue;
}

int cnstLimit = comp->vnStore->CoercedConstantValue<int>(curAssertion->op2.vn);
int cnstLimit = m_pCompiler->vnStore->CoercedConstantValue<int>(curAssertion->op2.vn);

if ((cnstLimit == 0) && (curAssertion->assertionKind == Compiler::OAK_NOT_EQUAL) &&
comp->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
m_pCompiler->vnStore->IsVNCheckedBound(curAssertion->op1.vn))
{
// we have arr.Len != 0, so the length must be atleast one
limit = Limit(Limit::keConstant, 1);
Expand Down Expand Up @@ -812,31 +804,31 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,

// Make sure the assertion is of the form != 0 or == 0 if it isn't a constant assertion.
if (!isConstantAssertion && (curAssertion->assertionKind != Compiler::OAK_NO_THROW) &&
(curAssertion->op2.vn != comp->vnStore->VNZeroForType(TYP_INT)))
(curAssertion->op2.vn != m_pCompiler->vnStore->VNZeroForType(TYP_INT)))
{
continue;
}
#ifdef DEBUG
if (comp->verbose)
if (m_pCompiler->verbose)
{
comp->optPrintAssertion(curAssertion, assertionIndex);
m_pCompiler->optPrintAssertion(curAssertion, assertionIndex);
}
#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() && comp->vnStore->IsVNInt32Constant(limit.vn))
if (limit.IsBinOpArray() && m_pCompiler->vnStore->IsVNInt32Constant(limit.vn))
{
Limit tempLimit = Limit(Limit::keConstant, comp->vnStore->ConstantValue<int>(limit.vn));
Limit tempLimit = Limit(Limit::keConstant, m_pCompiler->vnStore->ConstantValue<int>(limit.vn));
if (tempLimit.AddConstant(limit.cns))
{
limit = tempLimit;
}
}

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

if (comp->vnStore->IsVNConstant(arrLenVN))
if (m_pCompiler->vnStore->IsVNConstant(arrLenVN))
{
// Set arrLenVN to NoVN; this will make it match the "vn" recorded on
// constant limits (where we explicitly track the constant and don't
Expand Down Expand Up @@ -925,10 +917,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,

if (limit.vn != arrLenVN)
{
if (log)
{
JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn);
}
JITDUMP("Array length VN did not match arrLen=" FMT_VN ", limit=" FMT_VN "\n", arrLenVN, limit.vn);
continue;
}

Expand All @@ -938,10 +927,7 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
// Incoming limit doesn't tighten the existing upper limit.
if (limCns >= curCns)
{
if (log)
{
JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns);
}
JITDUMP("Bound limit %d doesn't tighten current bound %d\n", limCns, curCns);
continue;
}
}
Expand All @@ -968,12 +954,8 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,

case GT_GT:
case GT_GE:
// GT/GE being unsigned creates a non-contiguous range which we can't represent
// using single Range object.
if (!isUnsigned)
{
pRange->lLimit = limit;
}
pRange->lLimit = limit;
// it doesn't matter if it's isUnsigned or not here - it's not negative anyway.
break;

case GT_EQ:
Expand All @@ -985,13 +967,9 @@ void RangeCheck::MergeEdgeAssertions(Compiler* comp,
// All other 'cmpOper' kinds leave lLimit/uLimit unchanged
break;
}

if (log)
{
JITDUMP("The range after edge merging:");
JITDUMP(pRange->ToString(comp));
JITDUMP("\n");
}
JITDUMP("The range after edge merging:");
JITDUMP(pRange->ToString(m_pCompiler));
JITDUMP("\n");
}
}

Expand Down
7 changes: 1 addition & 6 deletions src/coreclr/jit/rangecheck.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,12 +686,7 @@ class RangeCheck
void MergeEdgeAssertions(GenTreeLclVarCommon* lcl, ASSERT_VALARG_TP assertions, Range* pRange);

// Inspect the assertions about the current ValueNum to refine pRange
static void MergeEdgeAssertions(Compiler* comp,
ValueNum num,
ValueNum preferredBoundVN,
ASSERT_VALARG_TP assertions,
Range* pRange,
bool log = true);
void MergeEdgeAssertions(ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange);

// The maximum possible value of the given "limit". If such a value could not be determined
// return "false". For example: CORINFO_Array_MaxLength for array length.
Expand Down
Loading