Skip to content

[NFC] Rename load_from/store_to_memref to load_from/store_to_buffer #20897

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

Merged
merged 1 commit into from
May 27, 2025

Conversation

Max191
Copy link
Contributor

@Max191 Max191 commented May 23, 2025

Renames iree_codegen.load_from_memref to iree_codegen.load_from_buffer, and iree_codegen.store_to_memref to iree_codegen.store_to_buffer for consistency with the op definition.

Signed-off-by: Max Dawkins <max.dawkins@gmail.com>
Copy link
Contributor

@qedawkins qedawkins left a comment

Choose a reason for hiding this comment

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

Why not just update the op description? memref is very clear to me.

@Max191
Copy link
Contributor Author

Max191 commented May 23, 2025

Why not just update the op description? memref is very clear to me.

This is more consistent with recent changes upstream: llvm/llvm-project@8f91b10

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.

I may make a bad suggestion in the previous review because I did not fully follow the upstream RFC: #20874 (review) I threw out the question because I recently saw the change.

I don't have a strong opinion about if it should be memref or buffer. People can have their custom buffer type system that does not use memref, and I think it is the idea of the upstream change. We might not have a real use case, so it may be an over-design idea. I think I'm more like looking for consistency, so I had the second comment about it: #20874 (comment)

It can be either memref or buffer, and I'll let you make the call.

@Max191
Copy link
Contributor Author

Max191 commented May 27, 2025

I like the consistency with upstream, so I will make the change from memref to buffer.

@Max191 Max191 merged commit b3128ba into iree-org:main May 27, 2025
39 of 42 checks passed
@Max191 Max191 deleted the load-from-store-to-memref-to-buffer branch May 27, 2025 18:42
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.

3 participants