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

iceblox_vlog does not(?) generate verilog that is compatible with Icarus Verilog #328

Open
X-Illuminati opened this issue Apr 10, 2024 · 0 comments · May be fixed by #329
Open

iceblox_vlog does not(?) generate verilog that is compatible with Icarus Verilog #328

X-Illuminati opened this issue Apr 10, 2024 · 0 comments · May be fixed by #329

Comments

@X-Illuminati
Copy link

When trying the blinky example from nextpnr, I encounter the following error:

icebox_vlog blinky.asc > blinky_chip.v
iverilog -o blinky_tb blinky_chip.v blinky_tb.v
blinky_chip.v:5: error: 'io_0_8_1' has already been declared in this scope.
blinky_chip.v:3:      : It was declared here as a net.
blinky_chip.v:505: error: 'io_13_9_1' has already been declared in this scope.
blinky_chip.v:3:      : It was declared here as a net.
blinky_chip.v:553: error: 'io_13_11_0' has already been declared in this scope.
blinky_chip.v:3:      : It was declared here as a net.
blinky_chip.v:567: error: 'io_13_12_1' has already been declared in this scope.
blinky_chip.v:3:      : It was declared here as a net.
blinky_chip.v:598: error: 'io_13_11_1' has already been declared in this scope.
blinky_chip.v:3:      : It was declared here as a net.
blinky_chip.v:612: error: 'io_13_12_0' has already been declared in this scope.
blinky_chip.v:3:      : It was declared here as a net.

I'm far from an expert on Verilog, but it seems this is due to a mixture of old and new style declarations[1]:

module chip (input io_0_8_1, output io_13_9_1, output io_13_11_0, output io_13_12_1, output io_13_11_1, output io_13_12_0);

wire io_0_8_1;
...

If the keyword wire is to be used in the body of the module, it should not be declared as an input or output in the module prototype. Vice-versa, if the new input/output keywords are to be used in the module prototype, the signal should not be re-declared in the module body.
Either of these two options works with iverilog:

module chip (input wire io_0_8_1, output wire io_13_9_1, output wire io_13_11_0, output reg io_13_12_1=0, output wire io_13_11_1, output wire io_13_12_0);

//wire io_0_8_1;
...
module chip (io_0_8_1, io_13_9_1, io_13_11_0, io_13_12_1, io_13_11_1, io_13_12_0);

input wire io_0_8_1;
...

I presume that the first is the preferred form, but the second may be better for backwards compatibility with other tools that only implement Verilog-1995.

It is possible that, in the past, iverilog was more tolerant of this.
I am currently running iverilog v12.0 stable (the packaged version for Ubuntu 23.10).


[1] https://stackoverflow.com/questions/43765545/verliog-modelsim-error-2388-already-declared-in-this-scope

X-Illuminati added a commit to X-Illuminati/icestorm that referenced this issue Apr 21, 2024
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;
```
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 a pull request may close this issue.

1 participant