More cleanups in rangecheck/assertionprop#129325
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR simplifies RyuJIT range-check handling by removing an unused “no range checks” configuration path and deleting the GTF_CHK_INDEX_INBND flag-based, deferred bounds-check removal logic in assertion propagation, replacing it with direct removal that preserves operand side effects.
Changes:
- Remove
FEATURE_ENABLE_NO_RANGE_CHECKS/JitNoRngChksconfig plumbing. - Remove
GTF_CHK_INDEX_INBNDand theGT_COMMAspecial-case path used to defer redundantGT_BOUNDS_CHECKremoval. - Remove the
optRemoveStandaloneRangeCheckwrapper and update bounds-check removal in assertion prop to directly replace the node with a side-effect-preservingNOTHING/COMMA.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/jit/optimizer.cpp | Removes optRemoveStandaloneRangeCheck wrapper (standalone removal now handled differently). |
| src/coreclr/jit/jitconfigvalues.h | Deletes JitNoRngChks config entry (previously gated by a feature define). |
| src/coreclr/jit/gentree.h | Removes GTF_CHK_INDEX_INBND flag definition. |
| src/coreclr/jit/compiler.h | Removes declarations tied to deleted helpers (optRemoveStandaloneRangeCheck, optAssertionProp_Comma). |
| src/coreclr/jit/assertionprop.cpp | Removes deferred/comma-based bounds-check elimination and directly rewrites redundant GT_BOUNDS_CHECK to side-effect-only execution. |
| src/coreclr/inc/switches.h | Deletes the dormant FEATURE_ENABLE_NO_RANGE_CHECKS toggle comment/define. |
|
FYI I've seen people interested in enabling |
We've significantly improved the range check analysis over the last few years, so the need for elided checks should be much less relevant today. I'd suggest introducing helper calls that remove the checks and using them where the profiler points out a problem |
GTF_CHK_INDEX_INBNDand unconditionally always removeGT_BOUNDS_CHECKin assertprop when we canFEATURE_ENABLE_NO_RANGE_CHECKSandDOTNET_JitNoRngChks, these were never enabled by default and I don't think they have any value.