-
Notifications
You must be signed in to change notification settings - Fork 292
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
[ExtractInstances] Add the extract instances metadata to OM Classes #7667
Conversation
1b83ab2
to
9dc5943
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gist of this looks good, just some minor suggestions and questions.
9dc5943
to
fca36c1
Compare
395be6e
to
568c7fc
Compare
@mikeurbach , @uenoku, this PR is ready for another round of review, please take a look. |
// This builds a ClassOp, with the specified fieldNames and fieldTypes as | ||
// ports. The output property is set from the input property port. | ||
ClassOp static buildSimpleClassOp( | ||
mlir::OpBuilder &odsBuilder, mlir::Location loc, mlir::Twine name, | ||
mlir::ArrayRef<mlir::StringRef> fieldNames, | ||
mlir::ArrayRef<mlir::Type> fieldTypes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be moved to the section let builders = [..]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more minor comments from me, but I think this is looking good.
This change adds the
ExtractInstances
metadata to the OM classes. The OM dialect can then be parsed to generate the respective metadata files.This change enables to move all the metadata to the OM dialect, instead of existing as verbatim ops.