`write_blif` incorrect behavior when PO is connected to PI of the same index

[lee30sonia]

Describe the bug PO is not written out in BLIF when it is connected to a PI of the same index.

To Reproduce

#include <mockturtle/networks/mig.hpp>
#include <mockturtle/io/verilog_reader.hpp>
#include <mockturtle/io/write_blif.hpp>
#include <lorina/verilog.hpp>

int main()
{
  using namespace mockturtle;

  mig_network mig;
  if ( lorina::read_verilog( "mig.v", verilog_reader( mig ) ) != lorina::return_code::success )
    return -1;

  write_blif( mig, "mig.blif" );

  return 0;
}

mig.v:

module top( x0 );
  input x0 ;
  output y0 , y1 ;
  assign y0 = x0 ;
  assign y1 = x0 ;
endmodule

output mig.blif:

.model top
.inputs pi1 
.outputs po0 po1 
.names new_n0
0
.names new_n1
1
.names pi1 po0
1 1
.end

The second PO (index 1) is dangling in BLIF because its printing is prevented by line 300 of write_blif.hpp. @Nozidoali, you were the last one who updated this file. Any idea why it is written like this? I don't understand why the condition ( topo_ntk.get_node( f ) != index ) should exist.

Additional context Minimize the problem reported in #595.

Check list

• [x] 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.
• [x] I have minimized the testcase.

[Nozidoali]

Hi! I just looked at it quickly, and I think the condition on line 300 is indeed wrong.

When writing latches, I use the flag "skip_feedthrough" to toggle the following:

ri_name -> ro_name   (skip_feedthrough = false)
ri_name -> li<id> -> ro_name   (skip_feedthrough = true)

The condition does not apply to POs. I kept it because it was in the previous version.

Please let me know if it works after removing this line. I can also add some test cases to catch this issue.

[lee30sonia]

You changed the conditioning logic from || to && in #573, why?

This parameter was first added in #245, but its purpose was not really documented and I've never seen it being used anywhere. I am prompted to remove it. If you need a flag like that for some purpose related to sequential interfaces, I suggest to use a more descriptive name and/or add clear documentation (is it really a choice a user can/should make?). In any case, we should carefully examine the code.

[Nozidoali]

I see. I changed it because with || we cannot achieve the desired format in this test case

[lee30sonia]

Fixed by #603