Skip to content

Conversation

@jakobbotsch
Copy link
Member

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 produce FIELD_LIST in physical promotion.

@jakobbotsch jakobbotsch added this to the 11.0.0 milestone Aug 15, 2025
@github-actions github-actions bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Aug 15, 2025
@dotnet-policy-service
Copy link
Contributor

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

@jakobbotsch
Copy link
Member Author

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).

@jakobbotsch
Copy link
Member Author

/azp run Fuzzlyn

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jakobbotsch jakobbotsch marked this pull request as ready for review September 2, 2025 08:42
Copilot AI review requested due to automatic review settings September 2, 2025 08:42
Copy link
Contributor

Copilot AI left a 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

@jakobbotsch
Copy link
Member Author

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.

@jakobbotsch jakobbotsch requested a review from EgorBo September 2, 2025 08:47
@jakobbotsch
Copy link
Member Author

Some jitstress failures in the "Stress gtSplitTree" phase. Looking at it...

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jakobbotsch
Copy link
Member Author

/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jakobbotsch
Copy link
Member Author

jitstress is looking good now. @EgorBo can you take a look?

@jakobbotsch jakobbotsch merged commit 308613e into dotnet:main Sep 4, 2025
135 of 137 checks passed
@jakobbotsch jakobbotsch deleted the physical-promoted-call-arg-fields branch September 4, 2025 09:11
@github-actions github-actions bot locked and limited conversation to collaborators Oct 5, 2025
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants