Write verilog doesn't write declaration for power net.

[smunaut]

This is a follow up to #27 which improved the situation in some case but doesn't handle all of them properly.

Any net that's POWER/GROUND doesn't get a wire declaration when writing a powered netlist.

Say you have two power pins vdd1 and vdd2 and in the database they connect to a single net vdd. What gets written is something like :

assign vdd1 = vdd;
assign vdd2 = vdd;

without any pre-declaration of vdd. Now I couldn't be 100% sure from the verilog LRM, but checking what iverilog accepts, the RHS of an assign must be pre-declared, so the above is not actually valid verilog.

There is a wire vdd that should have been written prior to those assigns.

Alternatively, it could also have been written with the LHS/RHS side swapped I think :

assign vdd = vdd1;
assign vdd = vdd2;

since the LHS can be implicitely declared and the RHS is then the pin name which is existing already.

[parallaxsw]

you need to supply a testcase

[smunaut]

In what format ? DEF ? OpenRoad ODB ? Verilog ?

[parallaxsw]

Obviously I would prefer verilog so openroad isn't in the way but I assume the issue is physical wire related. ODB versions are not compatible so they become unreadable so DEF is better. The smaller the better.

[smunaut]

sta.tar.gz

Here are a couple examples, one that works and the other not. The only difference being the presence of a SPECIALNET section. Of course here that section doesn't serve much purpose, but in the real file it's filled wiht the actual routing path for that net ...

This won't have any wire declaration and thus the assign will have undefined signal in the RHS (if those are not used anywhere else).

read_lef -tech sky130_fd_sc_hd__nom.tlef
read_def openframe_project_wrapper.bad.def
write_verilog -include_pwr_gnd bad.v

This will write proper wire declarations :

read_lef -tech sky130_fd_sc_hd__nom.tlef
read_def openframe_project_wrapper.ok.def
write_verilog -include_pwr_gnd ok.v

As a side note, it seems both of these produce a wrong output if -include_pwr_gnd isn't specified because those assigns for the power net still get written even though the ports are not present ...

[parallaxsw]

Please indicate exactly what Is "bad" about bad.v; eg, what should it look like in your opinion.

[smunaut]

Ah well you can just compare the two.

The bad is missing the wire vgnd; / wire vdpwr; / wire vapwr; lines.

[parallaxsw]

That is not a test case. It is a few files. Please package all of the files required to reproduce the problem including a tcl run script.

[smunaut]

sta.tar.gz

[parallaxsw]

This bool DbInstanceNetIterator::hasNext() if (!net->getSigType().isSupply() || !net->isSpecial()) { introduced by 32da6397c7 Matt Liberty 2022-03-24 prevents the power/ground nets from being visible to OpenSTA. Restoring the iterator to its previous version fixes the problem. I have no idea what the intent of the change to the dbNetwork iterator was.

[smunaut]

@parallaxsw Thank you for looking into this. @maliberty Ping ?

[maliberty]

From the commit:

    This hides VDD/VSS and prevents write_verilog from including

      wire VDD;
      wire VSS;

    which confuses other tools.

Unfortunately I didn't say which other tools. My guess would be formal verification or LVS. We can try restoring it and see what happens.

[smunaut]

I mean, it makes sense to not write them if not writing a powered netlist for sure.

[parallaxsw]

It doesn't make any sense to change the net iterator to change write_verilog. write_verilog shouild just omit the wire declarations if -include_pwr_gnd is not present (as @smunaut has pointed out).

[maliberty]

Fixed in https://github.com/The-OpenROAD-Project/OpenROAD/pull/5174

[smunaut]

@parallaxsw May I suggest to add the following :

diff --git a/verilog/VerilogWriter.cc b/verilog/VerilogWriter.cc
index e6037a9..1e10507 100644
--- a/verilog/VerilogWriter.cc
+++ b/verilog/VerilogWriter.cc
@@ -418,7 +418,7 @@ VerilogWriter::writeAssigns(Instance *inst)
         && (include_pwr_gnd_
             || !(network_->isPower(net) || network_->isGround(net)))
         && (network_->direction(port)->isAnyOutput()
-            || network_->direction(port)->isPowerGround())
+            || (include_pwr_gnd_ && network_->direction(port)->isPowerGround()))
         && !stringEqual(network_->name(port), network_->name(net))) {
       // Port name is different from net name.
       string port_vname = netVerilogName(network_->name(port),

ATM in the test case above, if you look at the netlist_weird.nl.v file being written you'll see it creates an invalid verilog. Now you could argue the input database is "weird" because you have power ports connected to a net that's not declared as a power net ... but still I think since writing the power port is gated by "include_pwrgnd", it makes sense to also gate it here using the same condition or you end up in this situation where you write assigns for a port that you didn't write and is thus non-existent.

[parallaxsw]

I agree that condition should be there. Good catch.