Skip to content
34 changes: 12 additions & 22 deletions src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10595,28 +10595,18 @@ GenTree* Compiler::fgOptimizeAddition(GenTreeOp* add)
if (op2->IsIntegralConst())
{
// ADD(NEG(x), CONST) => XOR(x, CONST)

auto isSubToXorValid = [=](uint64_t cns, IntegralRange range) {
// cns - x, where x in [lo, hi]
uint64_t lo = IntegralRange::SymbolicToRealValue(range.GetLowerBound());
uint64_t hi = IntegralRange::SymbolicToRealValue(range.GetUpperBound());

// OR of all numbers in [lo, hi]
uint64_t knownBits = (lo == hi) ? 0 : (UINT64_MAX >> BitOperations::LeadingZeroCount(lo ^ hi));
knownBits = lo | knownBits;

// Zero out bits outside of TYPE. This handles cases that rely on overflow
uint32_t sizeInBits = genTypeSize(add->TypeGet()) * BITS_PER_BYTE;
knownBits &= (1ULL << (sizeInBits - 1)) - 1;

// At every bit pos with a 1 in knownBits, cns also needs 1.
// Otherwise borrowing occurs and XOR is not equivalent to SUB
return (cns & knownBits) == knownBits;
};

IntegralRange range = IntegralRange::ForNode(op1->gtGetOp1(), this);
uint64_t cns = (uint64_t)op2->AsIntConCommon()->IntegralValue();
if (isSubToXorValid(cns, range))
uint64_t cns = (uint64_t)op2->AsIntConCommon()->IntegralValue();
IntegralRange range = IntegralRange::ForNode(op1->gtGetOp1(), this);
uint64_t lo = IntegralRange::SymbolicToRealValue(range.GetLowerBound());
uint64_t hi = IntegralRange::SymbolicToRealValue(range.GetUpperBound());
uint64_t knownBits = BitOperations::BitsetFromRange(lo, hi);

// Zero out bits outside of TYPE. This handles cases that rely on overflow (int.MaxValue - x)
uint32_t sizeInBits = genTypeSize(add->TypeGet()) * BITS_PER_BYTE;
knownBits &= (1ULL << (sizeInBits - 1)) - 1;

bool noCarry = (cns & knownBits) == knownBits;
if (noCarry)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what does this rewrite do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just factored out the BitsetFromRange since we now use it in morph and rangecheck and adjusted the code a little. We can use BitsetFromRange to do other stuff for example transform ADD to OR if we wanted.

{
add->SetOper(GT_XOR, GenTree::PRESERVE_VN);
add->gtOp1 = op1->gtGetOp1();
Expand Down
24 changes: 22 additions & 2 deletions src/coreclr/jit/rangecheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,9 +757,23 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
break;

case VNF_NEG:
case VNF_NOT:
{
Range r1 = GetRangeFromAssertionsWorker(comp, funcApp.m_args[0], assertions, --budget, visited);
Range unaryOpResult = RangeOps::Negate(r1);
Range unaryOpResult = Range(Limit(Limit::keUnknown));
switch (funcApp.m_func)
{
case VNF_NEG:
unaryOpResult = RangeOps::Negate(r1);
break;

case VNF_NOT:
unaryOpResult = RangeOps::Not(r1);
break;

default:
unreached();
}

// We can use the result only if it never overflows.
result = unaryOpResult.IsConstantRange() ? unaryOpResult : result;
Expand All @@ -772,6 +786,7 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
case VNF_SUB:
case VNF_AND:
case VNF_OR:
case VNF_XOR:
case VNF_RSH:
case VNF_RSZ:
case VNF_UMOD:
Expand All @@ -797,6 +812,9 @@ Range RangeCheck::GetRangeFromAssertionsWorker(
case VNF_OR:
binOpResult = RangeOps::Or(r1, r2);
break;
case VNF_XOR:
binOpResult = RangeOps::Xor(r1, r2);
break;
case VNF_LSH:
binOpResult = RangeOps::ShiftLeft(r1, r2);
break;
Expand Down Expand Up @@ -1680,7 +1698,9 @@ Range RangeCheck::ComputeRangeForBinOp(BasicBlock* block, GenTreeOp* binop, bool
{
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
// To handle the Log2 pattern of "63 ^ LZCNT(x | 1)" we are missing precise
// range info for "LZCNT(x | 1)" (should be [0, 63]) and 64bit support. Special case it:
// https://github.com/dotnet/runtime/pull/113790
if (binop->OperIs(GT_XOR))
{
int upperBound;
Expand Down
46 changes: 46 additions & 0 deletions src/coreclr/jit/rangecheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,46 @@ struct RangeOps
return Range(Limit(Limit::keUnknown));
}

static Range Xor(const Range& r1, const Range& r2)
Copy link
Copy Markdown
Member

@EgorBo EgorBo May 13, 2026

Choose a reason for hiding this comment

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

I checked locally and this entire thing has no impact on the SPMI diffs, do we need it then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did you run diffs with #126529 included?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It produces diffs in System.Number:TryParseHexFloatingPoint and System.Linq.Parallel.Tests.UnorderedSources:Default inside libraries_tests (check).
Before the commit that removed XOR handling from the TryGetRange path it was more I think. Anyway I'll just close this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it is better to close it indeed, the entire diffs is 12 contexts, almost all of them in libraries_tests_no_tiered_compilation while the logic is not trivial to verify, so it's mostly just +80 LOC of risky changes as I see it, unless there is a strong argument that it helped you to replace unsafe code with safe somewhere, etc.

{
int r1ConstVal;
int r2ConstVal;
bool r1IsConstVal = r1.IsSingleValueConstant(&r1ConstVal);
bool r2IsConstVal = r2.IsSingleValueConstant(&r2ConstVal);

// Both ranges are single constant values.
// Example: [5..5] ^ [3..3] = [6..6]
if (r1IsConstVal && r2IsConstVal)
{
return Range(Limit(Limit::keConstant, r1ConstVal ^ r2ConstVal));
}

auto isSubToXorValid = [=](uint64_t cns, Range range) {
uint64_t lo = (uint64_t)range.LowerLimit().GetConstant();
uint64_t hi = (uint64_t)range.UpperLimit().GetConstant();
uint64_t knownBits = BitOperations::BitsetFromRange(lo, hi);

// Zero out bits outside of TYPE. This handles cases that rely on overflow (int.MaxValue - x)
uint32_t sizeInBits = genTypeSize(TYP_INT) * BITS_PER_BYTE;
knownBits &= (1ULL << (sizeInBits - 1)) - 1;

// Can sub be perfomed without carry?
return (cns & knownBits) == knownBits;
};

// Example: [3..5] ^ [-1..-1] = [-6..-4]
if (r1IsConstVal && r2.IsConstantRange() && isSubToXorValid(r1ConstVal, r2))
{
return Subtract(r1, r2);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you trace for me what does r1 = [INT_MAX, INT_MAX], r2 = [-1, 1] return here

Copy link
Copy Markdown
Member

@EgorBo EgorBo May 7, 2026

Choose a reason for hiding this comment

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

I think we need to check Subtract's result for IsConstantRange (which checks IsValid as well) and return its value, otherwase unknown..unknown.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

can you trace for me what does r1 = [INT_MAX, INT_MAX], r2 = [-1, 1] return here

This is equivalent to sub so it will call Subtract(r1, r2).
It then calls Negate(r2) which returns [-1, 1].
It then calls Add(r1, [-1, 1]).
Inside that, CheckedOps::AddOverflows(2147483647, 1, false) fails and we return Limit(Limit::keUnknown)

}
if (r2IsConstVal && r1.IsConstantRange() && isSubToXorValid(r2ConstVal, r1))
{
return Subtract(r2, r1);
}

return Range(Limit(Limit::keUnknown));
}

static Range UnsignedMod(const Range& r1, const Range& r2)
{
// For X UMOD Y we only handle the case when Y is a fixed positive constant.
Expand Down Expand Up @@ -661,6 +701,12 @@ struct RangeOps
return result;
}

static Range Not(const Range& range)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

last I checked Not produced 0 diffs, so we'd better remove it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#126553 (comment) Here I was talking about 2 large diffs in libraries_tests_no_tiered_compilation caused by NOT handling.

Additionally it fixes regressions from my recent transformation (#126529).

Example regression in libraries.pmi in SqlDecimal:op_Multiply (diffs):
image

Which this fixes by adding NOT handling (diffs):
image

{
Range cns = Limit(Limit::keConstant, -1);
return Subtract(cns, range);
}

//------------------------------------------------------------------------
// EvalRelop: Evaluate the relation between two ranges for the given relop
// Example: "x >= y" is AlwaysTrue when "x.LowerLimit() >= y.UpperLimit()"
Expand Down
23 changes: 23 additions & 0 deletions src/coreclr/jit/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3601,6 +3601,29 @@ double BitOperations::UInt64BitsToDouble(uint64_t value)
return result;
}

//------------------------------------------------------------------------
// BitOperations::BitsetFromRange: Gets a bitset from OR'ing all numbers in [lo, hi]
//
// Arguments:
// lo - The range minimum.
// hi - The range maximum.
//
// Return Value:
// The bitset
//
uint64_t BitOperations::BitsetFromRange(uint64_t lo, uint64_t hi)
{
if (lo == hi)
{
return lo;
}

uint64_t mask = UINT64_MAX >> BitOperations::LeadingZeroCount(lo ^ hi);
mask = lo | mask;

return mask;
}

namespace MagicDivide
{
template <int TableBase = 0, int TableSize, typename Magic>
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,8 @@ class BitOperations
static float UInt32BitsToSingle(uint32_t value);

static double UInt64BitsToDouble(uint64_t value);

static uint64_t BitsetFromRange(uint64_t lo, uint64_t hi);
};

// The CLR requires that critical section locks be initialized via its ClrCreateCriticalSection API...but
Expand Down
Loading