Update GetRangeFromAssertions to handle some basic TYP_LONG scenarios where it FitsIn<int32_t>#128906
Update GetRangeFromAssertions to handle some basic TYP_LONG scenarios where it FitsIn<int32_t>#128906tannergooding wants to merge 4 commits into
Conversation
… where it FitsIn<int32_t>
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR broadens assertion-based range derivation in the JIT so it can reason about some TYP_LONG value numbers when the resulting values are known to fit in int32, and wires the updated API through rangecheck and assertion propagation to enable additional folding / bounds-check reasoning.
Changes:
- Update
ValueNumStore::IsVNIntegralConstantto coerce constants asint64_t, allowingTYP_LONGconstants that fit to be recognized asint32constants. - Extend
RangeCheck::GetRangeFromAssertions/worker to accept an explicitvar_typesand add limited handling forTYP_LONGscenarios (notablyRSZ/RSHshift cases and other VN ops). - Update assertion propagation and range analysis callsites to pass the expression type and tolerate unknown ranges where
TYP_LONGcan’t be represented as anint32-basedRange.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/valuenum.h | Enables integral-constant extraction from TYP_LONG VNs via int64_t coercion. |
| src/coreclr/jit/rangecheck.h | Updates GetRangeFromAssertions signature and adds Range::IsUnknown() helper. |
| src/coreclr/jit/rangecheck.cpp | Implements the new typed assertion-range logic and extends range computation to consult it in more cases. |
| src/coreclr/jit/assertionprop.cpp | Adapts assertion-prop folding to the new API and to possibly-unknown ranges for wider types. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/coreclr/jit/rangecheck.cpp:796
- In VNF_Cast handling, when
resultis non-constant (e.g., casting to/from types that Range can’t represent) the code unconditionally propagatescastOpRangeif it’s a constant range. This is unsound for sign-changing casts (e.g.,int -> uint,uint -> long) when the operand range can include negatives: the cast changes negative values to large positives, butcastOpRangewould still contain negatives and exclude the large values.
This can cause incorrect tightening and downstream folding/removal based on a range that doesn’t describe the cast result.
// Now see if we can do better by looking at the cast source.
// if its range is within the castTo range, we can use that (and the cast is basically a no-op).
if (varTypeIsIntegral(arg0Typ))
{
Range castOpRange =
GetRangeFromAssertionsWorker(comp, arg0Typ, arg0VN, assertions, --budget, visited);
if (castOpRange.IsConstantRange())
{
if (!result.IsConstantRange())
{
result = castOpRange;
}
else if ((castOpRange.LowerLimit().GetConstant() >= result.LowerLimit().GetConstant()) &&
(castOpRange.UpperLimit().GetConstant() <= result.UpperLimit().GetConstant()))
{
result = castOpRange;
}
}
This is an alternative to #128676. It needs confirmation that the diffs/TP is acceptable and may require a few iterations or pulling back prior to it being ready for review.