Have ComputeRange call into GetRangeFromAssertions for non dependent/symbolic cases#128922
Have ComputeRange call into GetRangeFromAssertions for non dependent/symbolic cases#128922tannergooding wants to merge 1 commit into
ComputeRange call into GetRangeFromAssertions for non dependent/symbolic cases#128922Conversation
github-actions
Bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR adjusts JIT range analysis so RangeCheck::ComputeRange can leverage GetRangeFromAssertions for additional TYP_INT-actual expressions (i.e., cases that aren’t dependent/symbolic/explicitly handled), and improves integral-constant extraction by avoiding ssize_t truncation.
Changes:
- Update
ValueNumStore::IsVNIntegralConstantto coerce constants viaint64_t(avoids truncation on 32-bit / for wider constants). - Rework
RangeCheck::ComputeRangeconstant handling to useIsVNIntegralConstant. - Route remaining
genActualType(expr) == TYP_INTcases inComputeRangethroughGetRangeFromAssertions(and remove the prior small-type / cast type-based fallbacks).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/valuenum.h | Fix integral constant extraction to use int64_t coercion before FitsIn<T> checks. |
| src/coreclr/jit/rangecheck.cpp | Use IsVNIntegralConstant for constants and call GetRangeFromAssertions for remaining TYP_INT-actual expressions. |
…ent/symbolic cases
tannergooding
force-pushed
the
computerange-calls-fromassertprop
branch
from
ebb0766 to
e41eb09
Compare
|
diffs most of the TP hit seems to be from this change, but it does get some decent wins. |
| { | ||
| // TODO: We could return `0, keUnknown` if the constant is known positive | ||
| // but this would require more handling in other places to take advantage of. | ||
| range = Limit(Limit::keUnknown); |
There was a problem hiding this comment.
I don't think we can TBH, a never-negative long that doesn't fit into INT32 can be negative when casted to INT32?
| // We want to handle constants first since it can avoid other more expensive work | ||
|
|
||
| int cns; | ||
| if (m_compiler->vnStore->IsVNIntegralConstant(vn, &cns)) |
There was a problem hiding this comment.
nit: we do double lookup here: IsVNConstant and then IsVNIntegralConstant
This is a smaller change from #128906 that doesn't involve more complex handling around
TYP_LONG