-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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: Transform multi-reg args to FIELD_LIST in physical promotion #104370
JIT: Transform multi-reg args to FIELD_LIST in physical promotion #104370
Conversation
This allows promoted fields to stay in registers when they are passed as multi-reg arguments.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
As a note: I think long term we would benefit from making For a more uniform representation we would also allow |
Before this change the JIT has two places where it may introduce temps during call args morphing: 1. Directly during `fgMorphArgs` as part of making struct copies for some struct args, like implicit byref args and other struct args requiring to be of `LCL_VAR` shape 2. During `EvalArgsToTemps` as part of making sure evaluation order is maintained while we get the call into the right shape with registers To make this work we have `CallArg::m_tmpNum` that communicates the local from 1 to 2; the responsibility of creating the actual `LCL_VAR` node to put in the late argument list was left to `EvalArgsToTemps` while `fgMorphArgs` would just change the early setup node to a store into the temporary. This PR changes it so that 1 directly introduces the right late node when it creates its temporary. That is, it puts the call argument into the right shape immediately and avoids relying on `EvalArgsToTemps` to create the local node in the late list. The benefit of that is that we no longer need to communicate the temporary as part of every `CallArg`. However, the motivation here is something else: as part of dotnet#104370 we may have arguments that need multiple temporaries to copy, so having things working in this way introduces complications for that.
Before this change the JIT has two places where it may introduce temps during call args morphing: 1. Directly during `fgMorphArgs` as part of making struct copies for some struct args, like implicit byref args and other struct args requiring to be of `LCL_VAR` shape 2. During `EvalArgsToTemps` as part of making sure evaluation order is maintained while we get the call into the right shape with registers To make this work we have `CallArg::m_tmpNum` that communicates the local from 1 to 2; the responsibility of creating the actual `LCL_VAR` node to put in the late argument list was left to `EvalArgsToTemps` while `fgMorphArgs` would just change the early setup node to a store into the temporary. This PR changes it so that 1 directly introduces the right late node when it creates its temporary. That is, it puts the call argument into the right shape immediately and avoids relying on `EvalArgsToTemps` to create the local node in the late list. The benefit of that is that we no longer need to communicate the temporary as part of every `CallArg`. However, the motivation here is something else: as part of dotnet#104370 we may have arguments that need multiple temporaries to copy, so having things working in this way introduces complications for that.
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-jit-experimental |
Azure Pipelines successfully started running 3 pipeline(s). |
…ion-field-list-args
Before this change the JIT has two places where it may introduce temps during call args morphing: 1. Directly during `fgMorphArgs` as part of making struct copies for some struct args, like implicit byref args and other struct args requiring to be of `LCL_VAR` shape 2. During `EvalArgsToTemps` as part of making sure evaluation order is maintained while we get the call into the right shape with registers To make this work we have `CallArg::m_tmpNum` that communicates the local from 1 to 2; the responsibility of creating the actual `LCL_VAR` node to put in the late argument list was left to `EvalArgsToTemps` while `fgMorphArgs` would just change the early setup node to a store into the temporary. This PR changes it so that 1 directly introduces the right late node when it creates its temporary. That is, it puts the call argument into the right shape immediately and avoids relying on `EvalArgsToTemps` to create the local node in the late list. The benefit of that is that we no longer need to communicate the temporary as part of every `CallArg`. However, the motivation here is something else: as part of #104370 we may have arguments that need multiple temporaries to copy, so having things working in this way introduces complications for that.
The current win-arm64 failures look like bad codegen by MSVC for some JIT code in Edit: here's a small repro: https://godbolt.org/z/PGd5d6K9d |
/azp run runtime-coreclr superpmi-diffs, runtime-coreclr jitstress, runtime-coreclr libraries-jitstress, runtime-jit-experimental |
Azure Pipelines successfully started running 4 pipeline(s). |
cc @dotnet/jit-contrib PTAL @EgorBo |
Ping @EgorBo |
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.
LGTM
This allows promoted fields to stay in registers when they are passed as
multi-reg arguments.
Example linux-x64 diff with
DOTNET_JitStressModeNames=STRESS_NO_OLD_PROMOTION
:Base:
Diff:
I don't expect super large diffs with old promotion enabled because most structs that are candidates to be multi-register arguments are handled just fine by old promotion. However, this moves us closer to being able to remove old promotion.