This repository was archived by the owner on Jan 23, 2023. It is now read-only.
JIT: add extra check to struct of struct of x promotion #19156
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Avoid promoting structs that contain struct fields that themselves wrap single simple fields, if those single simple fields are smaller than their enclosing struct. Otherwise we run the risk of losing track of the "extra" bytes in the innner struct. Addresses #19149.
|
@briansull PTAL @mgravell if you have bigger or other test cases around, feel free to give this a try or send me pointers. No diffs for pmi across x64 fx. Still need to add test case and look at whether there's something that we could do to flag the potential off the end access (so we could keep promotion but make it dependent, perhaps?). |
erozenfeld
approved these changes
Jul 27, 2018
src/jit/lclvars.cpp
Outdated
| if (fieldSize != innerStructSize) | ||
| { | ||
| JITDUMP("Promotion blocked: struct contains struct field with one field," | ||
| "but that field is not the same size as its parent."); |
|
Reran jit-diffs across all pri0 tests... including the new test. It's the only case with diffs. |
|
All these new failures seem unrelated (AVX_ro and infratructure). The first run had the same code, the new commit fixed a a comment and added a test case. |
|
I am tracking the CC. @dotnet/dnceng on the Out of Space issue that occurred for the Ubuntu run. |
|
Am going to ignore these recent failures. |
AndyAyersMS
added a commit
to AndyAyersMS/coreclr
that referenced
this pull request
Jul 31, 2018
Port of dotnet#19156. Avoid promoting structs that contain struct fields that themselves wrap single simple fields, if those single simple fields are smaller than their enclosing struct. Otherwise we run the risk of losing track of the "extra" bytes in the innner struct. Addresses #19149.
gbalykov
pushed a commit
to gbalykov/coreclr
that referenced
this pull request
Aug 9, 2018
Port of dotnet#19156. Avoid promoting structs that contain struct fields that themselves wrap single simple fields, if those single simple fields are smaller than their enclosing struct. Otherwise we run the risk of losing track of the "extra" bytes in the innner struct. Addresses #19149.
jashook
pushed a commit
to jashook/coreclr
that referenced
this pull request
Aug 14, 2018
Avoid promoting structs that contain struct fields that themselves wrap single simple fields, if those single simple fields are smaller than their enclosing struct. Otherwise we run the risk of losing track of the "extra" bytes in the innner struct. Addresses #19149.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Avoid promoting structs that contain struct fields that themselves
wrap single simple fields, if those single simple fields are smaller
than their enclosing struct.
Otherwise we run the risk of losing track of the "extra" bytes in the
inner struct.
Addresses #19149.