Skip to content

JIT: Use precise multireg arg check for old promotion #112883

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

Merged
merged 2 commits into from
Feb 25, 2025

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Feb 24, 2025

lvaIsMultiregStruct does not account for whether the argument was actually passed in multiple regs, it just checks if the type is a type that could sometimes be passed in multiple registers. Change this to a precise check. Add a quirk for SysV where it seems the previous check provided a somewhat good heuristic.

`lvaIsMultiregStruct` does not account for whether the argument was
_actually_ passed in multiple regs, it just checks if the type is a type
that could sometimes be passed in multiple registers. Change this to a
precise check.
@Copilot Copilot AI review requested due to automatic review settings February 24, 2025 21:33
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.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 24, 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

cc @dotnet/jit-contrib PTAL @EgorBo

Diffs.
Should make #112884 zero-diff.

@jakobbotsch jakobbotsch requested a review from EgorBo February 25, 2025 10:04
//
bool Compiler::StructPromotionHelper::IsSysVMultiRegType(ClassLayout* layout)
{
#ifdef UNIX_AMD64_ABI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not the case for arm64 (sysv/win)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not according to diffs... To be honest I did not spend too much time looking at why this turns out to be a good heuristic for SysV, since I hope to remove all of this in this or next release.

@jakobbotsch jakobbotsch merged commit e6c5bb4 into dotnet:main Feb 25, 2025
104 of 112 checks passed
@jakobbotsch jakobbotsch deleted the promotion-multireg-arg branch February 25, 2025 12:01
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 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