[release/6.0] Fix loop cloning of array of struct with array field#67496
[release/6.0] Fix loop cloning of array of struct with array field#67496carlossanlop merged 1 commit intodotnet:release/6.0from
Conversation
Loop cloning needs to parse what morph creates from GT_INDEX nodes to determine if there are array accesses with bounds checks that could potentially be optimized. For jagged array access, this can be a "comma chain" of bounds checks and array element address expressions. For a case where an array of structs had a struct field, such as `ValueTuple<int[], int>[]`, cloning was confusing the expression `a[i].Item1[j]` for the jagged array access `a[i][j]`. The fix here is to keep track of the type of the `GT_INDEX` node that is being morphed, in the `GT_BOUNDS_CHECK` node that is created for it. (This is the only thing cloning parses, to avoid the need to parse the very complex trees morph can create.) This type is then checked when parsing the "comma chain" trees. If a non-`TYP_REF` is found (such as a `TYP_STRUCT` in the above example), no more levels of array indexing are considered. (`TYP_REF` is what an array object would have, for a jagged array.) Fixes dotnet#66254.
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue Details[Port #67130 to release/6.0] Loop cloning needs to parse what morph creates from GT_INDEX nodes The fix here is to keep track of the type of the Fixes #66254.
|
|
/azp run runtime-coreclr outerloop, runtime-coreclr jitstress |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
@BruceForstall the PR's message might need to follow the Servicing Pull Request template. |
|
The jitstress items are all known issues or infra (out of disk space). |
jeffschwMSFT
left a comment
jeffschwMSFT
left a comment
There was a problem hiding this comment.
Approved. We will take for consideration in 6.0.x
leecow
added
Servicing-approved
ghost
locked as resolved and limited conversation to collaborators
6 participants
Fixes Issue #66254
main PR #67130
Description
The JIT compiler "loop cloning" optimization was incorrectly treating an array-of-structs as an array-of-arrays if one of the fields of the struct was an array. E.g., for
ValueTuple<int[], int>[], cloning was confusing the expressiona[i].Item1[j]as a jagged array accessa[i][j].The fix prevents this by detecting if the array element type is not what is expected, when parsing each level of the array indexing expression.
Customer Impact
This is silent bad code generation, so the JIT generates incorrect code that likely leads to a crash.
Regression
Yes, this is a regression in .NET 6.
Testing
Multiple JIT stress runs; new regression test; manual testing.
Verified the failure in release/6.0 before the fix, and success after the fix.
Risk
Low; the fix prevents an optimization from occurring, but doesn't introduce any new optimizations.