steveicarus / iverilog

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

Compiler fails to initialize array value for vvp #1155

Closed ericastor closed 3 weeks ago

ericastor commented 1 month ago

The following Verilog:

module sample(
  output wire [7:0] out
);
  wire [7:0] x1;
  wire [7:0] x2[0:1];
  wire [7:0] x3[0:0];
  assign x3[0] = 8'h0;
  assign x2[0] = {x3[4'h0]};
  assign x2[1] = {x3[4'h0]};
  assign x1 = 2'h2;
  assign out = x2[x1 > 2'h1 ? 2'h1 : x1];
endmodule

triggers the following vvp error for me at 676b36e:

vvp: array.cc:709: vvp_vector4_t __vpiArray::get_word(unsigned int): Assertion `vsig' failed.

ericastor commented 1 month ago

Looking a bit deeper - this compiles to vvp as:

#! 
:ivl_version "13.0 (devel)" "(v12_0)";
:ivl_delay_selection "TYPICAL";
:vpi_time_precision + 0;
...
S_0x11ba790 .scope module, "sample" "sample" 2 1;
 .timescale 0 0;
    .port_info 0 /OUTPUT 8 "out";
L_0x1255a90 .functor BUFZ 8, L_0x1255660, C4<00000000>, C4<00000000>, C4<00000000>;
L_0x120d6e8 .functor BUFT 1, C4<00000001>, C4<0>, C4<0>, C4<0>;
v0x11b9f60_0 .net/2u *"_ivl_10", 7 0, L_0x120d6e8;  1 drivers
v0x120ce90_0 .net *"_ivl_12", 0 0, L_0x1255790;  1 drivers
L_0x120d730 .functor BUFT 1, C4<00000001>, C4<0>, C4<0>, C4<0>;
v0x120cf50_0 .net/2u *"_ivl_14", 7 0, L_0x120d730;  1 drivers
v0x120d010_0 .net *"_ivl_16", 7 0, L_0x1255900;  1 drivers
v0x120d0f0_0 .net *"_ivl_8", 7 0, L_0x1255660;  1 drivers
v0x120d220_0 .net "out", 7 0, L_0x1255a90;  1 drivers
L_0x120d6a0 .functor BUFT 1, C4<00000010>, C4<0>, C4<0>, C4<0>;
v0x120d300_0 .net "x1", 7 0, L_0x120d6a0;  1 drivers
L_0x120d658 .functor BUFT 1, C4<00000000>, C4<0>, C4<0>, C4<0>;
v0x120d3e0 .array "x2", 1 0;
v0x120d3e0_0 .net v0x120d3e0 0, 7 0, L_0x120d658; 1 drivers
v0x120d4e0 .array "x3", 0 0;
v0x120d4e0_0 .net v0x120d4e0 0, 7 0, v0x120d3e0_0; Alias to x2
L_0x1255660 .array/port v0x120d3e0, L_0x1255900;
L_0x1255790 .cmp/gt 8, L_0x120d6a0, L_0x120d6e8;
L_0x1255900 .functor MUXZ 8, L_0x120d6a0, L_0x120d730, L_0x1255790, C4<>;
# The file index is used to find the file name in the following table.
:file_names 3;
    "N/A";
    "<interactive>";
    "/tmp/test.v";

If I understand this correctly - it looks like we never end up initializing the net corresponding to x2[1]? And in fact, lldb shows that the relevant invocation of get_word returns a nullptr for nets[address]. Not sure where this happens, so I'm not sure how to fix it.

caryr commented 1 month ago

The code for vvp certainly looks wrong so this is likely a compiler issue. There are debug flags for the compiler (described in the man page) that provide information as the code is transformed. There is also the vlog95 target to see if this is just a problem in the VVP code generator. My guess is the compiler is collapsing the two assignments to the x2 array since they are identical or the code generator is not noticing all this has been collapsed into a single nexus and is only printing the first assignment it finds in the nexus.

ericastor commented 1 month ago

Unfortunately, using the vlog95 target just results in:

/tmp/test.v:5: vlog95 error: Array nets (x2) are not supported.
/tmp/test.v:6: vlog95 error: Array nets (x3) are not supported.

I've attached the full compiler logs with all the debug flags enabled, along with the resulting vvp file

test.vvp.log

test.vvp.txt

martinwhitaker commented 1 month ago

It is a bug in the vvp code generator, specifically around line 699 in vvp_scope.c. Having already assigned a signal to that nexus, the code generator should be creating a net alias for the second assignment. But it is mistakenly identifying it as a whole-array alias.

martinwhitaker commented 1 month ago

This might be enough to fix it

diff --git a/tgt-vvp/vvp_scope.c b/tgt-vvp/vvp_scope.c
index 978b27d59..793c2eec3 100644
--- a/tgt-vvp/vvp_scope.c
+++ b/tgt-vvp/vvp_scope.c
@@ -696,7 +696,8 @@ static void draw_net_in_scope(ivl_signal_t sig)
                       so the word count for the signal and the alias
                       *must* match. */

-                 if (ivl_signal_dimensions(nex_data->net) > 0 &&
+                 if (nex_data->net != sig &&
+                      ivl_signal_dimensions(nex_data->net) > 0 &&
                       word_count == ivl_signal_array_count(nex_data->net)) {
                     if (iword == 0) {
                      fprintf(vvp_out, "v%p .array \"%s\", v%p; Alias to %s \n",
ericastor commented 1 month ago

I can confirm that https://github.com/steveicarus/iverilog/issues/1155#issuecomment-2270012294 fixes my example!

caryr commented 1 month ago

@martinwhitaker has this fix been turned in and can this be closed?

martinwhitaker commented 1 month ago

@caryr , no, I haven't yet found the time to write some tests and convince myself the fix is correct.

martinwhitaker commented 3 weeks ago

Fixed in the master branch.