lsils / mockturtle

C++ logic network library
MIT License
202 stars 136 forks source link

`write_blif` outputs duplicate `.names` #587

Closed phyzhenli closed 1 year ago

phyzhenli commented 1 year ago

Describe the bug

I use read_blif to read a sequential blif file, and then convert it to MIG. Next, I output the MIG via write_blif. However, when I use abc to read the output.blif, abc tells me that Signal xxx is defined more than once.

To Reproduce Steps to reproduce the behavior:

  1. Which version of mockturtle (commit or PR number) are you using? (Preferably the latest, unless there are special reasons.) Commits on Dec 22, 2022
  2. A complete snippet of your code (usually a cpp file including main).
    
    #include <lorina/blif.hpp>
    #include <mockturtle/mockturtle.hpp>

int main() { mockturtle::names_view<mockturtle::sequential> klut; const auto result = lorina::read_blif( "ISCAS_89/s38417.blif", mockturtle::blif_reader( klut ) ); if ( result != lorina::return_code::success ) { std::cout << "Read benchmark failed\n"; return -1; } mockturtle::mig_npn_resynthesis resyn; mockturtle::names_view<mockturtle::sequential> named_dest; mockturtle::node_resynthesis( named_dest, klut, resyn ); mockturtle::write_blif( named_dest, "output.blif" ); return 0;


3. The benchmark circuit for which the error occurs. Please try to minimize it first by using the testcase minimizer ([docs](https://mockturtle.readthedocs.io/en/latest/debugging.html#testcase-minimizer), [example code](https://github.com/lsils/mockturtle/blob/master/examples/minimize.cpp)).
[s38417.zip](https://github.com/lsils/mockturtle/files/10374197/s38417.zip)

4. Error messages or print-outs you see (if any) (preferably copy-pasted as-is).
abc `read_blif output.blif` error: Line 37537: Signal "g16496" is defined more than once.

**Environment**
 - OS: [e.g. Linux]
 ubuntu 20.04
 - Compiler: [e.g. GCC 12]
 9.4.0
 - Compilation mode: [DEBUG or RELEASE]
N/A

**Additional context**
Add any other context about the problem here
Same error with `write_blif` after converting to AIG/XAG/XMG.

**Check list**
 * [ ] I have tried to run in DEBUG mode and there was no assertion failure (or the reported bug is an assertion failure).
 * [x] I have made sure that the provided code compiles and the testcase reproduces the error.
 * [ ] I have minimized the testcase.
lee30sonia commented 1 year ago

Seems to be related to #573. @Nozidoali, could you take a look and propose a fix?

With a quick investigation:

Nozidoali commented 1 year ago

I reproduced the issue, and it is indeed because of the RI/PO collision.

Note that usually, the CO's name is consistent with the node's name (the \<output> in ".name \<intputs> \<output>"). However, node substitution (e.g., node resynthesis) detaches CO labels from the original definition, and CO names differ from node names (e.g., "g16496" to "new_n1494"). In this case, write_blif has to bridge them using a "virtual" wire (lines 283-284 of write_blif). As mentioned, if multiple COs have the same name (e.g., "g16496"), and are to bridge to the same new signal ("new_n1494"), current implementation will define the "virtual wire" multiple times.

To fix it, we may:

lee30sonia commented 1 year ago

I don't think the issue is related to substitute_node, because if you remove the node_resynthesis lines in the provided code, the problem persists.

What do you mean by "usually, the CO's name is consistent with the node's name"? If a PO or RI has a name, its name (in this case "g16496") is set in names_view when reading in the original file (blif_reader line 127 and 212). At this point, the PO or RI is not even pointed to a node signal yet, but merely declared.

For example, I get the following result by removing node_resynthesis adding some printing code:

[blif_reader line 127] set_output_name to "g16496" for CO index=4 (PO)
[blif_reader line 212] set_output_name to "g16496" for CO index=128 (RI)
[write_blif line 284] "g16496" printed to output.blif in foreach_co with f=!4447, index=4
[write_blif line 284] "g16496" printed to output.blif in foreach_co with f=!4447, index=128
lee30sonia commented 1 year ago

Fixed by #603