steveicarus / iverilog

Icarus Verilog
https://steveicarus.github.io/iverilog/
GNU General Public License v2.0
2.86k stars 530 forks source link

ivl: logic_lpm.c:485: find_local_signal: Assertion `! sig' failed. #1120

Open vicencb opened 6 months ago

vicencb commented 6 months ago

Hi, this verilog code assigning the same wire to two different ports with different direction through two levels of hierarchy triggers an assertion failure.

module a ();
  wire aw;
  b b_inst ( .first(aw), .second(aw) );
endmodule

module b ( input wire first, output wire second );
  c c_inst ( .first(first), .second(second) );
endmodule

module c ( input wire first, output wire second );
  assign second = 1'b1;
endmodule

To reproduce execute this: iverilog -tvlog95 test.v

martinwhitaker commented 6 months ago

It seems likely this has the same root cause as #1119, but I'll leave it as a separate issue for now.

vicencb commented 6 months ago

I was trying to use iverilog to unroll generate loops in a project. The below patch avoids all the failed assertions, but the generated code is still wrong. For example, the c c_inst ( first, first ); should be c c_inst (first, second);

Then, the generate scopes are printed with the further below patch.

--- a/tgt-vlog95/logic_lpm.c
+++ b/tgt-vlog95/logic_lpm.c
@@ -462,6 +462,7 @@ static void emit_nexus_port_signal(ivl_scope_t scope, ivl_nexus_t nex)
          if (scope != ivl_signal_scope(t_sig)) continue;
          assert(! sig);
          sig = t_sig;
+         break;
        }
       }
    /* There will not be a signal for an empty port. */
@@ -485,6 +486,7 @@ static ivl_signal_t find_local_signal(const ivl_scope_t scope, ivl_nexus_t nex,
        assert(! sig);
        sig = t_sig;
        *word = ivl_nexus_ptr_pin(nex_ptr);
+       break;
       }
       return sig;
 }
@@ -512,10 +514,12 @@ void emit_nexus_port_driver_as_ca(ivl_scope_t scope, ivl_nexus_t nex)
        if (t_lpm) {
          assert(! lpm);
          lpm = t_lpm;
+         break;
        }
        if (t_net_const) {
          assert(! net_const);
          net_const = t_net_const;
+         break;
        }
        if (t_nlogic) {
          assert(! nlogic);
@@ -597,6 +601,7 @@ void emit_nexus_as_ca(ivl_scope_t scope, ivl_nexus_t nex, unsigned allow_UD,
          if (t_lpm) {
            assert(! lpm);
            lpm = t_lpm;
+           break;
          }
          if (t_net_const) {
            if (scope != ivl_const_scope(t_net_const)) {
@@ -885,7 +890,10 @@ static ivl_signal_t nexus_is_signal(ivl_scope_t scope, ivl_nexus_t nex,
                  *lsb -= ivl_lpm_base(t_lpm);
                  *msb = *lsb - (int)ivl_lpm_width(t_lpm) + 1;
            }
-         } else lpm = t_lpm;
+         } else {
+           lpm = t_lpm;
+           break;
+         }
        }
        if (t_net_const) {
          assert(! net_const);
--- a/tgt-vlog95/scope.c
+++ b/tgt-vlog95/scope.c
@@ -1121,13 +1121,9 @@ int emit_scope(ivl_scope_t scope, const ivl_scope_t parent)
        emit_named_block_scope(scope);
        return 0; /* A named begin/fork is handled in line. */
    case IVL_SCT_GENERATE:
-       fprintf(stderr, "%s:%u: vlog95 sorry: generate scopes are not "
-                       "currently translated \"%s\".\n",
-                       ivl_scope_file(scope),
-                       ivl_scope_lineno(scope),
-                       ivl_scope_tname(scope));
-       vlog_errors += 1;
-       return 0;
+       fprintf(vlog_out, "%*cbegin: ", indent, ' ');
+       emit_id(ivl_scope_tname(scope));
+       break;
    case IVL_SCT_PACKAGE:
        assert(indent == 0);
        assert(! parent);
@@ -1160,7 +1156,7 @@ int emit_scope(ivl_scope_t scope, const ivl_scope_t parent)
        vlog_errors += 1;
        return 0;
       }
-      fprintf(vlog_out, ";");
+      if (sc_type != IVL_SCT_GENERATE) fprintf(vlog_out, ";");
       emit_scope_file_line(scope);
       fprintf(vlog_out, "\n");
       indent += indent_incr;
@@ -1168,13 +1164,13 @@ int emit_scope(ivl_scope_t scope, const ivl_scope_t parent)
    /* Output the scope ports for this scope. */
       if (sc_type == IVL_SCT_MODULE) {
        emit_module_port_defs(scope);
-      } else {
+      } else if (sc_type != IVL_SCT_GENERATE) {
        emit_task_func_port_defs(scope);
       }

       emit_scope_variables(scope);

-      if (sc_type == IVL_SCT_MODULE) {
+      if (sc_type == IVL_SCT_MODULE || sc_type == IVL_SCT_GENERATE) {
        unsigned count = ivl_scope_lpms(scope);
          /* Output the LPM devices. */
        for (idx = 0; idx < count; idx += 1) {
@@ -1194,7 +1190,7 @@ int emit_scope(ivl_scope_t scope, const ivl_scope_t parent)
        }
       }

-      if (sc_type == IVL_SCT_MODULE || sc_type == IVL_SCT_PACKAGE) {
+      if (sc_type == IVL_SCT_MODULE || sc_type == IVL_SCT_PACKAGE || sc_type == IVL_SCT_GENERATE) {
          /* Output any initial blocks for tasks or functions or named
           * blocks defined in this module. Used to initialize local
           * variables. */
@@ -1304,6 +1300,11 @@ int emit_scope(ivl_scope_t scope, const ivl_scope_t parent)
        free(package_name);
        fprintf(vlog_out, " */\n");
        break;
+   case IVL_SCT_GENERATE:
+       fprintf(vlog_out, "%*cend  /* ", indent, ' ');
+       emit_id(ivl_scope_tname(scope));
+       fprintf(vlog_out, " */\n");
+       break;
    default:
        assert(0);
        break;
martinwhitaker commented 6 months ago

Because you have connected b.first and b.second to the same top level wire (aw), the compiler collapses aw, b.first, b.second, c.first, and c.second into a single net. So the vlog95 code generator doesn't know which name to use. But because Verilog also allows port coercion, generating c c_inst(first, first); is valid.

martinwhitaker commented 6 months ago

However, adding a second instance of module b shows a big problem. I'm not sure we can fix this, other than converting the assertions to "sorry, we can't translate this" messages.