Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor codegen for "big" (> 4 fields) structs init #71565

Closed
hypeartist opened this issue Jul 1, 2022 · 6 comments · Fixed by #88090
Closed

Poor codegen for "big" (> 4 fields) structs init #71565

hypeartist opened this issue Jul 1, 2022 · 6 comments · Fixed by #88090
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Priority:2 Work that is important, but not critical for the release question Answer questions and provide assistance, not an issue with source code or documentation.
Milestone

Comments

@hypeartist
Copy link

hypeartist commented Jul 1, 2022

FWm4DESWIAACiUX

category:cq
theme:structs
skill-level:expert
cost:large
impact:large

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 1, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 1, 2022
@ghost
Copy link

ghost commented Jul 1, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

FWm4DESWIAACiUX

Author: hypeartist
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@jakobbotsch
Copy link
Member

This is essentially a duplicate of #6534 -- the JIT does not currently do promotion for structs with more than 4 fields. It is an area that I hope we will be improving in .NET 8.

@hypeartist
Copy link
Author

@jakobbotsch Is there any info about what are all those restrictions (#6494 (comment) especially the one about 4 fields limit) based on?

@SingleAccretion
Copy link
Contributor

The 4 fields limit is just a CQ heuristic. Some of the other restrictions represent unimplemented functionality.

@hypeartist
Copy link
Author

hypeartist commented Jul 1, 2022

@jakobbotsch @SingleAccretion On the second thought my guess is that the issue some way related to ABI stuff - particularly the fact that the number of args that could be passed in regs is 4

@hypeartist
Copy link
Author

hypeartist commented Jul 5, 2022

Another strangeness.
Method M1C vs M1A. Both operate with the same 'big' struct A:

image

PS: or even like this:

image

@JulieLeeMSFT JulieLeeMSFT added the question Answer questions and provide assistance, not an issue with source code or documentation. label Jul 5, 2022
@JulieLeeMSFT JulieLeeMSFT added this to the 7.0.0 milestone Jul 11, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 11, 2022
@jakobbotsch jakobbotsch modified the milestones: 7.0.0, 8.0.0 Jul 12, 2022
@jakobbotsch jakobbotsch added the Priority:2 Work that is important, but not critical for the release label Jun 20, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 27, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 29, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Priority:2 Work that is important, but not critical for the release question Answer questions and provide assistance, not an issue with source code or documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants