Remove ArrLen propagation from earlyprop#116410
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
I remember the ArrLen early propagation also contributes to loop optimizations like unrolling, which happens before VN. |
Loop unrolling happens before early prop |
c55facc to
6cb229d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR removes the array length propagation feature from the early propagation phase and cleans up related debugging and null-check folding logic. Key changes include:
- Removal of the SSA-based array length propagation and associated debugging/SSA update code.
- Updating constant range checks to use CORINFO_Array_MaxLength instead of INT32_MAX.
- Refactoring the null-check folding functions and the signature of optCanMoveNullCheckPastTree to remove an unused parameter.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/jit/earlyprop.cpp | Removed array length propagation code and refactored debug output and null-check handling. |
| src/coreclr/jit/compiler.h | Updated function signature for optCanMoveNullCheckPastTree to match the refactored implementation. |
Comments suppressed due to low confidence (2)
src/coreclr/jit/earlyprop.cpp:217
- The change from INT32_MAX to CORINFO_Array_MaxLength for the upper bound check is a critical update; please verify that CORINFO_Array_MaxLength is defined consistently and used appropriately across all relevant platforms.
if ((actualConstVal < 0) || (actualConstVal > CORINFO_Array_MaxLength))
src/coreclr/jit/earlyprop.cpp:262
- The removal of the tree cloning, SSA updates, and the associated constant propagation logic in optEarlyPropRewriteTree significantly changes the propagation strategy; please ensure that the updated value numbering phase fully addresses the necessary optimizations.
JITDUMP("optEarlyProp Rewriting " FMT_BB "\n", compCurBB->bbNum);
|
PTAL @jakobbotsch @dotnet/jit-contrib a few diffs I've removed GT_ARR_LENGTH forward substitution (SSA-based) from early-prop, the diffs imply it was not useful anyway. Also, removed a few dead vars/args. There is still a big chunk of code in the early prop responsible for constant-folding of GT_BOUNDS_CHECK (when index is constant and the length is constant at its SSA def). |
It looks unnecessary given all downstream phases are fine with just Value Numbering
Diffs