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.

Sign up for GitHub

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

Jump to bottom

Implement some basic MLIR operations #7

Closed
wants to merge 2 commits into from
Closed

Implement some basic MLIR operations #7

wants to merge 2 commits into from

Conversation

asl
Copy link
Collaborator

@asl asl commented Jan 22, 2025 •
edited
Loading

  • Integer, boolean and "easy" singleton types
  • Some constants to check parsing, printing and roundtrip
  • Some additional boilerplate and test fixes

@asl asl requested a review from qobilidop January 22, 2025 20:29
This was linked to issues Jan 22, 2025
Implement basic type lowering #4
Open
Implement constant lowering #5
Open
This was unlinked from issues Jan 22, 2025
Implement basic type lowering #4
Open
Implement constant lowering #5
Open
This was linked to issues Jan 22, 2025
Create MLIR dialect entries for basic P4 types #8
Open
Add MLIR dialect ops for P4 constants #10
Open
@qobilidop
Copy link
Member

It seems the build-and-test action is not triggered. I guess that's because the base branch is boilerplate here. Is it possible to rebase this PR onto main so we can trigger that check?

@asl
Copy link
Collaborator Author

asl commented Jan 23, 2025

It seems the build-and-test action is not triggered. I guess that's because the base branch is boilerplate here. Is it possible to rebase this PR onto main so we can trigger that check?

This is a stacked PR. boilerplate should be merged first, then GitHub will automatically rebase it onto main.

@qobilidop
Copy link
Member

I see. Then we have to fix the build issue in #3 first.

@asl
Copy link
Collaborator Author

asl commented Jan 23, 2025

I see. Then we have to fix the build issue in #3 first.

Feel free to take this if you have a time!

@qobilidop
Copy link
Member

I see. Then we have to fix the build issue in #3 first.

Feel free to take this if you have a time!

Sure, I'll take a look! Most likely this weekend.

@qobilidop qobilidop mentioned this pull request Jan 28, 2025
Closed
qobilidop
qobilidop reviewed Jan 28, 2025
View reviewed changes
lib/Dialect/P4HIR/P4HIR_Attrs.cpp

Type BoolType::parse(mlir::AsmParser &parser) { return get(parser.getContext()); }

void BoolType::print(mlir::AsmPrinter &printer) const {}
Copy link
Member

Choose a reason for hiding this comment

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

Got build errors that there are duplicate symbols BoolType::parse and BoolType::print in this file and P4HIR_Types.cpp. I'm not quite sure which ones should be kept.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it was removed in one of subsequent PRs. The Attr version is redundant certainly.

@asl asl mentioned this pull request Jan 31, 2025
Merged
@asl
Copy link
Collaborator Author

asl commented Jan 31, 2025

Superseded by #46

@asl asl closed this Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qobilidop qobilidop qobilidop left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add MLIR dialect ops for P4 constants Create MLIR dialect entries for basic P4 types
2 participants
@asl @qobilidop