icebox_vlog: improve module declaration and unit testing #329
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolves YosysHQ#328 Summary: - By default emit Verilog-2001 style module declarations with direction and signal type defined in the module prototype and not in the body of the module - Add -C option to output Verilog-1995 module declarations for compatibility with older toolchains - Add some comments to the file to assist future maintainers Background: Currently, icebox_vlog outputs a mixed-style module declaration in which it puts the port direction in the module prototype and later declares the signals as ports or wires. ```verilog module chip (input pin1, output pin2); wire pin1; reg pin2 = 0; ``` The inclusion of the direction in the module prototype is a feature of newer Verilog-2001 specification. It seems probable that older versions of Icarus Verilog had some leeway in handling this mixed-style of declaration. But, it seems that at least the version included in newer Ubuntu distributions considers this to be a syntax error. In this commit, the module declaration above will be emitted as: ```verilog module chip (input wire pin1, output reg pin2 = 0); ``` Or if you sepcificy the -C option for Verilog-1995 compatibility: ```verilog module chip (pin1, pin2); input pin1; output pin2; wire pin1; reg pin2 = 0; ```
Makefile provides various targets for testing icebox_vlog. - %_syn.v targets will use icebox_vlog to generate a .v file from the blinky.asc file - %_syntb targets will invoke iverilog on the .v files - %.json targets will invoke yosys on the .v files - test target will attempt several combinations of command line options for icebox_vlog (all combinations of -s, -l, -p, -c, -C) and will then trigger iverilog and yosys targets to attempt parsing the .v files Output files generated by `make test` can be compared after making code changes to icebox_vlog.py to identify any regressions.
|
It seems clear to me that icebox_vlog is largely superceded by the yosys "write_verilog" command. This change may be seen mostly as a personal journey in understanding, but it may prove useful for continuing to work with unmaintained scripts in other projects. |
|
@gatecat I hope you can review this or assign to a relevant reviewer. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #328
First commit changes iceblox_vlog to output Verilog-2001 module declarations
by default. An additional command line option (-C) supports output of
Verilog-1995 module declarations for backwards compatibility.
Second commit adds test cases that try out different combinations of
commandline options for iceblox_vlog and ensure they parse correctly with
iverilog and yosys.
I didn't see any other obvious regression tests, so I added these to support
development in the future.