-
Notifications
You must be signed in to change notification settings - Fork 77
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
core: add argument and result types getters to CallableOpInterface #2535
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## sasha/memref/malloc-riscv #2535 +/- ##
==========================================================
Coverage 89.82% 89.82%
==========================================================
Files 351 352 +1
Lines 44116 44169 +53
Branches 6593 6593
==========================================================
+ Hits 39625 39674 +49
- Misses 3531 3535 +4
Partials 960 960 ☔ View full report in Codecov by Sentry. |
Codecov ReportThis pull request has no tests Mr Lopoukhine.
Additional details and impacted filesactually no thanks ☔ View full report in Codecov by Théo |
Weird indeed, I was aware that there were no tests but the fact that CodeCov doesn't bring this up is somewhat surprising. I'll add some |
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.
Really nice! I like it! Thanks! 🚀
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.
Looks good, I just noticed that I don't understand a single thing about MLIR interfaces defined in tablegen
raise NotImplementedError() | ||
|
||
@classmethod | ||
@abc.abstractmethod | ||
def get_argument_types(cls, op: Operation) -> tuple[Attribute, ...]: | ||
raise NotImplementedError() | ||
|
||
@classmethod | ||
@abc.abstractmethod | ||
def get_result_types(cls, op: Operation) -> tuple[Attribute, ...]: | ||
raise NotImplementedError() |
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.
Is there a reference you can give to the MLIR equivalent?
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.
I tried to hunt for this, but the best one I found was at the link in the docstring for the whole trait. I also found it quite difficult to navigate the tablegen.
This adds some more of the capabilities of the MLIR equivalent.
Note stacked PR