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

[ArcToLLVM] Add support for index dialect #7699

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

fabianschuiki
Copy link
Contributor

Support the index dialect when lowering from Arc to LLVM. Also fix a minor issue in the lowering SimGetPortOp.

@fabianschuiki fabianschuiki added the Arc Involving the `arc` dialect label Oct 14, 2024
Copy link
Member

@maerhart maerhart left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -440,20 +441,19 @@ struct SimGetPortOpLowering : public ModelAwarePattern<arc::SimGetPortOp> {
.getValue());
ModelInfoMap &model = modelIt->second;

auto type = typeConverter->convertType(op.getValue().getType());
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We don't specifically restrict the type of arc model ports, right? So maybe we should check for null type here and return failure? Or at least assert?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Fixed

Support the index dialect when lowering from Arc to LLVM. Also fix a
minor issue in the lowering `SimGetPortOp`.
@fabianschuiki fabianschuiki merged commit 947a635 into main Oct 14, 2024
4 checks passed
@fabianschuiki fabianschuiki deleted the fschuiki/arcilator-index branch October 14, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arc Involving the `arc` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants