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][AArch64][Lowering] Support fields with structs containing constant arrays or pointers #1136

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

bruteforceboy
Copy link
Contributor

This PR adds support for function arguments with structs that contain constant arrays or pointers for AArch64.

For example,

typedef struct {
  int a[42];
} CAT;

void pass_cat(CAT a) {}

As usual, the main ideas are gotten from the original CodeGen, and I have added a couple of tests.

@bruteforceboy bruteforceboy changed the title [CIR][AArch64][Lowering] Support Function Arguments With Structs Containing Constant Arrays or Pointers [CIR][AArch64][Lowering] Support function arguments with structs containing constant arrays or pointers Nov 18, 2024
@bruteforceboy bruteforceboy changed the title [CIR][AArch64][Lowering] Support function arguments with structs containing constant arrays or pointers [CIR][AArch64][Lowering] Support fields with structs containing constant arrays or pointers Nov 18, 2024
@bruteforceboy bruteforceboy marked this pull request as ready for review November 18, 2024 14:35
Copy link
Collaborator

@smeenai smeenai left a comment

Choose a reason for hiding this comment

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

Should we also add a test for a struct containing a smaller array (which should be passed via registers instead of pointer)?

@bruteforceboy
Copy link
Contributor Author

Should we also add a test for a struct containing a smaller array (which should be passed via registers instead of pointer)?

IMHO, the test is okay as it is) The main idea of the PR is adding support for the types because of the "NYI" fail previously. The sizes or how it is passed doesn't really matter (for this PR at least).

Copy link
Collaborator

@smeenai smeenai left a comment

Choose a reason for hiding this comment

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

Checking for different array sizes would get some coverage for the size calculation e.g. making sure that int[4] is passed in registers but long[4] isn't. You're right that it's not strictly necessary here though.

@bcardosolopes bcardosolopes merged commit 0ace889 into llvm:main Nov 25, 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.

3 participants