More precise range assertions for GT_CNS_INT#130385
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR refines the range computation for integral constant nodes (GT_CNS_INT/GT_CNS_LNG) in JIT assertion propagation by selecting a narrower rangeType based on the constant’s magnitude, enabling tighter derived non-negative range assertions.
Changes:
- Replace coarse
FitsIn<int32_t>/FitsIn<uint32_t>range typing with a more granular threshold-based selection (8/16/32-bit signed/unsigned) for constants. - Use the selected narrower type’s maximum to tighten the inferred upper bound when the constant is known non-negative.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/coreclr/jit/assertionprop.cpp:346
- The
GT_CNS_INT/GT_CNS_LNGswitch case no longer returns or breaks after computingrangeType. This causes an unintended fallthrough intocase GT_QMARK, wherenode->AsQmark()will be called on a constant node (invalid cast / potential crash). Add the missingreturn(or abreakplus an appropriate return) after selecting the tighterrangeType.
}
case GT_QMARK:
return Union(ForNode(node->AsQmark()->ThenNode(), compiler),
ForNode(node->AsQmark()->ElseNode(), compiler));
503f60c to
3523185
Compare
| if (constValue < 0) | ||
| { | ||
| rangeType = TYP_INT; | ||
| // We don't try and reason about lower bounds here to simplify the logic. | ||
| break; |
There was a problem hiding this comment.
I will word this better.
|
SuperPMI status isn't loading for me now, but diffs looked small but positive. We have cases where we were emitting overflow checks for small constants that provably can't overflow, and this helps with some of those. The one concern I had was there appears to be a regression in the |
Fixes #121400; We can use a simple binary search approach to narrow the upper bound for GT_CNS_INT type nodes in assertionprop. Credit for the approach to the comment here: #120980 (comment)