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

[Handshake] Persist handshake.FuncOp argument names #2048

Merged
merged 3 commits into from
Nov 2, 2021

Conversation

mortbopet
Copy link
Contributor

@mortbopet mortbopet commented Oct 28, 2021

This commit modifies the parsing of handshake.func operations to store the SSA argument names as a new attribute argNames. This attribute is then later used in handshakeToFIRRTL to guide argument naming. This follows how FIRRTL and Calyx implement argument naming.

This, unfortunately, does not solve how to persist names from standard->handshake - a builtin.func does not persist parsed SSA names so i've yet to figure out a method of doing this without requiring a change to the parser of builtin.func.

If provided, an argNames attribute set for a builtin.func will be copied to the handshake.func during StandardToHandshake so that is a halfway-solution for driving naming from a builtin.func level.

Copy link
Contributor

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

Nice, seems like a good enhancement to me.

This commit modifies the parsing of `handshake.func` operations to store the SSA argument names as a new attribute `argNames`. This attribute is then later used in `handshakeToFIRRTL` to guide argument naming.

This, unfortunately, does not solve how to persist names from `standard`->`handshake` - a `builtin.func` does not persist parsed SSA names. For reference, these are discarded at this point: https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/BuiltinDialect.cpp#L116-L119

If provided, an `argNames` attribute set for a `builtin.func` will be copied to the `handshake.func` during `StandardToHandshake`.
@mortbopet mortbopet merged commit 6cb07bf into llvm:main Nov 2, 2021
@mortbopet mortbopet deleted the handshake/better_names branch November 5, 2021 14:27
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