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

[DT][GPU] Implement EncodingLayoutResolverAttrInterface for GPUEncodingLayoutAttr. #20027

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

jtuyls
Copy link
Contributor

@jtuyls jtuyls commented Feb 19, 2025

Implements the EncodingLayoutAttrInterface (cloneWithSimplifiedConfig and getLayout methods) for GPU.

This replaces the targetAttr field in GPUEncodingLayoutAttr with a dictionary for storing the serialized encoding info.

This PR also removes an unused /unneeded encoding = #iree_cpu.cpu_encoding_layout<> attribute from the llvmcpu encoding materialization tests which leads to confusion on what this does.

I will implement the SerializedEncodingLayoutAttrInterface in a separate PR.

@jtuyls jtuyls force-pushed the impl-layout-attr-interface branch 2 times, most recently from d88b61c to 4eb88b9 Compare February 19, 2025 22:15
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comment, it looks better to me!

@hanhanW hanhanW changed the title [DT][GPU] Implement EncodingLayoutAttrInterface [DT][GPU] Implement EncodingLayoutAttrInterface for GPUEncodingLayoutAttr. Feb 19, 2025
@jtuyls jtuyls force-pushed the impl-layout-attr-interface branch from 4eb88b9 to 2c6a819 Compare February 19, 2025 23:46
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

Looks good, just few final nits.

@jtuyls jtuyls force-pushed the impl-layout-attr-interface branch from 2c6a819 to 7116165 Compare February 20, 2025 11:21
@jtuyls jtuyls force-pushed the impl-layout-attr-interface branch from 7116165 to 0447670 Compare February 20, 2025 21:26
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

LGTM, just two final nits.

btw, can you update the PR description to reflect the changes? I think we also update the parameters of GPUEncodingLayout, and we need the update and why in the PR description.

@jtuyls jtuyls changed the title [DT][GPU] Implement EncodingLayoutAttrInterface for GPUEncodingLayoutAttr. [DT][GPU] Implement EncodingLayoutResolverAttrInterface for GPUEncodingLayoutAttr. Feb 20, 2025
@jtuyls jtuyls force-pushed the impl-layout-attr-interface branch from 0447670 to 4f00a07 Compare February 20, 2025 23:04
@jtuyls jtuyls force-pushed the impl-layout-attr-interface branch from 4f00a07 to b6a0d81 Compare February 20, 2025 23:13
Copy link
Contributor

@hanhanW hanhanW left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jtuyls jtuyls merged commit d9a3a7c into iree-org:main Feb 20, 2025
43 checks passed
@jtuyls jtuyls deleted the impl-layout-attr-interface branch February 20, 2025 23:53
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