Skip to content
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: Handle TYP_DOUBLE on arm softFP similarly to TYP_LONG #103869

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Jun 23, 2024

Avoids PUTARG_REG being a multi-reg node on arm32 to handle this special case.

The gist of the change: we expect multi-reg structs, including TYP_LONG on arm32/x86, to be put into a FIELD_LIST(PUTARG_REG(low), PUTARG_REG(high)) form before LSRA. For multi-reg structs this happens during morph, while for TYP_LONG we keep them as primitives until lowering, so it happens during lowering.

There was a special case for arm32 with softFP where TYP_DOUBLE also ends up being a similar multi-reg struct that is passed in two integer registers. There was a bunch of special casing to handle this in the backend, by making PUTARG_REG and BITCAST multireg nodes on arm32 only.
This PR instead puts TYP_DOUBLE into the expected multireg-struct form during lowering, allowing it to be handled like any other multireg struct.

Motivation is to avoid arm32-specific logic in #103866, which currently fails in CI for arm32 softFP because of this special case.

Avoids `PUTARG_REG` being a multi-reg node on arm32 to handle this
special case.
@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 Jun 23, 2024
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 @dotnet/samsung @kunalspathak

Copy link
Contributor

@tomeksowi tomeksowi left a comment

Choose a reason for hiding this comment

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

Thank you for the cleanup. LGTM, shouldn't affect RISC-V.

Copy link
Member

@clamp03 clamp03 left a comment

Choose a reason for hiding this comment

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

Thank you.
+ Checked coreclr test on armel tizen

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jakobbotsch jakobbotsch merged commit d0c8f83 into dotnet:main Jun 25, 2024
107 checks passed
@jakobbotsch jakobbotsch deleted the arm32-softfp-double-args branch June 25, 2024 12:59
@github-actions github-actions bot locked and limited conversation to collaborators Jul 26, 2024
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.

5 participants