-
Notifications
You must be signed in to change notification settings - Fork 76
Adding mechanisms to avoid unnecessary swizzle conversions. #584
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
base: main
Are you sure you want to change the base?
Adding mechanisms to avoid unnecessary swizzle conversions. #584
Conversation
[update] module.dart [new] swizzle_opt.dart [new] swizzle_opt_test.dart
…ed when the swizzle was not being replaced).
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.
I'll take a deeper look through this as soon as possible, but at first review, I'm a little concerned about the approach of post-processing generated SystemVerilog with regular expressions rather than modifying the generation mechanism before SystemVerilog is generated.
For example, the _collapseArrays
flow in the SystemVerilogSynthesizer
stack handles collapsing of assignments more generically by reference before SystemVerilog is emitted:
rohd/lib/src/synthesizers/systemverilog.dart
Line 1284 in 4413925
void _collapseArrays() { |
Is there a reasonable way to transform your algorithm to apply more generically pre-SV generation? My intuition is that it would become more robust and less brittle to specific SV generation styling, since these regular expressions are not as robust as a full SystemVerilog parser anyways.
Another consideration is that generated SV may contain "custom" SystemVerilog modules written by users, and we wouldn't want to modify those at all.
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.
In this PR and #586, the implementation should really be from the perspective of generation rather than post-processing of SystemVerilog output. As implemented, this is a Dart implementation of a SystemVerilog code modifier. Unfortunately, that approach won't work well for ROHD's applications:
- Code generation may change, and this implementation without a full SystemVerilog parser is not robust to those types of changes
- Custom SystemVerilog modules in ROHD may have their own specific style and implementation, and it breaks the API contract to modify that code
A couple ways this could be implemented:
- Keep it in ROHD, and move the algorithmic manipulation inside the synthesizer stack
- Make a SystemVerilog "beautifier", similar to verible, which contains these features. This probably doesn't belong in the ROHD repo, and would be a substantial effort to build.
Happy to discuss ways to migrate over to the right spot in ROHD if you need more guidance!
Description & Motivation
The use of swizzle conversions could be unnecessary in the generated SystemVerilog code when there are assignments between a LogicArray with 1 dimension (packed), and a Logic of the same width. This changes add mechanisms to identify those cases and replace the swizzle conversions with simple assignments.
Related Issue(s)
#559
Testing
A unit testing script was created to validate the [SystemVerilogSwizzleOptimizer] functionality.
rohd/test/swizzle_opt_test.dart
Backwards-compatibility
No.
Documentation
The [SystemVerilogSwizzleOptimizer] class was added with its code documentation, no need to add more.
rohd/lib/src/utilities/swizzle_opt.dart