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

[Encoding] Introduce EncodingLayoutAttrInterface. #19216

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hanhanW
Copy link
Contributor

@hanhanW hanhanW commented Nov 19, 2024

The revisions introduces a new attribute interface in the Encoding dialect. The interface is used to query layout information needed to materialize encoding attributes.

Any backend can implement the interface to interpret an encoding layout based on their needs.

The current expectation of the interface is to propagate layout information from backends to the host compilation or other targets. The expectation can be adjusted as long as we identify additional needs for encodings/layouts.

It is a step towards #17924

The revisions introduces a new attribute interface in the Encoding
dialect. The interface is used to query layout information needed to
materialize encoding attributes.

Any backend can implement the interface to interpret an encoding layout
based on their needs.

The current expectation of the interface is to propagate layout
information from backends to the host compliation or other targets. The
expectation can be adjusted as long as we identify additional needs for
encodings/layouts.

It is a step towards iree-org#17924

Signed-off-by: hanhanW <[email protected]>
),
/*methodBody=*/"",
/*defaultImplementation=*/[{
return {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to insert an assert to fail here? I assume it must be re-implemented by whoever inherits the interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could do it. The other way to think about it is that the caller can check if the return value is nullptr or not; raise the error with proper messages. Adding an assertion is being careful to me, while it could add confusion to users because the error messages and stack dump are all bad. Both work for me because we do not have a working case today..

Copy link
Contributor

Choose a reason for hiding this comment

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

As a user I would definitely prefer not to check null every time I call this function, if such API call guarantees that it will definitely return something sensible other than nullptr.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants