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

[MLIR] Add support for Tuples with integer keys #1260

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

rsetaluri
Copy link
Collaborator

@rsetaluri rsetaluri commented Apr 6, 2023

See #1252.

Fixes this issue by prefixing tuple field keys with "_". This must be in sync between:

  • The emission of struct types
  • The emission of struct extraction

Note that struct creation does not need to be aware of this.

We use a common tuple_key_to_str() function (still specific to MLIR) to achieve this.

@rsetaluri rsetaluri changed the title [MLIR] Prefix tuple field keys with _ [MLIR] Add support for Tuple's with integer keys Apr 6, 2023
@rsetaluri rsetaluri changed the title [MLIR] Add support for Tuple's with integer keys [MLIR] Add support for Tuples with integer keys Apr 6, 2023
Copy link
Collaborator

@leonardt leonardt 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, but I wonder if we could somehow encapsulate the retrieval of a safe index string into the type? E.g. add an API to tuple/product to get a "safe_index" for each field (where tuple would prepend the underscore). This would avoid having to deal with the Product vs Tuple check in the backend and allows for future extensibility (e.g. a user could subclass Tuple and define custom behavior, but they may need the ability to provide a "safe_index").

magma/backend/mlir/hardware_module.py Outdated Show resolved Hide resolved
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