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

[RISC-V][LoongArch64] Pass FP struct fields at arbitrary offsets in ArgIterator and CallDescrWorker #105800

Merged
merged 11 commits into from
Aug 10, 2024

Conversation

tomeksowi
Copy link
Contributor

Use new info calculated in #103945 on the VM side (shuffling thunks will be handled in a separate PR).

Stems from #101796, part of #84834, cc @dotnet/samsung @shushanhf @LuckyXu-HF

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 1, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shushanhf @LuckyXu-HF PTAL, I don't really know LA asm and I had no way to test it..

@tomeksowi
Copy link
Contributor Author

@MichalStrehovsky @jkotas can anyone review please?

@jkotas
Copy link
Member

jkotas commented Aug 5, 2024

@dotnet/samsung @shushanhf @LuckyXu-HF Could you please review and sign-off?

Copy link
Contributor

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

Copy link
Member

@sirntar sirntar left a comment

Choose a reason for hiding this comment

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

I left review of the LA64 parts for @shushanhf and/or @LuckyXu-HF, but as for the RV64, everything looks good to me.

@tomeksowi I saw that the EmptyStructs test fails, but I'm not sure if this pr was supposed to solve this problem (?).

@shushanhf
Copy link
Contributor

OK, Thanks very much.
I will test it based on the latest patch right now.

src/coreclr/vm/callingconvention.h Outdated Show resolved Hide resolved
Co-authored-by: Qiao Pengcheng <qiaopengcheng@loongson.cn>
Copy link
Contributor

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

@tomeksowi
Copy link
Contributor Author

tomeksowi commented Aug 9, 2024

@tomeksowi I saw that the EmptyStructs test fails, but I'm not sure if this pr was supposed to solve this problem (?).

This PR no, but #106112 disables them where not supported.

@risc-vv
Copy link

risc-vv commented Aug 9, 2024

3985f11 is being scheduled for building and testing

GIT: 3985f11e825beafbc9fd591d97942c4d69753333
REPO: dotnet/runtime
BRANCH: main

@risc-vv
Copy link

risc-vv commented Aug 9, 2024

88815d2 is being scheduled for building and testing

GIT: 88815d2db74d1ac9039fe09874b81af9cdd9e47b
REPO: dotnet/runtime
BRANCH: main

Release-build FAILED

buildinfo.json
${{details}}

@jkotas jkotas merged commit 46c0166 into dotnet:main Aug 10, 2024
96 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-loongarch64 arch-riscv Related to the RISC-V architecture area-VM-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants