-
Notifications
You must be signed in to change notification settings - Fork 315
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
[FIRRTL] Add Layer Association to Probes #6551
Conversation
How do we verify layer symbols in ref types? hw.typealias also has a symbol in a type but unfortunately a malformed type alias is rejected at ExportVerilog at last. |
There's a couple of places this can happen:
I also considered if there was a way to get a symbol table into the type verifier. This seems possible if the type parameter includes a reference to the module. However, this may be problematic. |
79efb90
to
752c62e
Compare
Layers shouldn't need to walk their ops. If we want name resolution to be simple, probably probe ops need to check their parents to do this verification. |
This seems like a good idea to me. |
b3dac3e
to
48d05e8
Compare
48d05e8
to
1931bb7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IR changes LGTM.
I don't understand the full story but these changes adding the layer parameter and IR printing/parsing seem solid so far.
Thanks for touching up utilities and passes to pass this through!
Add support for representing an optional layer in each probe type. This only handles storage and MLIR printing/parsing. Signed-off-by: Schuyler Eldridge <[email protected]>
1931bb7
to
fe27e45
Compare
FWIW stale branch. |
Add support for representing an optional layer in each probe type. This only handles storage and MLIR printing/parsing.
I'll be breaking this work up into a bunch of small pieces. This is the first.