Skip to content

Commit

Permalink
JIT: Optimize range checks for a[i & C], a[i % c] and a[(i & c1)>>c2)…
Browse files Browse the repository at this point in the history
…] patterns (dotnet#1644)

* Optimize range checks for array[x & c]

* Handle more cases...

* Fix possible crash

* Formatting

* Allow only [0..31] range for RSH operator

* Address feedback

* Add tests, also handle GT_LSH

* Address feedback

* Add a comment
  • Loading branch information
erozenfeld authored Mar 4, 2020
1 parent ebfb4ca commit 5c68e94
Show file tree
Hide file tree
Showing 3 changed files with 1,439 additions and 3 deletions.
54 changes: 51 additions & 3 deletions src/coreclr/src/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -773,11 +773,53 @@ void RangeCheck::MergeAssertion(BasicBlock* block, GenTree* op, Range* pRange DE
// Compute the range for a binary operation.
Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool monIncreasing DEBUGARG(int indent))
{
assert(binop->OperIs(GT_ADD));
assert(binop->OperIs(GT_ADD, GT_AND, GT_RSH, GT_LSH, GT_UMOD));

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

if (binop->OperIs(GT_AND, GT_RSH, GT_LSH, GT_UMOD))
{
if (!op2->IsIntCnsFitsInI32())
{
// only cns is supported for op2 at the moment for &,%,<<,>> operators
return Range(Limit::keUnknown);
}

int icon = -1;
if (binop->OperIs(GT_AND))
{
// x & cns -> [0..cns]
icon = static_cast<int>(op2->AsIntCon()->IconValue());
}
else if (binop->OperIs(GT_UMOD))
{
// x % cns -> [0..cns-1]
icon = static_cast<int>(op2->AsIntCon()->IconValue()) - 1;
}
else if (binop->OperIs(GT_RSH, GT_LSH) && op1->OperIs(GT_AND) && op1->AsOp()->gtGetOp2()->IsIntCnsFitsInI32())
{
// (x & cns1) >> cns2 -> [0..cns1>>cns2]
int icon1 = static_cast<int>(op1->AsOp()->gtGetOp2()->AsIntCon()->IconValue());
int icon2 = static_cast<int>(op2->AsIntCon()->IconValue());
if ((icon1 >= 0) && (icon2 >= 0) && (icon2 < 32))
{
icon = binop->OperIs(GT_RSH) ? (icon1 >> icon2) : (icon1 << icon2);
}
}

if (icon < 0)
{
return Range(Limit::keUnknown);
}
Range range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, icon));
JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler->getAllocatorDebugOnly()));
return range;
}

// other operators are expected to be handled above.
assert(binop->OperIs(GT_ADD));

Range* op1RangeCached = nullptr;
Range op1Range = Limit(Limit::keUndef);
// Check if the range value is already cached.
Expand Down Expand Up @@ -1032,6 +1074,12 @@ bool RangeCheck::ComputeDoesOverflow(BasicBlock* block, GenTree* expr)
{
overflows = DoesBinOpOverflow(block, expr->AsOp());
}
// GT_AND, GT_UMOD, GT_LSH and GT_RSH don't overflow
// Actually, GT_LSH can overflow so it depends on the analysis done in ComputeRangeForBinOp
else if (expr->OperIs(GT_AND, GT_RSH, GT_LSH, GT_UMOD))
{
overflows = false;
}
// Walk through phi arguments to check if phi arguments involve arithmetic that overflows.
else if (expr->OperGet() == GT_PHI)
{
Expand Down Expand Up @@ -1117,8 +1165,8 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas
range = ComputeRangeForLocalDef(block, expr->AsLclVarCommon(), monIncreasing DEBUGARG(indent + 1));
MergeAssertion(block, expr, &range DEBUGARG(indent + 1));
}
// If add, then compute the range for the operands and add them.
else if (expr->OperGet() == GT_ADD)
// compute the range for binary operation
else if (expr->OperIs(GT_ADD, GT_AND, GT_RSH, GT_LSH, GT_UMOD))
{
range = ComputeRangeForBinOp(block, expr->AsOp(), monIncreasing DEBUGARG(indent + 1));
}
Expand Down
Loading

0 comments on commit 5c68e94

Please sign in to comment.