netlist: fix instrumenting multiple bits of a vectored port

Covered in testcase_PIN_PIN_bus_bus2single

bin/FIJI/Netlist.pm

@@ -541,6 +541,7 @@ sub instrument_net {
 
     # Choose intermediate net name
     my $net_name_tmp = _sanitize_identifier(FIJI_NAMESPACE_PREFIX . $net_name . "_in_tmp");
+    my $net_tmp = $mod->find_net($net_name_tmp);
     my $name_check = _check_name_in_hierarchy($mod, $net_name_tmp);
 
     # Switch the driver from the original signal to the intermediate net.
@@ -552,17 +553,19 @@ sub instrument_net {
         if (ref($connection) eq "Verilog::Netlist::Pin") {
             # If the driver is a pin of a (sub)cell, connect this pin to the intermediate net
             $logger->debug("Connecting (output) pin \"" . $connection->cell->name . HIERSEP . $connection->name . "\" to intermediate net \"$net_name_tmp\"");
-            # FIXME: do concatenations really work? They are apparently split already by Verilog::perl but...
             for my $netname ($connection->netnames) {
                 if ($netname->{'netname'} eq $net->name) {
                     $driver = $connection;
                     $netname->{'netname'} = $net_name_tmp; # FIXME: do we need to force a re-link (by deleting $connection->nets)?
-                    # This net is a vector if the underlying net is a bus and we do not just select a single bit
-                    # If driver is a vector we need a vectored intermediate bus.
+                    # The intermediate net is a vector if the underlying net is a bus and we do not just select a single bit
                     # This is the case if
-                    #   - the underlying net is a vector and there are no indices
-                    #   - the underlying net is a vector and there are two different indices
-                    if (defined($net->msb) && (!defined($netname->{'msb'}) || $netname->{'msb'} != $netname->{'lsb'})) {
+                    #   - the underlying net is a vector and the driver covers all indices (implicitly by having no msb/lsb set)
+                    #   - the underlying net is a vector and the driver covers some indices (by having two different indices set)
+                    # Also, if we instrument a second bit of a vector we need to make sure this instrumentation is handled as if the
+                    # the driver itself is a vector.
+                    if (defined($net->msb) && (!defined($netname->{'msb'}) || ($netname->{'msb'} != $netname->{'lsb'}))) {
+                        $driver_is_vector = 1;
+                    } elsif (defined($net_tmp) && defined($net_tmp->userdata("first_instrumented_bit")))  {
                         $driver_is_vector = 1;
                     } else {
                         # Make sure we do not output the index unnecessarily (if the driver is a single bit of a vector)
@@ -575,10 +578,6 @@ sub instrument_net {
         } else {
             if (ref($connection) eq "Verilog::Netlist::Port") {
                 $driver_is_port = 1;
-                # If instrumented net is a vector we will need an intermediate bus
-                if (defined($net->msb)) {
-                    $driver_is_vector = 1;
-                }
                 $driver = $connection;
                 # If we are changing the name of a port of the top module we need to inform the VHDL generator
                 if ($mod->is_top) {
@@ -589,6 +588,14 @@ sub instrument_net {
                 $logger->debug("Transforming previous port named \"". $connection->name . "\" into an ordinary wire");
                 $connection->net->decl_type(undef);
                 $connection->net->net_type("wire");
+                my $data_type;
+                # If instrumented net is a vector we will need an intermediate bus
+                if (defined($net->msb)) {
+                    $driver_is_vector = 1;
+                    $data_type = "[".$net->msb.":".$net->lsb."]";
+                    $connection->data_type($data_type);
+                }
+                $connection->net->data_type($data_type);
                 # Unsetting the port<->net inter-references forces their automatic re-setting at link time
                 $connection->net->port(undef);
                 $connection->net(undef);
@@ -615,11 +622,14 @@ sub instrument_net {
                     $driver_is_vector = 1;
                     $driver_bit = $connection->userdata->{'fiji_driver_bit'};
                 }
-                $logger->debug("Connecting to intermediate net \"" . $net_name_tmp . "\" the continuous assignment of \"" . $connection->rhs . "\"");
-                # need to remember what was originally connected to this assign
-                # to instrument two nets driven by one assign
-                $connection->userdata('former_assign' => {'lhs' => $connection->lhs, 'rhs' => $connection->rhs});
-                $connection->lhs($net_name_tmp);
+
+                if (!defined($net_tmp) || !defined($net_tmp->userdata("first_instrumented_bit"))) {
+                    $logger->debug("Connecting to intermediate net \"" . $net_name_tmp . "\" the continuous assignment of \"" . $connection->rhs . "\"");
+                    # need to remember what was originally connected to this assign
+                    # to instrument two nets driven by one assign
+                    $connection->userdata('former_assign' => {'lhs' => $connection->lhs, 'rhs' => $connection->rhs});
+                    $connection->lhs($net_name_tmp);
+                }
 
             } else {
                 $logger->error("Driver instance is neither pin, port nor contassign?");
@@ -634,23 +644,73 @@ sub instrument_net {
     # 3.) If original signal is a bus forward orignal data from tmp net to untouched bits of original signal
     # 4.) Forward (injected bit of) intermediate net to FIC by assigning it to the output pin
     #
-    # If the name is already taken then assume we need to instrument another bit of a bus
-    # FIXME: maybe we should try harder to find out the reason why _check_name_in_hierarchy failed
-    if (defined($name_check)) {
+    # However, if the net is already instrumented we need to jump through some hoops:
+    # In that case we either have an error, or we need to instrument another bit of a bus.
+    # The latter complicates things a bit...
+    if (defined($net_tmp) && defined($net_tmp->userdata("first_instrumented_bit"))) {
+        # FIXME: maybe we should try harder to find out the reason why _check_name_in_hierarchy failed
         if (!defined($net->msb)) {
             my $err = "Tried to instrument non-vector signal (\"$net_name\") twice or we really have a naming conflict.";
             $logger->error($err);
             return $err;
         }
-        # We need to undo the previous assignment of the respective bit
-        foreach my $statement ($mod->statements) {
-            if ($statement->rhs =~ /^[ \t]*~?[ \t]*\Q$net_name_tmp\E(\[(\Q$driver_bit\E)\])?$/) {
-                $logger->debug("    unassigning \"" . $statement->lhs . " = " . $statement->rhs . "\"");
-                $statement->delete();
+
+        if (!defined($net_tmp->msb)) {
+            # If we instrument the second bit of a net (as wittnessed by the missing msb field)
+            # then we need to make sure the intermediate net is or becomes a vector
+            my $prev_bit = $net_tmp->userdata("first_instrumented_bit");
+            my $prev_driver = $net_tmp->userdata("first_driver");
+            $logger->debug("    widening \"" . $net_tmp->name . " to [" . $net->msb . ":" . $net->lsb . "]");
+            $net_tmp->msb($net->msb);
+            $net_tmp->lsb($net->lsb);
+            $net_tmp->data_type("[".$net->msb.":".$net->lsb."]");
+
+            # Additionally the previous exporting of the original signal needs to be adepted
+            foreach my $statement ($mod->statements) {
+                if ($statement->rhs =~ /^[ \t]*~?[ \t]*\Q$net_name_tmp\E$/) {
+                    $statement->rhs($net_name_tmp."[".$prev_bit."]");
+                    $logger->debug("    reassigning \"" . $statement->lhs . " = " . $statement->rhs . "\" --> \"".$net_name_tmp."[".$prev_bit."]\"");
+                }
+            }
+
+            # We also need to fix the previous assignment of the respective bit...
+            if (ref($prev_driver) eq "Verilog::Netlist::Pin") {
+                $logger->debug("    reassigning pin \"" . $prev_driver->name . "\": $net_name_tmp --> $net_name_tmp\[$prev_bit\]");
+                for my $netname ($prev_driver->netnames) {
+                    if ($netname->{'netname'} eq $net_name_tmp) {
+                        $netname->{'msb'} = $prev_bit;
+                        $netname->{'lsb'} = $prev_bit;
+                        last;
+                    }
+                }
+            } elsif (ref($prev_driver) eq "Verilog::Netlist::Port") {
+                $logger->debug("    reassigning port \"" . $prev_driver->name . "\"");
+                return "BORKED"; # not implemented yet
+            } elsif (ref($prev_driver) eq "Verilog::Netlist::ContAssign") {
+                $logger->debug("    reassigning ContAssign \"" . $prev_driver->lhs . " = " . $prev_driver->rhs . "\"");
+                $prev_driver->rhs($net_name_tmp."[".$prev_bit."]");
+            }
+
+            # And add the default assignments of the (possibly yet) uninstrumented bits
+            my ($low, $high) = _extract_low_high($net->lsb, $net->msb);
+            $logger->debug("    assigning \"" . $net_name."[".$low."-".$high."] = ...");
+            for (my $i = $low ; $i <= $high ; $i++) {
+                if ($i != $driver_bit && $i != $prev_bit) {
+                    generate_contassign($mod, $net_name."[".$i."]", $net_name_tmp."[".$i."]");
+                }
+            }
+        } else {
+            # For all additional bits (starting with the third one)
+            # the previous assignment of the respective bit needs to be undone
+            foreach my $statement ($mod->statements) {
+                if ($statement->rhs =~ /^[ \t]*~?[ \t]*\Q$net_name_tmp\E(\[(\Q$driver_bit\E)\])$/) {
+                    $logger->debug("    unassigning \"" . $statement->lhs . " = " . $statement->rhs . "\"");
+                    $statement->delete();
+                }
             }
         }
 
-        $logger->debug("    reassigning \"" . $net_name."[".$driver_bit."] = " . $ip->net->name . "\"");
+        $logger->debug("    assigning to currently processed signal \"" . $net_name."[".$driver_bit."] = " . $ip->net->name . "\"");
         generate_contassign($mod, $net_name."[".$driver_bit."]", $ip->net->name);
         $driver_is_vector = 1;
     } else {
@@ -719,6 +779,12 @@ sub instrument_net {
         generate_contassign($mod, $op->net->name, $net_name_tmp."[".$driver_bit."]");
     }
     $mod->link;
+    # Normally we set first_instrumented_bit way earlier but since we autogenerate
+    # the port we don't have the respective net available before linking thus...
+    if ($driver_is_vector && $driver_is_port && !defined($net_tmp)) {
+        my $port_net = $mod->find_net($net_name_tmp);
+        $port_net->userdata("first_instrumented_bit", $driver_bit);
+    }
     return undef;
 }