Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[TIR][Schedule] Transform layout #10538
[TIR][Schedule] Transform layout #10538
Changes from 10 commits
50e1e92
ab3f83d
203ea83
b4f58df
1e0c445
022ddd3
21bcab5
dcdf22c
a83c413
d518267
d82bccf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@vinx13 Sorry I'm late and missed the code review. On this particular change, I'm 100% in favor of having enum type on C++ side; However, it could be more to "just use string" on the python side, i.e. use "read" and "write" to indicate BufferType. If you agree with my opinion, would you mind sending a quick patch? Thanks a lot!
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.
This sounds good to me as well, and would give the same readability benefits at the caller side. I kind of like the
enum
usage in python as well, since it lets the reader know what options are possible, but that's the less important benefit.At some point, it would be nice to have a macro to define an enum in C++, along with its value/name mapping and an FFI interface, so that there would be a clear way to handle these.