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

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jakobbotsch
Copy link
Member Author

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

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 Copilot AI review requested due to automatic review settings September 2, 2025 08:42
Copy link
Contributor

@Copilot 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

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jakobbotsch
Copy link
Member Author

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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