-
Notifications
You must be signed in to change notification settings - Fork 5.2k
JIT: Always create FIELD_LIST
for struct args in physical promotion
#118778
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
JIT: Always create FIELD_LIST
for struct args in physical promotion
#118778
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
…ed-call-arg-fields
Spot checked some regressions. Mostly they happen when we start partially promoting a parameter, leading to some more spilling now. This is a point-in-time problem. It will be improved with #112740 and even more once we start using ABI information to make promotion decisions (e.g. a struct being passed in registers should result in promoting it as fields compatible with those registers). |
/azp run Fuzzlyn |
Azure Pipelines successfully started running 1 pipeline(s). |
…ed-call-arg-fields
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR simplifies physical promotion logic for struct arguments by always creating FIELD_LIST
nodes, matching the behavior already used for struct return values. Previously, struct arguments had additional constraints that are no longer necessary due to backend improvements.
Key changes:
- Removes complex logic that determined when struct arguments could be passed in registers
- Consolidates field list creation into a shared method for both arguments and returns
- Simplifies the costing model by removing argument-specific constraints
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/coreclr/jit/promotion.h | Updates method signatures to accept use parameter and adds new shared method declaration |
src/coreclr/jit/promotion.cpp | Removes complex argument handling logic, extracts shared field list creation method, and simplifies costing |
cc @dotnet/jit-contrib PTAL @EgorBo Diffs, diffs with old promotion disabled. Still dependent on tests above to run. Spot checked some regressions and they were because of the reason above. |
Some jitstress failures in the "Stress gtSplitTree" phase. Looking at it... |
…ed-call-arg-fields
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
Azure Pipelines successfully started running 2 pipeline(s). |
jitstress is looking good now. @EgorBo can you take a look? |
Today physical promotion will create
FIELD_LIST
for returned struct values. However, arguments did not get the same treatment and had constraints motivated by old limitations of the backend. Since those limitations are gone now we can just indiscriminately produceFIELD_LIST
in physical promotion.