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

Couple of improvements to the abi handling code (part 3) #9267

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented Sep 17, 2024

The first commit adds a couple of assertions against misuse of StructReturn. The next two commits are minor cleanups. And the last commit is a partial ABI fix for multi-value returns.

Helps with #9250

The SystemV and WindowsFastcall calling conventions expect the return
area pointer as first argument on most archs, while the Tail and Winch
calling conventions expect it as last argument on all archs. On arm64 x8
is used as return area pointer register. S390x already handled
everything correctly.
@bjorn3 bjorn3 requested a review from a team as a code owner September 17, 2024 19:15
@bjorn3 bjorn3 requested review from fitzgen and removed request for a team September 17, 2024 19:15
@fitzgen fitzgen requested review from cfallin and removed request for fitzgen September 17, 2024 19:45
@fitzgen
Copy link
Member

fitzgen commented Sep 17, 2024

Redirecting cause I'm a bit stretched thin right now and I saw y'all were discussing this stuff on Zulip.

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

These refactors look reasonable -- thanks for working on this!

@cfallin cfallin added this pull request to the merge queue Sep 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 17, 2024
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen labels Sep 17, 2024
@bjorn3
Copy link
Contributor Author

bjorn3 commented Sep 18, 2024

Fixed the bug with windows_fastcall and added a couple of Debug impl improvements.

@alexcrichton alexcrichton added this pull request to the merge queue Sep 18, 2024
Merged via the queue into bytecodealliance:main with commit 279334e Sep 18, 2024
71 checks passed
@bjorn3 bjorn3 deleted the abi_cleanup3 branch September 18, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants