Skip to content

Commit

Permalink
JIT: Range Checks for "(uint)i > cns" pattern (dotnet#62864)
Browse files Browse the repository at this point in the history 
Co-authored-by: SingleAccretion <62474226+SingleAccretion@users.noreply.github.com>
  • Loading branch information
@EgorBo @SingleAccretion
EgorBo and SingleAccretion committed Feb 25, 2022
1 parent afc2f11 commit e4dde6b
Show file tree
Hide file tree
Showing 7 changed files with 254 additions and 39 deletions.
41 changes: 27 additions & 14 deletions src/coreclr/jit/assertionprop.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,11 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
printf("Const_Loop_Bnd");
vnStore->vnDump(this, curAssertion->op1.vn);
}
else if (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND_UN)
{
printf("Const_Loop_Bnd_Un");
vnStore->vnDump(this, curAssertion->op1.vn);
}
else if (curAssertion->op1.kind == O1K_VALUE_NUMBER)
{
printf("Value_Number");
Expand Down Expand Up @@ -1110,17 +1115,10 @@ void Compiler::optPrintAssertion(AssertionDsc* curAssertion, AssertionIndex asse
printf("MT(%08X)", dspPtr(curAssertion->op2.u1.iconVal));
assert(curAssertion->op2.u1.iconFlags != GTF_EMPTY);
}
else if (curAssertion->op1.kind == O1K_BOUND_OPER_BND)
{
assert(!optLocalAssertionProp);
vnStore->vnDump(this, curAssertion->op2.vn);
}
else if (curAssertion->op1.kind == O1K_BOUND_LOOP_BND)
{
assert(!optLocalAssertionProp);
vnStore->vnDump(this, curAssertion->op2.vn);
}
else if (curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND)
else if ((curAssertion->op1.kind == O1K_BOUND_OPER_BND) ||
(curAssertion->op1.kind == O1K_BOUND_LOOP_BND) ||
(curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND) ||
(curAssertion->op1.kind == O1K_CONSTANT_LOOP_BND_UN))
{
assert(!optLocalAssertionProp);
vnStore->vnDump(this, curAssertion->op2.vn);
Expand Down Expand Up @@ -2011,6 +2009,7 @@ void Compiler::optDebugCheckAssertion(AssertionDsc* assertion)
case O1K_BOUND_OPER_BND:
case O1K_BOUND_LOOP_BND:
case O1K_CONSTANT_LOOP_BND:
case O1K_CONSTANT_LOOP_BND_UN:
case O1K_VALUE_NUMBER:
assert(!optLocalAssertionProp);
break;
Expand Down Expand Up @@ -2108,8 +2107,9 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex,
}

AssertionDsc& candidateAssertion = *optGetAssertion(assertionIndex);
if (candidateAssertion.op1.kind == O1K_BOUND_OPER_BND || candidateAssertion.op1.kind == O1K_BOUND_LOOP_BND ||
candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND)
if ((candidateAssertion.op1.kind == O1K_BOUND_OPER_BND) || (candidateAssertion.op1.kind == O1K_BOUND_LOOP_BND) ||
(candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND) ||
(candidateAssertion.op1.kind == O1K_CONSTANT_LOOP_BND_UN))
{
AssertionDsc dsc = candidateAssertion;
dsc.assertionKind = dsc.assertionKind == OAK_EQUAL ? OAK_NOT_EQUAL : OAK_EQUAL;
Expand Down Expand Up @@ -2370,7 +2370,20 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
optCreateComplementaryAssertion(index, nullptr, nullptr);
return index;
}

else if (vnStore->IsVNConstantBoundUnsigned(relopVN))
{
AssertionDsc dsc;
dsc.assertionKind = OAK_NOT_EQUAL;
dsc.op1.kind = O1K_CONSTANT_LOOP_BND_UN;
dsc.op1.vn = relopVN;
dsc.op2.kind = O2K_CONST_INT;
dsc.op2.vn = vnStore->VNZeroForType(TYP_INT);
dsc.op2.u1.iconVal = 0;
dsc.op2.u1.iconFlags = GTF_EMPTY;
AssertionIndex index = optAddAssertion(&dsc);
optCreateComplementaryAssertion(index, nullptr, nullptr);
return index;
}
return NO_ASSERTION_INDEX;
}

Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6443,6 +6443,7 @@ class Compiler
GenTree* fgOptimizeHWIntrinsic(GenTreeHWIntrinsic* node);
#endif
GenTree* fgOptimizeCommutativeArithmetic(GenTreeOp* tree);
GenTree* fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp);
GenTree* fgOptimizeAddition(GenTreeOp* add);
GenTree* fgOptimizeMultiply(GenTreeOp* mul);
GenTree* fgOptimizeBitwiseAnd(GenTreeOp* andOp);
Expand Down Expand Up @@ -7680,6 +7681,7 @@ class Compiler
O1K_BOUND_OPER_BND,
O1K_BOUND_LOOP_BND,
O1K_CONSTANT_LOOP_BND,
O1K_CONSTANT_LOOP_BND_UN,
O1K_EXACT_TYPE,
O1K_SUBTYPE,
O1K_VALUE_NUMBER,
Expand Down Expand Up @@ -7756,7 +7758,12 @@ class Compiler
bool IsConstantBound()
{
return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) &&
op1.kind == O1K_CONSTANT_LOOP_BND);
(op1.kind == O1K_CONSTANT_LOOP_BND));
}
bool IsConstantBoundUnsigned()
{
return ((assertionKind == OAK_EQUAL || assertionKind == OAK_NOT_EQUAL) &&
(op1.kind == O1K_CONSTANT_LOOP_BND_UN));
}
bool IsBoundsCheckNoThrow()
{
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gentree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2045,7 +2045,7 @@ struct GenTree

void SetUnsigned()
{
assert(OperIs(GT_ADD, GT_SUB, GT_CAST) || OperIsMul());
assert(OperIs(GT_ADD, GT_SUB, GT_CAST, GT_LE, GT_LT, GT_GT, GT_GE) || OperIsMul());
gtFlags |= GTF_UNSIGNED;
}

Expand Down
147 changes: 147 additions & 0 deletions src/coreclr/jit/morph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12176,6 +12176,14 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac)
case GT_GE:
case GT_GT:

if (!optValnumCSE_phase && (op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST)))
{
tree = fgOptimizeRelationalComparisonWithCasts(tree->AsOp());
oper = tree->OperGet();
op1 = tree->gtGetOp1();
op2 = tree->gtGetOp2();
}

// op2's value may be changed, so it cannot be a CSE candidate.
if (op2->IsIntegralConst() && !gtIsActiveCSE_Candidate(op2))
{
Expand Down Expand Up @@ -13968,6 +13976,145 @@ GenTree* Compiler::fgOptimizeBitwiseAnd(GenTreeOp* andOp)
return nullptr;
}

//------------------------------------------------------------------------
// fgOptimizeRelationalComparisonWithCasts: Recognizes comparisons against
// various cast operands and tries to remove them. E.g.:
//
// * GE int
// +--* CAST long <- ulong <- uint
// | \--* X int
// \--* CNS_INT long
//
// to:
//
// * GE_un int
// +--* X int
// \--* CNS_INT int
//
// same for:
//
// * GE int
// +--* CAST long <- ulong <- uint
// | \--* X int
// \--* CAST long <- [u]long <- int
// \--* ARR_LEN int
//
// These patterns quite often show up along with index checks
//
// Arguments:
// cmp - the GT_LE/GT_LT/GT_GE/GT_GT tree to morph.
//
// Return Value:
// Returns the same tree where operands might have narrower types
//
// Notes:
// TODO-Casts: consider unifying this function with "optNarrowTree"
//
GenTree* Compiler::fgOptimizeRelationalComparisonWithCasts(GenTreeOp* cmp)
{
assert(cmp->OperIs(GT_LE, GT_LT, GT_GE, GT_GT));
assert(!optValnumCSE_phase);

GenTree* op1 = cmp->gtGetOp1();
GenTree* op2 = cmp->gtGetOp2();

// Caller is expected to call this function only if we have CAST nodes
assert(op1->OperIs(GT_CAST) || op2->OperIs(GT_CAST));

if (!op1->TypeIs(TYP_LONG))
{
// We can extend this logic to handle small types as well, but currently it's done mostly to
// assist range check elimination
return cmp;
}

GenTree* castOp;
GenTree* knownPositiveOp;

bool knownPositiveIsOp2;
if (op2->IsIntegralConst() || ((op2->OperIs(GT_CAST) && op2->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH))))
{
// op2 is either a LONG constant or (T)ARR_LENGTH
knownPositiveIsOp2 = true;
castOp = cmp->gtGetOp1();
knownPositiveOp = cmp->gtGetOp2();
}
else
{
// op1 is either a LONG constant (yes, it's pretty normal for relops)
// or (T)ARR_LENGTH
castOp = cmp->gtGetOp2();
knownPositiveOp = cmp->gtGetOp1();
knownPositiveIsOp2 = false;
}

if (castOp->OperIs(GT_CAST) && varTypeIsLong(castOp->CastToType()) && castOp->AsCast()->CastOp()->TypeIs(TYP_INT) &&
castOp->IsUnsigned() && !castOp->gtOverflow())
{
bool knownPositiveFitsIntoU32 = false;
if (knownPositiveOp->IsIntegralConst() && FitsIn<UINT32>(knownPositiveOp->AsIntConCommon()->IntegralValue()))
{
// BTW, we can fold the whole condition if op2 doesn't fit into UINT_MAX.
knownPositiveFitsIntoU32 = true;
}
else if (knownPositiveOp->OperIs(GT_CAST) && varTypeIsLong(knownPositiveOp->CastToType()) &&
knownPositiveOp->AsCast()->CastOp()->OperIs(GT_ARR_LENGTH))
{
knownPositiveFitsIntoU32 = true;
// TODO-Casts: recognize Span.Length here as well.
}

if (!knownPositiveFitsIntoU32)
{
return cmp;
}

JITDUMP("Removing redundant cast(s) for:\n")
DISPTREE(cmp)
JITDUMP("\n\nto:\n\n")

cmp->SetUnsigned();

// Drop cast from castOp
if (knownPositiveIsOp2)
{
cmp->gtOp1 = castOp->AsCast()->CastOp();
}
else
{
cmp->gtOp2 = castOp->AsCast()->CastOp();
}
DEBUG_DESTROY_NODE(castOp);

if (knownPositiveOp->OperIs(GT_CAST))
{
// Drop cast from knownPositiveOp too
if (knownPositiveIsOp2)
{
cmp->gtOp2 = knownPositiveOp->AsCast()->CastOp();
}
else
{
cmp->gtOp1 = knownPositiveOp->AsCast()->CastOp();
}
DEBUG_DESTROY_NODE(knownPositiveOp);
}
else
{
// Change type for constant from LONG to INT
knownPositiveOp->ChangeType(TYP_INT);
#ifndef TARGET_64BIT
assert(knownPositiveOp->OperIs(GT_CNS_LNG));
knownPositiveOp->BashToConst(static_cast<int>(knownPositiveOp->AsIntConCommon()->IntegralValue()));
#endif
fgUpdateConstTreeValueNumber(knownPositiveOp);
}
DISPTREE(cmp)
JITDUMP("\n")
}
return cmp;
}

//------------------------------------------------------------------------
// fgPropagateCommaThrow: propagate a "comma throw" up the tree.
//
Expand Down
13 changes: 10 additions & 3 deletions src/coreclr/jit/rangecheck.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
Limit limit(Limit::keUndef);
genTreeOps cmpOper = GT_NONE;
bool isConstantAssertion = false;
bool isUnsigned = false;

// Current assertion is of the form (i < len - cns) != 0
if (curAssertion->IsCheckedBoundArithBound())
Expand Down Expand Up @@ -658,7 +659,7 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
}
}
// Current assertion is of the form (i < 100) != 0
else if (curAssertion->IsConstantBound())
else if (curAssertion->IsConstantBound() || curAssertion->IsConstantBoundUnsigned())
{
ValueNumStore::ConstantBoundInfo info;

Expand All @@ -671,8 +672,9 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
continue;
}

limit = Limit(Limit::keConstant, info.constVal);
cmpOper = (genTreeOps)info.cmpOper;
limit = Limit(Limit::keConstant, info.constVal);
cmpOper = (genTreeOps)info.cmpOper;
isUnsigned = info.isUnsigned;
}
// Current assertion is of the form i == 100
else if (curAssertion->IsConstantInt32Assertion())
Expand Down Expand Up @@ -828,11 +830,16 @@ void RangeCheck::MergeEdgeAssertions(ValueNum normalLclVN, ASSERT_VALARG_TP asse
case GT_LT:
case GT_LE:
pRange->uLimit = limit;
if (isUnsigned)
{
pRange->lLimit = Limit(Limit::keConstant, 0);
}
break;

case GT_GT:
case GT_GE:
pRange->lLimit = limit;
// it doesn't matter if it's isUnsigned or not here - it's not negative anyway.
break;

case GT_EQ:
Expand Down
Loading

0 comments on commit e4dde6b

Please sign in to comment.