Skip to content
Draft
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
10 changes: 10 additions & 0 deletions src/coreclr/jit/clrjit.natvis
Original file line number Diff line number Diff line change
Expand Up @@ -271,4 +271,14 @@ Documentation for VS debugger format specifiers: https://learn.microsoft.com/vis
</Expand>
</Type>

<!-- RangeCheck -->

<Type Name="Limit">
<DisplayString Condition="type == Limit::LimitType::keConstant">const: {cns}</DisplayString>
</Type>

<Type Name="Range">
<DisplayString Condition="lLimit.type == Limit::LimitType::keConstant &amp;&amp; uLimit.type == Limit::LimitType::keConstant">const range: [{lLimit.cns}..{uLimit.cns}]</DisplayString>
</Type>

</AutoVisualizer>
109 changes: 38 additions & 71 deletions src/coreclr/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1170,7 +1170,7 @@ 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, GT_XOR, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL));
assert(binop->OperIs(GT_ADD, GT_OR, GT_XOR, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL));

// For XOR we only care about Log2 pattern for now
if (binop->OperIs(GT_XOR))
Expand Down Expand Up @@ -1204,79 +1204,24 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
std::swap(op1IsCns, op2IsCns);
}

// Special cases for binops where op2 is a constant
if (binop->OperIs(GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD))
// Special case for UMOD
if (binop->OperIs(GT_UMOD))
{
if (!op2IsCns)
if (op2IsCns)
{
// only cns is supported for op2 at the moment for &,%,<<,>> operators
return Range(Limit::keUnknown);
}

ssize_t op2Cns = vnStore->CoercedConstantValue<ssize_t>(op2VN);
if (!FitsIn<int>(op2Cns))
{
return Range(Limit::keUnknown);
}

int op1op2Cns = 0;
int icon = -1;
if (binop->OperIs(GT_AND))
{
// x & cns -> [0..cns]
icon = static_cast<int>(op2Cns);
}
else if (binop->OperIs(GT_UMOD))
{
// x % cns -> [0..cns-1]
icon = static_cast<int>(op2Cns) - 1;
}
else if (binop->OperIs(GT_RSH, GT_LSH) && op1->OperIs(GT_AND) &&
vnStore->IsVNIntegralConstant<int>(op1->AsOp()->gtGetOp2()->gtVNPair.GetConservative(), &op1op2Cns))
{
// (x & cns1) >> cns2 -> [0..cns1>>cns2]
int icon1 = op1op2Cns;
int icon2 = static_cast<int>(op2Cns);
if ((icon1 >= 0) && (icon2 >= 0) && (icon2 < 32))
{
icon = binop->OperIs(GT_RSH) ? (icon1 >> icon2) : (icon1 << icon2);
}
}
else if (binop->OperIs(GT_RSZ))
{
// (x u>> cns) -> [0..(x's max value >> cns)]
int shiftBy = static_cast<int>(op2->AsIntCon()->IconValue());
if (shiftBy < 0)
ssize_t op2Cns = vnStore->CoercedConstantValue<ssize_t>(op2VN);
if (FitsIn<int>(op2Cns) && (op2Cns >= 1))
{
return Range(Limit::keUnknown);
// x % cns -> [0..cns-1]
Range range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, static_cast<int>(op2Cns) - 1));
JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler));
return range;
}

int op1Width = (int)(genTypeSize(op1) * BITS_PER_BYTE);
if (shiftBy >= op1Width)
{
return Range(Limit(Limit::keConstant, 0));
}

// Calculate max possible value of op1, e.g. UINT_MAX for TYP_INT/TYP_UINT
uint64_t maxValue = (1ULL << op1Width) - 1;
icon = (int)(maxValue >> static_cast<int>(op2->AsIntCon()->IconValue()));
}

if (icon >= 0)
{
Range range(Limit(Limit::keConstant, 0), Limit(Limit::keConstant, icon));
JITDUMP("Limit range to %s\n", range.ToString(m_pCompiler));
return range;
}
// Generalized range computation not implemented for these operators
else if (binop->OperIs(GT_AND, GT_UMOD))
{
return Range(Limit::keUnknown);
}
return Range(Limit::keUnknown);
}

// other operators are expected to be handled above.
assert(binop->OperIs(GT_ADD, GT_MUL, GT_LSH, GT_RSH, GT_RSZ));
assert(!binop->OperIs(GT_UMOD, GT_XOR));

Range* op1RangeCached = nullptr;
Range op1Range = Limit(Limit::keUndef);
Expand Down Expand Up @@ -1326,6 +1271,7 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
assert(op2Range.IsValid());

Range r = Range(Limit::keUnknown);

if (binop->OperIs(GT_ADD))
{
r = RangeOps::Add(op1Range, op2Range);
Expand All @@ -1349,7 +1295,19 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
else if (binop->OperIs(GT_RSH))
{
r = RangeOps::ShiftRight(op1Range, op2Range);
JITDUMP("Right shift range: %s >> %s = %s\n", op1Range.ToString(m_pCompiler), op2Range.ToString(m_pCompiler),
JITDUMP("BinOp Right shift range: %s >> %s = %s\n", op1Range.ToString(m_pCompiler),
op2Range.ToString(m_pCompiler), r.ToString(m_pCompiler));
}
else if (binop->OperIs(GT_AND))
{
r = RangeOps::And(op1Range, op2Range);
JITDUMP("BinOp AND range: %s & %s = %s\n", op1Range.ToString(m_pCompiler), op2Range.ToString(m_pCompiler),
r.ToString(m_pCompiler));
}
else if (binop->OperIs(GT_OR))
{
r = RangeOps::Or(op1Range, op2Range);
JITDUMP("BinOp OP range: %s | %s = %s\n", op1Range.ToString(m_pCompiler), op2Range.ToString(m_pCompiler),
r.ToString(m_pCompiler));
}

Expand Down Expand Up @@ -1547,6 +1505,15 @@ bool RangeCheck::DoesBinOpOverflow(BasicBlock* block, GenTreeOp* binop, const Ra
Range convertedOp2Range = RangeOps::ConvertShiftToMultiply(*op2Range);
return MultiplyOverflows(op1Range->UpperLimit(), convertedOp2Range.UpperLimit());
}
if (binop->OperIs(GT_OR))
{
// OR never overflows if both operands are non-negative.
if (op1Range->LowerLimit().IsConstant() && op2Range->LowerLimit().IsConstant() &&
(op1Range->LowerLimit().GetConstant() >= 0) && (op2Range->LowerLimit().GetConstant() >= 0))
{
return false;
}
}

return true;
}
Expand Down Expand Up @@ -1653,8 +1620,8 @@ bool RangeCheck::ComputeDoesOverflow(BasicBlock* block, GenTree* expr, const Ran
{
overflows = DoesVarDefOverflow(block, expr->AsLclVarCommon(), range);
}
// Check if add overflows.
else if (expr->OperIs(GT_ADD, GT_MUL, GT_LSH))
// Check if these binary ops overflow.
else if (expr->OperIs(GT_ADD, GT_MUL, GT_LSH, GT_OR))
{
overflows = DoesBinOpOverflow(block, expr->AsOp(), range);
}
Expand Down Expand Up @@ -1755,7 +1722,7 @@ Range RangeCheck::ComputeRange(BasicBlock* block, GenTree* expr, bool monIncreas
MergeAssertion(block, expr, &range DEBUGARG(indent + 1));
}
// compute the range for binary operation
else if (expr->OperIs(GT_XOR, GT_ADD, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL))
else if (expr->OperIs(GT_XOR, GT_OR, GT_ADD, GT_AND, GT_RSH, GT_RSZ, GT_LSH, GT_UMOD, GT_MUL))
{
range = ComputeRangeForBinOp(block, expr->AsOp(), monIncreasing DEBUGARG(indent + 1));
}
Expand Down
92 changes: 81 additions & 11 deletions src/coreclr/jit/rangecheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,17 @@ struct RangeOps
return Limit(Limit::keUnknown);
}

// Perform 'value' & 'cns'
static Limit AndByConstant(const Limit& value, const Limit& cns)
{
assert(cns.IsConstant());
if (value.IsConstant())
{
return Limit(Limit::keConstant, value.GetConstant() & cns.GetConstant());
}
return Limit(Limit::keConstant, cns.GetConstant());
}

// Given two ranges "r1" and "r2", perform a generic 'op' operation on the ranges.
template <typename Operation>
static Range ApplyRangeOp(Range& r1, Range& r2, Operation op)
Expand Down Expand Up @@ -437,35 +448,94 @@ struct RangeOps
static Range ShiftRight(Range& r1, Range& r2)
{
Limit& r1lo = r1.LowerLimit();
Limit& r1hi = r1.UpperLimit();
Limit& r2lo = r2.LowerLimit();

Limit& r1hi = r1.UpperLimit();
Limit& r2hi = r2.UpperLimit();

Range result = Limit(Limit::keUnknown);

// For now we only support r1 >> positive_cns (to simplify)
if (!r2lo.IsConstant() || !r2hi.IsConstant() || (r2lo.cns < 0) || (r2hi.cns < 0))
if (!r2lo.IsConstant() || !r2hi.IsConstant() || (r2lo.GetConstant() < 0) || (r2hi.GetConstant() < 0))
{
return result;
}

// Check lo ranges if they are dependent and not unknown.
if (r1lo.IsDependent())
if (r1lo.IsConstant())
{
result.lLimit = Limit(Limit::keDependent);
// Lower limit is CNS1 >> CNS2 = CNS3
result.lLimit = ShiftRightConstantLimit(r1lo, r2lo);
}

if (r1hi.IsConstant())
{
// Upper limit is CNS1 >> CNS2 = CNS3
result.uLimit = ShiftRightConstantLimit(r1hi, r2hi);
}
else if (r1lo.IsConstant())
else if (result.lLimit.IsConstant() && result.lLimit.GetConstant() >= 0 && r2hi.GetConstant() <= 31)
{
result.lLimit = ShiftRightConstantLimit(r1lo, r2lo);
// Since r1hi is effectively [0..int32.MaxValue] and we're performing >> by a constant ([0..31])
// we can compute the max upper bound.
result.uLimit = Limit(Limit::keConstant, INT32_MAX >> r2hi.GetConstant());
}

return result;
}

static Range And(Range& r1, Range& r2)
{
Limit& r1lo = r1.LowerLimit();
Limit& r2lo = r2.LowerLimit();

Limit& r1hi = r1.UpperLimit();
Limit& r2hi = r2.UpperLimit();

Range result = Limit(Limit::keUnknown);

// AND is a commutative operation, put the constant (if any) on the right.
if (r1lo.IsConstant())
{
std::swap(r1lo, r2lo);
}
if (r1hi.IsConstant())
{
std::swap(r1hi, r2hi);
}

if (r1hi.IsDependent())
if (r2lo.IsConstant() && (r2lo.GetConstant() >= 0))
{
result.uLimit = Limit(Limit::keDependent);
result.lLimit = AndByConstant(r1lo, r2lo);
}
else if (r1hi.IsConstant())
if (r2hi.IsConstant() && (r2hi.GetConstant() >= 0))
{
result.uLimit = ShiftRightConstantLimit(r1hi, r2hi);
result.uLimit = AndByConstant(r1hi, r2hi);
}
return result;
}

static Range Or(Range& r1, Range& r2)
{
Limit& r1lo = r1.LowerLimit();
Limit& r2lo = r2.LowerLimit();

Limit& r1hi = r1.UpperLimit();
Limit& r2hi = r2.UpperLimit();

Range result = Limit(Limit::keUnknown);

// Currently, we only handle the case when all limits are positive constants.
if (!r1lo.IsConstant() || !r2lo.IsConstant() || !r1hi.IsConstant() || !r2hi.IsConstant())
{
return result;
}

if ((r1lo.GetConstant() >= 0) && (r2lo.GetConstant() >= 0))
{
result.lLimit = Limit(Limit::keConstant, r1lo.GetConstant() | r2lo.GetConstant());
}
if ((r1hi.GetConstant() >= 0) && (r2hi.GetConstant() >= 0))
{
result.uLimit = Limit(Limit::keConstant, r1hi.GetConstant() | r2hi.GetConstant());
}

return result;
Expand Down
76 changes: 37 additions & 39 deletions src/libraries/System.Private.CoreLib/src/System/Convert.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2555,53 +2555,51 @@ private static unsafe int ConvertToBase64Array(char* outChars, byte* inData, int
int i;

// get a pointer to the base64 table to avoid unnecessary range checking
fixed (byte* base64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/="u8)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove unsafe from ConvertToBase64Array's two callers?

ReadOnlySpan<byte> base64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/="u8;
for (i = offset; i < calcLength; i += 3)
{
for (i = offset; i < calcLength; i += 3)
if (insertLineBreaks)
{
if (insertLineBreaks)
if (charcount == Base64LineBreakPosition)
{
if (charcount == Base64LineBreakPosition)
{
outChars[j++] = '\r';
outChars[j++] = '\n';
charcount = 0;
}
charcount += 4;
outChars[j++] = '\r';
outChars[j++] = '\n';
charcount = 0;
}
outChars[j] = (char)base64[(inData[i] & 0xfc) >> 2];
outChars[j + 1] = (char)base64[((inData[i] & 0x03) << 4) | ((inData[i + 1] & 0xf0) >> 4)];
outChars[j + 2] = (char)base64[((inData[i + 1] & 0x0f) << 2) | ((inData[i + 2] & 0xc0) >> 6)];
outChars[j + 3] = (char)base64[inData[i + 2] & 0x3f];
j += 4;
charcount += 4;
}
outChars[j] = (char)base64[(inData[i] & 0xfc) >> 2];
outChars[j + 1] = (char)base64[((inData[i] & 0x03) << 4) | ((inData[i + 1] & 0xf0) >> 4)];
outChars[j + 2] = (char)base64[((inData[i + 1] & 0x0f) << 2) | ((inData[i + 2] & 0xc0) >> 6)];
outChars[j + 3] = (char)base64[inData[i + 2] & 0x3f];
j += 4;
}

// Where we left off before
i = calcLength;
// Where we left off before
i = calcLength;

if (insertLineBreaks && (lengthmod3 != 0) && (charcount == Base64LineBreakPosition))
{
outChars[j++] = '\r';
outChars[j++] = '\n';
}
if (insertLineBreaks && (lengthmod3 != 0) && (charcount == Base64LineBreakPosition))
{
outChars[j++] = '\r';
outChars[j++] = '\n';
}

switch (lengthmod3)
{
case 2: // One character padding needed
outChars[j] = (char)base64[(inData[i] & 0xfc) >> 2];
outChars[j + 1] = (char)base64[((inData[i] & 0x03) << 4) | ((inData[i + 1] & 0xf0) >> 4)];
outChars[j + 2] = (char)base64[(inData[i + 1] & 0x0f) << 2];
outChars[j + 3] = (char)base64[64]; // Pad
j += 4;
break;
case 1: // Two character padding needed
outChars[j] = (char)base64[(inData[i] & 0xfc) >> 2];
outChars[j + 1] = (char)base64[(inData[i] & 0x03) << 4];
outChars[j + 2] = (char)base64[64]; // Pad
outChars[j + 3] = (char)base64[64]; // Pad
j += 4;
break;
}
switch (lengthmod3)
{
case 2: // One character padding needed
outChars[j] = (char)base64[(inData[i] & 0xfc) >> 2];
outChars[j + 1] = (char)base64[((inData[i] & 0x03) << 4) | ((inData[i + 1] & 0xf0) >> 4)];
outChars[j + 2] = (char)base64[(inData[i + 1] & 0x0f) << 2];
outChars[j + 3] = (char)base64[64]; // Pad
j += 4;
break;
case 1: // Two character padding needed
outChars[j] = (char)base64[(inData[i] & 0xfc) >> 2];
outChars[j + 1] = (char)base64[(inData[i] & 0x03) << 4];
outChars[j + 2] = (char)base64[64]; // Pad
outChars[j + 3] = (char)base64[64]; // Pad
j += 4;
break;
}

return j;
Expand Down
Loading