Generating cell model .v file merges files incompatible with IcarusVerilog

[donn]

(Note: The reason I'm raising this as an issue instead of a private report and discussion is that it directly affects an issue raised by @mahmoodulhassan-lm with Fault. AUCOHL/Fault#13)

The constructed file list for sky130 is as follows:

/opt/pdks/sky130/sky130_fd_sc_hd/latest/models/*/*.v
/opt/pdks/sky130/sky130_fd_sc_hd/latest/cells/*/*.*.v
/opt/pdks/sky130/sky130_fd_sc_hd/latest/cells/*/*.v

• Now note that this required a separate tweak to sky130, namely, I had to symlink ./libraries/sky130_fd_sc_hd to ./sky130_fd_sc_hd. Otherwise it simply fails to find any files.

That said, the issue in question appears to be from the skywater PDK's libraries/sky130_fd_sc_hd/latest/cells/dlxbn/sky130_fd_sc_hd__dlxbn.behavioral.v file: it is not compatible with IcarusVerilog. From what I can gather, open_pdks merges anything ending with a .v in both the cells and models folders, when only a certain number of files should've been actually merged. I'm not 100% sure naturally, but that appears to be the case.

[RTimothyEdwards]

@donn : For the first thing, the prior behavior of open_pdks was to expect the variable defined by --with-sky130-source= to include the "libraries/" at the end of the path name. I wanted that removed from --enable-sky130-pdk=, but that implies that the sky130/Makefile has to be modified to add the "libraries/" back to all the places referencing SKYWATER_PATH.

[RTimothyEdwards]

@donn : For the 2nd thing, it looks like wire 1; is invalid verilog. I have seen something similar to this before and logged it as an issue in skywater-pdk. I'll see if I can find that issue. I'm still waiting for @mithro to allow pull requests on the libraries so that these things can be cleaned up.

It is not really the responsibility of open_pdks to keep cleaning up errors in the library sources, but this case ("wire 1" or possibly "wire 0") could be detected and eliminated by a regexp in a filter file. All the verilog files are being passed through the existing filter custom/scripts/inc_verilog.py, so that script could contain a simple re.match() to catch the erroneous line and eliminate it.

[RTimothyEdwards]

@donn : The issue I raised with skywater-pdk was #178 ; however, that specific error was wire 1'b0 which is probably different enough from the present case that it was not fixed by the pull request #231, so either #178 should be reopened with this example, or a separate issue raised.

[donn]

For the first thing, the prior behavior of open_pdks was to expect the variable defined by --with-sky130-source= to include the "libraries/" at the end of the path name. I wanted that removed from --enable-sky130-pdk=, but that implies that the sky130/Makefile has to be modified to add the "libraries/" back to all the places referencing SKYWATER_PATH.

It is not really the responsibility of open_pdks to keep cleaning up errors in the library sources, but this case ("wire 1" or possibly "wire 0") could be detected and eliminated by a regexp in a filter file. All the verilog files are being passed through the existing filter custom/scripts/inc_verilog.py, so that script could contain a simple re.match() to catch the erroneous line and eliminate it.

So be it, I guess. I can get on those.

For the 2nd thing, it looks like wire 1; is invalid verilog. I have seen something similar to this before and logged it as an issue in skywater-pdk. I'll see if I can find that issue. I'm still waiting for @mithro to allow pull requests on the libraries so that these things can be cleaned up.

Yeah, it did look like invalid Verilog, but I've learned to treat the Verilog standard more like a suggestion at this point with all the weird per-simulator/parser quirks.

The issue I raised with skywater-pdk was #178 ; however, that specific error was wire 1'b0 which is probably different enough from the present case that it was not fixed by the pull request #231, so either #178 should be reopened with this example, or a separate issue raised.

Fixing that particular error by simply prefixing the 1 with a \ works, yet there are a number of other problems with missing modules that I encountered:

416 error(s) during elaboration.
*** These modules were missing:
        sky130_fd_sc_hd__udp_dff$NSR_pp$PG$N referenced 6 times.
        sky130_fd_sc_hd__udp_dff$PR_pp$PG$N referenced 12 times.
        sky130_fd_sc_hd__udp_dff$PS_pp$PG$N referenced 10 times.
        sky130_fd_sc_hd__udp_dff$P_pp$PG$N referenced 17 times.
        sky130_fd_sc_hd__udp_dlatch$PR_pp$PG$N referenced 10 times.
        sky130_fd_sc_hd__udp_dlatch$P_pp$PG$N referenced 13 times.
        sky130_fd_sc_hd__udp_dlatch$lP_pp$PG$N referenced 1 times.
        sky130_fd_sc_hd__udp_mux_2to1 referenced 35 times.
        sky130_fd_sc_hd__udp_mux_2to1_N referenced 3 times.
        sky130_fd_sc_hd__udp_mux_4to2 referenced 3 times.
***

This means some or all files under models/ are getting excluded… We'll need to investigate that at any rate.

[RTimothyEdwards]

@donn : Did you include the primitives.v file from the library? That's were all of those modules should be defined.

[donn]

Ah. Welp. That's on me. I'll have to update Fault to allow including multiple files I guess.