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

[HW] Fold extracts of constant structs #6010

Merged
merged 2 commits into from
Sep 4, 2023

Conversation

fzi-hielscher
Copy link
Contributor

Fold hw.struct_extract operations of hw.aggregate_constant to the constant value of their respective field.

This breaks a test in HWtoLLVM, which checks correct conversion of the extract(constant) pattern. Since dialect conversion inevitably performs a fold on the input operations before converting, the extract operation will always end up as hw.constant. There does not appear to be a dedicated test for converting constants, so I opted to just keep it in and adapt the expected output.

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

LGTM

@fzi-hielscher
Copy link
Contributor Author

Merging, assuming there are no objections to my tampering with the HWtoLLVM test, @fabianschuiki @maerhart ?

@maerhart
Copy link
Member

maerhart commented Sep 4, 2023

Thanks for implementing this folder @fzi-hielscher!
Regarding the HWToLLVM test-case: this one is supposed to test the aggregate_constant operation, I'm not quite sure anymore why there's also an extract in there (probably that it isn't completely folded away). So, as long as the addressof and load are still there, it's all good.

@fzi-hielscher fzi-hielscher merged commit d838fb8 into llvm:main Sep 4, 2023
5 checks passed
@fzi-hielscher fzi-hielscher deleted the const-struct-fold branch September 5, 2023 14:48
@fabianschuiki
Copy link
Contributor

Thanks a lot @fzi-hielscher 🎉 !

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.

4 participants