Fix an issue in node substitution in XAG and XMG

include/mockturtle/networks/xag.hpp

@@ -476,23 +476,20 @@ class xag_network
         return std::make_pair( n, get_constant( diff_pol ) );
       }
     }
-    else if ( child0.index == 0 ) /* constant child */
+    else if ( _is_and && child0.index == 0 ) /* constant child */
     {
-      if ( _is_and )
-      {
-        return std::make_pair( n, child0.complement ? child1 : get_constant( false ) );
-      }
-      else
-      {
-        return std::make_pair( n, child1 ^ child0.complement );
-      }
+      return std::make_pair( n, child0.complement ? child1 : get_constant( false ) );
+    }
+    else if ( !_is_and && child1.index == 0 )
+    {
+      return std::make_pair( n, child0 ^ child1.complement );
     }
 
     // node already in hash table
     storage::element_type::node_type _hash_obj;
     _hash_obj.children[0] = child0;
     _hash_obj.children[1] = child1;
-    if ( const auto it = _storage->hash.find( _hash_obj ); it != _storage->hash.end() )
+    if ( const auto it = _storage->hash.find( _hash_obj ); it != _storage->hash.end() && it->second != old_node )
     {
       return std::make_pair( n, signal( it->second, 0 ) );
     }
@@ -522,6 +519,9 @@ class xag_network
 
   void replace_in_outputs( node const& old_node, signal const& new_signal )
   {
+    if ( is_dead( old_node ) )
+      return;
+
     for ( auto& output : _storage->outputs )
     {
       if ( output.index == old_node )
@@ -541,7 +541,7 @@ class xag_network
   void take_out_node( node const& n )
   {
     /* we cannot delete CIs or constants */
-    if ( n == 0 || is_ci( n ) )
+    if ( n == 0 || is_ci( n ) || is_dead( n ) )
       return;
 
     auto& nobj = _storage->nodes[n];
@@ -615,8 +615,7 @@ class xag_network
 #pragma endregion
 
 #pragma region Structural properties
-  auto
-  size() const
+  auto size() const
   {
     return static_cast<uint32_t>( _storage->nodes.size() );
   }

include/mockturtle/networks/xmg.hpp

@@ -589,7 +589,7 @@ class xmg_network
     _hash_obj.children[0] = child0;
     _hash_obj.children[1] = child1;
     _hash_obj.children[2] = child2;
-    if ( const auto it = _storage->hash.find( _hash_obj ); it != _storage->hash.end() )
+    if ( const auto it = _storage->hash.find( _hash_obj ); it != _storage->hash.end() && it->second != old_node )
     {
       return std::make_pair( n, signal( it->second, 0 ) );
     }
@@ -621,6 +621,9 @@ class xmg_network
 
   void replace_in_outputs( node const& old_node, signal const& new_signal )
   {
+    if ( is_dead( old_node ) )
+      return;
+
     for ( auto& output : _storage->outputs )
     {
       if ( output.index == old_node )
@@ -640,7 +643,7 @@ class xmg_network
   void take_out_node( node const& n )
   {
     /* we cannot delete CIs or constants */
-    if ( n == 0 || is_ci( n ) )
+    if ( n == 0 || is_ci( n ) || is_dead( n ) )
       return;
 
     auto& nobj = _storage->nodes[n];

test/networks/mig.cpp

@@ -4,6 +4,7 @@
 #include <kitty/dynamic_truth_table.hpp>
 #include <kitty/operations.hpp>
 #include <kitty/operators.hpp>
+#include <mockturtle/algorithms/cleanup.hpp>
 #include <mockturtle/algorithms/simulation.hpp>
 #include <mockturtle/networks/mig.hpp>
 #include <mockturtle/traits.hpp>
@@ -712,6 +713,67 @@ TEST_CASE( "node substitution in MIGs", "[mig]" )
   } );
 }
 
+TEST_CASE( "invoke take_out_node two times on the same node in MIG", "[mig]" )
+{
+  mig_network mig;
+  const auto x1 = mig.create_pi();
+  const auto x2 = mig.create_pi();
+
+  const auto f1 = mig.create_and( x1, x2 );
+  const auto f2 = mig.create_or( x1, x2 );
+  (void)f2;
+
+  CHECK( mig.fanout_size( mig.get_node( x1 ) ) == 2u );
+  CHECK( mig.fanout_size( mig.get_node( x2 ) ) == 2u );
+
+  /* delete node */
+  CHECK( !mig.is_dead( mig.get_node( f1 ) ) );
+  mig.take_out_node( mig.get_node( f1 ) );
+  CHECK( mig.is_dead( mig.get_node( f1 ) ) );
+  CHECK( mig.fanout_size( mig.get_node( x1 ) ) == 1u );
+  CHECK( mig.fanout_size( mig.get_node( x2 ) ) == 1u );
+
+  /* ensure that double-deletion has no effect on the fanout-size of x1 and x2 */
+  CHECK( mig.is_dead( mig.get_node( f1 ) ) );
+  mig.take_out_node( mig.get_node( f1 ) );
+  CHECK( mig.is_dead( mig.get_node( f1 ) ) );
+  CHECK( mig.fanout_size( mig.get_node( x1 ) ) == 1u );
+  CHECK( mig.fanout_size( mig.get_node( x2 ) ) == 1u );
+}
+
+TEST_CASE( "substitute node and restrash in MIG", "[mig]" )
+{
+  mig_network mig;
+  auto const x1 = mig.create_pi();
+  auto const x2 = mig.create_pi();
+
+  auto const f1 = mig.create_and( x1, x2 );
+  auto const f2 = mig.create_and( f1, x2 );
+  mig.create_po( f2 );
+
+  CHECK( mig.fanout_size( mig.get_node( x1 ) ) == 1 );
+  CHECK( mig.fanout_size( mig.get_node( x2 ) ) == 2 );
+  CHECK( mig.fanout_size( mig.get_node( f1 ) ) == 1 );
+  CHECK( mig.fanout_size( mig.get_node( f2 ) ) == 1 );
+
+  CHECK( simulate<kitty::static_truth_table<2u>>( mig )[0]._bits == 0x8 );
+
+  /* substitute f1 with x1
+   *
+   * this is a very interesting test case because replacing f1 with x1
+   * in f2 makes f2 and f1 equal.  a correct implementation will
+   * create a new entry in the hash, although (x1, x2) is already
+   * there, because (x1, x2) will be deleted in the next step.
+   */
+  mig.substitute_node( mig.get_node( f1 ), x1 );
+  CHECK( simulate<kitty::static_truth_table<2u>>( mig )[0]._bits == 0x8 );
+
+  CHECK( mig.fanout_size( mig.get_node( x1 ) ) == 1 );
+  CHECK( mig.fanout_size( mig.get_node( x2 ) ) == 1 );
+  CHECK( mig.fanout_size( mig.get_node( f1 ) ) == 0 );
+  CHECK( mig.fanout_size( mig.get_node( f2 ) ) == 1 );
+}
+
 TEST_CASE( "substitute node with complemented node in mig_network", "[mig]" )
 {
   mig_network mig;

test/networks/xag.cpp

@@ -9,6 +9,7 @@
 #include <kitty/dynamic_truth_table.hpp>
 #include <kitty/operations.hpp>
 #include <kitty/operators.hpp>
+#include <mockturtle/algorithms/cleanup.hpp>
 #include <mockturtle/algorithms/simulation.hpp>
 #include <mockturtle/networks/xag.hpp>
 #include <mockturtle/traits.hpp>
@@ -659,6 +660,92 @@ TEST_CASE( "create nary functions in XAGs", "[xag]" )
   CHECK( result[2] == copy );
 }
 
+TEST_CASE( "invoke take_out_node two times on the same node in XAG", "[xag]" )
+{
+  xag_network xag;
+  const auto x1 = xag.create_pi();
+  const auto x2 = xag.create_pi();
+
+  const auto f1 = xag.create_and( x1, x2 );
+  const auto f2 = xag.create_or( x1, x2 );
+  (void)f2;
+
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 2u );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 2u );
+
+  /* delete node */
+  CHECK( !xag.is_dead( xag.get_node( f1 ) ) );
+  xag.take_out_node( xag.get_node( f1 ) );
+  CHECK( xag.is_dead( xag.get_node( f1 ) ) );
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 1u );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 1u );
+
+  /* ensure that double-deletion has no effect on the fanout-size of x1 and x2 */
+  CHECK( xag.is_dead( xag.get_node( f1 ) ) );
+  xag.take_out_node( xag.get_node( f1 ) );
+  CHECK( xag.is_dead( xag.get_node( f1 ) ) );
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 1u );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 1u );
+}
+
+TEST_CASE( "substitute node and restrash in XAG", "[xag]" )
+{
+  xag_network xag;
+  auto const x1 = xag.create_pi();
+  auto const x2 = xag.create_pi();
+
+  auto const f1 = xag.create_and( x1, x2 );
+  auto const f2 = xag.create_and( f1, x2 );
+  xag.create_po( f2 );
+
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 1 );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 2 );
+  CHECK( xag.fanout_size( xag.get_node( f1 ) ) == 1 );
+  CHECK( xag.fanout_size( xag.get_node( f2 ) ) == 1 );
+
+  CHECK( simulate<kitty::static_truth_table<2u>>( xag )[0]._bits == 0x8 );
+
+  /* substitute f1 with x1
+   *
+   * this is a very interesting test case because replacing f1 with x1
+   * in f2 makes f2 and f1 equal.  a correct implementation will
+   * create a new entry in the hash, although (x1, x2) is already
+   * there, because (x1, x2) will be deleted in the next step.
+   */
+  xag.substitute_node( xag.get_node( f1 ), x1 );
+  CHECK( simulate<kitty::static_truth_table<2u>>( xag )[0]._bits == 0x8 );
+
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 1 );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 1 );
+  CHECK( xag.fanout_size( xag.get_node( f1 ) ) == 0 );
+  CHECK( xag.fanout_size( xag.get_node( f2 ) ) == 1 );
+}
+
+TEST_CASE( "trivial case (constant) detection in replace_in_node of xag_network", "[xag]" )
+{
+  xag_network xag;
+  auto const x1 = xag.create_pi();
+  auto const x2 = xag.create_pi();
+
+  auto const f1 = xag.create_xor( x1, x2 );
+  auto const f2 = xag.create_and( x1, x2 );
+  xag.create_po( f1 );
+  xag.create_po( f2 );
+
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 2 );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 2 );
+  CHECK( xag.fanout_size( xag.get_node( f1 ) ) == 1 );
+  CHECK( xag.fanout_size( xag.get_node( f2 ) ) == 1 );
+
+  xag.substitute_node( xag.get_node( x1 ), xag.get_constant( true ) );
+
+  CHECK( xag.is_dead( xag.get_node( f1 ) ) );
+  CHECK( xag.fanout_size( xag.get_node( x1 ) ) == 0 );
+  CHECK( xag.fanout_size( xag.get_node( x2 ) ) == 2 );
+  CHECK( xag.get_node( xag.po_at( 0 ) ) == xag.get_node( x2 ) );
+  CHECK( xag.get_node( xag.po_at( 1 ) ) == xag.get_node( x2 ) );
+}
+
 TEST_CASE( "substitute node with complemented node in xag_network", "[xag]" )
 {
   xag_network xag;

test/networks/xmg.cpp

@@ -6,6 +6,7 @@
 #include <kitty/dynamic_truth_table.hpp>
 #include <kitty/operations.hpp>
 #include <kitty/operators.hpp>
+#include <mockturtle/algorithms/cleanup.hpp>
 #include <mockturtle/algorithms/simulation.hpp>
 #include <mockturtle/networks/xmg.hpp>
 #include <mockturtle/traits.hpp>
@@ -730,6 +731,67 @@ TEST_CASE( "node substitution in xmgs", "[xmg]" )
   } );
 }
 
+TEST_CASE( "invoke take_out_node two times on the same node in XMG", "[xmg]" )
+{
+  xmg_network xmg;
+  const auto x1 = xmg.create_pi();
+  const auto x2 = xmg.create_pi();
+
+  const auto f1 = xmg.create_and( x1, x2 );
+  const auto f2 = xmg.create_or( x1, x2 );
+  (void)f2;
+
+  CHECK( xmg.fanout_size( xmg.get_node( x1 ) ) == 2u );
+  CHECK( xmg.fanout_size( xmg.get_node( x2 ) ) == 2u );
+
+  /* delete node */
+  CHECK( !xmg.is_dead( xmg.get_node( f1 ) ) );
+  xmg.take_out_node( xmg.get_node( f1 ) );
+  CHECK( xmg.is_dead( xmg.get_node( f1 ) ) );
+  CHECK( xmg.fanout_size( xmg.get_node( x1 ) ) == 1u );
+  CHECK( xmg.fanout_size( xmg.get_node( x2 ) ) == 1u );
+
+  /* ensure that double-deletion has no effect on the fanout-size of x1 and x2 */
+  CHECK( xmg.is_dead( xmg.get_node( f1 ) ) );
+  xmg.take_out_node( xmg.get_node( f1 ) );
+  CHECK( xmg.is_dead( xmg.get_node( f1 ) ) );
+  CHECK( xmg.fanout_size( xmg.get_node( x1 ) ) == 1u );
+  CHECK( xmg.fanout_size( xmg.get_node( x2 ) ) == 1u );
+}
+
+TEST_CASE( "substitute node and restrash in XMG", "[xmg]" )
+{
+  xmg_network xmg;
+  auto const x1 = xmg.create_pi();
+  auto const x2 = xmg.create_pi();
+
+  auto const f1 = xmg.create_and( x1, x2 );
+  auto const f2 = xmg.create_and( f1, x2 );
+  xmg.create_po( f2 );
+
+  CHECK( xmg.fanout_size( xmg.get_node( x1 ) ) == 1 );
+  CHECK( xmg.fanout_size( xmg.get_node( x2 ) ) == 2 );
+  CHECK( xmg.fanout_size( xmg.get_node( f1 ) ) == 1 );
+  CHECK( xmg.fanout_size( xmg.get_node( f2 ) ) == 1 );
+
+  CHECK( simulate<kitty::static_truth_table<2u>>( xmg )[0]._bits == 0x8 );
+
+  /* substitute f1 with x1
+   *
+   * this is a very interesting test case because replacing f1 with x1
+   * in f2 makes f2 and f1 equal.  a correct implementation will
+   * create a new entry in the hash, although (x1, x2) is already
+   * there, because (x1, x2) will be deleted in the next step.
+   */
+  xmg.substitute_node( xmg.get_node( f1 ), x1 );
+  CHECK( simulate<kitty::static_truth_table<2u>>( xmg )[0]._bits == 0x8 );
+
+  CHECK( xmg.fanout_size( xmg.get_node( x1 ) ) == 1 );
+  CHECK( xmg.fanout_size( xmg.get_node( x2 ) ) == 1 );
+  CHECK( xmg.fanout_size( xmg.get_node( f1 ) ) == 0 );
+  CHECK( xmg.fanout_size( xmg.get_node( f2 ) ) == 1 );
+}
+
 TEST_CASE( "create nary functions in XMGs", "[xmg]" )
 {
   xmg_network xmg;