[release/8.0-staging] JIT: Home float parameters before integer parameters #98749
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Parameters that are going into float registers can come from integer registers in the presence of struct promotion. We need to home those before integer parameters or the source register could have been overridden by the integer parameter homing logic. Ideally it seems like the homing logic should be unified to handle all parameters simultaneously, but this seems like a simple enough fix. I do not think we have ABIs where we have the opposite kind constraint (integer parameters coming from float registers). Fix #96306
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsBackport of #96439 to release/8.0-staging /cc @jakobbotsch Customer Impact
[Select one or both of the boxes. Describe how this issue impacts customers, citing the expected and actual behaviors and scope of the issue. If customer-reported, provide the issue number.] Regression
[If yes, specify when the regression was introduced. Provide the PR or commit if known.] Testing[How was the fix verified? How was the issue missed previously? What tests were added?] Risk[High/Medium/Low. Justify the indication by mentioning how risks were measured and addressed.] IMPORTANT: If this backport is for a servicing release, please verify that:
|
This comment was marked as outdated.
This comment was marked as outdated.
|
Friendly reminder that Monday March 11th is the Code Complete date for the April Release. If you want this change to be included in that release, please make sure to mark this as ready for review, get an area owner sign-off, confirm the CI is green or failures are unrelated, and send an email to Tactics requesting approval. @jakobbotsch @AndyAyersMS |
|
I pushed another change motivated by validation done in #99286. It ensures that we never try to home a parameter passed in a float register in an integer register, since that's not supported when we home float registers first. This case could happen because we allow enregistration of structs of fitting size in integer registers if they have only whole uses. It is a very rare case because such enregistration only kicks in when the struct parameter also was not promoted. @AndyAyersMS Can you take another look? cc @dotnet/jit-contrib |
jakobbotsch
added
Servicing-approved
|
Approved by tactics via email. The failures look like infra failures; going to retry them. |
|
CI green after retries. Merging! |
Backport of #96439 to release/8.0-staging
/cc @jakobbotsch
Customer Impact
On windows-x64 the JIT may corrupt the incoming value of a parameter in certain cases when optimizing. This requires a mix of
float/doubleparameters and a parameter of a struct type with a singlefloat,doubleorVector2field. Furthermore, it requires the JIT to decide to enregister the struct parameter's field in the same register as one of thefloatordoubleparameters are passed in. In these cases the JIT will corrupt the value of thefloatordoubleparameter.For example, a combination of parameters that can hit the problem is:
Reported by customers in #96306 and #98748.
Regression
The underlying issue exists for a long time; however, in .NET 8 we made promotion of these structs more likely to happen, which makes the issue more likely to happen.
Testing
Regression test added.
Risk
Low risk. This reorders the order in which float and integer/struct parameters are handled to make sure that no such conflict happens.