Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
14 changes: 9 additions & 5 deletions src/coreclr/jit/assertionprop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1651,9 +1651,12 @@ void Compiler::optCreateComplementaryAssertion(AssertionIndex assertionIndex)
optMapComplementary(optAddAssertion(reversed), assertionIndex);
}
else if (candidateAssertion.KindIs(OAK_LT_UN, OAK_LE_UN) &&
candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS))
candidateAssertion.GetOp2().KindIs(O2K_CHECKED_BOUND_ADD_CNS, O2K_VN))
{
// Assertions such as "X > checkedBndVN" aren't very useful.
// Assertions such as "X u> checkedBndVN" or "X u> Y" aren't very useful: unsigned ">" /
// ">=" complementaries don't tighten ranges (they describe non-contiguous regions in the
// signed-int domain that Range can't represent) and rarely match a direct relop fold
// either. Skip the complementary to keep assertion-table pressure down.
return;
}
else if (AssertionDsc::IsReversible(candidateAssertion.GetKind()))
Expand Down Expand Up @@ -1825,15 +1828,16 @@ AssertionInfo Compiler::optCreateJTrueBoundsAssertion(GenTree* tree)
}

// "X relop Y" where neither side is a constant nor a checked bound.
// For now, we only create such assertions for signed comparisons of int32 (and smaller, after promotion).
// We create such assertions for both signed and unsigned comparisons of int32 (and smaller, after promotion).
// This widens what global assertion prop can reason about: e.g. "b > a" combined with "a > 10"
// can be used to deduce "b > 10".
// can be used to deduce "b > 10". The unsigned variant enables folding patterns like
// "(uint)(i + 1) <= (uint)len" given "(uint)i < (uint)len".
//
// To keep table pressure under control, we only create the assertion if at least one of the
// operands already has assertions registered. Otherwise the new assertion has no other facts
// it can chain with and is unlikely to enable any deduction, while still consuming a slot
// (and potentially crowding out useful ones).
if (!isUnsignedRelop && (op1VN != op2VN) && !vnStore->IsVNConstant(op1VN) && !vnStore->IsVNConstant(op2VN) &&
if ((op1VN != op2VN) && !vnStore->IsVNConstant(op1VN) && !vnStore->IsVNConstant(op2VN) &&
(optAssertionHasAssertionsForVN(op1VN) || optAssertionHasAssertionsForVN(op2VN)))
{
Comment on lines 1830 to 1842
AssertionDsc dsc = AssertionDsc::CreateRelopVN(this, relopFunc, op1VN, op2VN);
Expand Down
86 changes: 82 additions & 4 deletions src/coreclr/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,71 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
}
}
}

// Generic fold: "<cmp>(ADD(base, 1), other)" is implied by an asserted
// "base <relop> other" (with matching signedness). For an asserted strict
// less-than the implication "ADD(base, 1) <= other" is sound:
// * Signed: base < other => base <= INT32_MAX - 1 => base + 1 <= other
// (no signed overflow).
// * Unsigned: (uint)base < (uint)other => (uint)base <= UINT32_MAX - 1
// => (uint)(base + 1) <= (uint)other (no unsigned wrap).
// This catches patterns like "(uint)(offset + 1) <= (uint)length" given
// "(uint)offset < (uint)length", which is the typical "Slice(offset + 1)" check.
//
// Only "<=" / ">" can be concluded once normalized to "ADD(base, 1) <relop> other";
// "<", ">=", "==", "!=" remain undetermined. To keep the assertion-table scan
// proportional to cases that can benefit, require op1 to be ADD(base, 1) where
// base has assertions registered.
ValueNum addBase;
int addCns;
if (!result.IsSingleValueConstant() && ((cmpOper == GT_LE) || (cmpOper == GT_GT)) &&
comp->vnStore->IsVNBinFuncWithConst(funcApp.m_args[0], VNF_ADD, &addBase, &addCns) &&
(addCns == 1) && (addBase != funcApp.m_args[1]) &&
comp->optAssertionHasAssertionsForVN(addBase))
{
ValueNum otherVN = funcApp.m_args[1];
BitVecOps::Iter iter(comp->apTraits, assertions);
unsigned assertionBit = 0;
while (iter.NextElem(&assertionBit))
{
const Compiler::AssertionDsc& a =
comp->optGetAssertion(GetAssertionIndex(assertionBit));

// We only consume O2K_VN-style relop assertions; checked-bound assertions
// already have dedicated handling elsewhere.
if (!a.IsRelop() || !a.GetOp2().KindIs(Compiler::O2K_VN))
{
continue;
}

ValueNum aOp1 = a.GetOp1().GetVN();
ValueNum aOp2 = a.GetOp2().GetVN();
bool aIsUnsigned;
genTreeOps aCmp = Compiler::AssertionDsc::ToCompareOper(a.GetKind(), &aIsUnsigned);

// Normalize so that the assertion reads "addBase <aCmp> otherVN".
if ((aOp1 == otherVN) && (aOp2 == addBase))
{
aCmp = GenTree::SwapRelop(aCmp);
}
else if ((aOp1 != addBase) || (aOp2 != otherVN))
{
continue;
}

// Signedness must match: e.g. signed "addBase < other" doesn't imply
// "(uint)addBase != UINT32_MAX" (negative addBase has bit 31 set), so
// the no-overflow argument doesn't carry across signedness.
// Also require a strict less-than: only "<" gives "addBase + 1 <= other".
if ((aIsUnsigned != isUnsigned) || (aCmp != GT_LT))
{
continue;
}

result = Range(Limit(Limit::keConstant, (cmpOper == GT_LE) ? 1 : 0));
break;
}
}
}
break;
}
Expand Down Expand Up @@ -1411,10 +1476,6 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler* comp
ValueNum op2VN = curAssertion.GetOp2().GetVN();

cmpOper = Compiler::AssertionDsc::ToCompareOper(curAssertion.GetKind(), &isUnsigned);
if (isUnsigned)
{
continue;
}

ValueNum otherVN;
if (op1VN == normalLclVN)
Expand All @@ -1431,6 +1492,17 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler* comp
cmpOper = GenTree::SwapRelop(cmpOper);
}

// For unsigned compares, we can only derive a useful constant range when the
// direction is "<" or "<=". The asserted "X u< Y" implies X is in [0, Y_upper - 1]
// only when otherVN is known to be non-negative; otherwise (uint)Y could be huge
// and we cannot bound X within the signed-int domain. Skip unsigned ">/>=" since
// they would produce non-contiguous ranges (existing behavior at the tightening
// step below also assumes lLimit = 0 for unsigned LT/LE).
if (isUnsigned && (cmpOper != GT_LT) && (cmpOper != GT_LE))
{
continue;
}

if (budget <= 0)
{
continue;
Expand All @@ -1443,6 +1515,12 @@ void RangeCheck::MergeEdgeAssertionsWorker(Compiler* comp
continue;
}

if (isUnsigned && (otherRange.LowerLimit().GetConstant() < 0))
{
// For unsigned LT/LE we need otherVN's range to be non-negative.
continue;
}

// Derive a constant limit for normalLclVN from the constant range of otherVN.
// We use the most useful bound for each direction of the comparison.
int derivedLimit;
Expand Down
18 changes: 18 additions & 0 deletions src/tests/JIT/opt/RangeChecks/ElidedBoundsChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ static bool TryStripFirstChar(ref ReadOnlySpan<char> span, char value)
return false;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static ReadOnlySpan<char> SliceOffsetPlusOne(ReadOnlySpan<char> span, int offset)
{
// X64-NOT: ThrowArgumentOutOfRangeException
// ARM64-NOT: ThrowArgumentOutOfRangeException
if ((uint)offset < (uint)span.Length)
{
return span.Slice(offset + 1);
}
return span;
}

[Fact]
public static int TestEntryPoint()
{
Expand Down Expand Up @@ -139,6 +151,12 @@ public static int TestEntryPoint()
if (TryStripFirstChar(ref chars, 'h') != false)
return 0;

if (SliceOffsetPlusOne("hello".AsSpan(), 1).Length != 3)
return 0;

Comment thread
EgorBo marked this conversation as resolved.
if (SliceOffsetPlusOne("hello".AsSpan(), 100).Length != 5)
return 0;

return 100;
}
}
Loading