-
Notifications
You must be signed in to change notification settings - Fork 404
[ExportVerilog] Add disallowDeclAssignments lowering option. #9298
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
Conversation
This option controls whether assignments are emitted separately from declarations or inlined within them. When enabled, both continuous assignments and blocking assignments are always emitted separately rather than being inlined in net and variable declarations. This is useful for tools or coding styles that prefer explicit separation between declarations and assignments, improving and compatibility with certain linters or synthesis tools.
seldridge
left a comment
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.
LGTM
| // CHECK-LABEL: module test( | ||
| // DISALLOW-LABEL: module test( | ||
| hw.module @test(in %v: i1) { | ||
| // CHECK: wire w = v; | ||
| // DISALLOW: wire w; | ||
| // DISALLOW: assign w = v; | ||
| %w = sv.wire : !hw.inout<i1> | ||
| sv.assign %w, %v : i1 | ||
| // CHECK: initial begin | ||
| // DISALLOW: initial begin | ||
| sv.initial { | ||
| // CHECK: automatic logic l = v; | ||
| // DISALLOW: automatic logic l; | ||
| // DISALLOW-NEXT: l = v; | ||
| %l = sv.logic : !hw.inout<i1> | ||
| sv.bpassign %l, %v : i1 | ||
| } | ||
| } |
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.
Nit: you can collapse a lot of the checks down using --check-prefixes=CHECK,ALLOW and --check-prefixes=CHECK,DISALLOW to avoid some of the FileCheck duplication.
| // DISALLOW: automatic logic l; | ||
| // DISALLOW-NEXT: l = v; |
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.
It is outside of my understanding of the spec to know if this makes sense. You may want to throw this at verilator --lint-only and make sure it's happy with it. I realize we generally avoid any use of automatic logic due to old problems with tools.
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.
Verilator is ok with this (and we've emitted this form when it's not possible to inline the assignment). automatic logic for sure has poor support generally so I feel we should flip the option at this point.
This option controls whether assignments are emitted separately from declarations or inlined within them. When enabled, both continuous assignments and blocking assignments are always emitted separately rather than being inlined in net and variable declarations.
This is useful for tools or coding styles that prefer explicit separation between declarations and assignments, improving compatibility with certain linters or synthesis tools.
Fixes #9297.