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

[CIR][ABI][Lowering] Fixes calls with union type #1119

Merged
merged 5 commits into from
Nov 14, 2024

Conversation

gitoleg
Copy link
Collaborator

@gitoleg gitoleg commented Nov 13, 2024

This PR handles calls with unions passed by value in the calling convention pass.

Implementation

As one may know, data layout for unions in CIR and in LLVM differ one from another. In CIR we track all the union members, while in LLVM IR only the largest one.

And here we need to take this difference into account: we need to find a type of the largest member and treat it as the first (and only) union member in order to preserve all the logic from the original codegen.

There is a method StructType::getLargestMember - but looks like it produces different results (with the one I implemented or better to say copy-pasted). Maybe it's done intentionally, I don't know.

The LLVM IR produced has also some difference from the original one. In the original IR gep is emitted - and we can not do the same. If we create getMemberOp we may fail on type checking for unions - since the first member type may differ from the largest type. This is why we create bitcast instead. Relates to the issue #1061

Copy link

github-actions bot commented Nov 13, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

Mostly good, one observation regarding duplication

@bcardosolopes bcardosolopes merged commit d1ad076 into llvm:main Nov 14, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants