Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

please add a simple option for dumping *all* arrays in a design #75

Closed
kwantam opened this issue Aug 7, 2015 · 8 comments
Closed

please add a simple option for dumping *all* arrays in a design #75

kwantam opened this issue Aug 7, 2015 · 8 comments

Comments

@kwantam
Copy link

kwantam commented Aug 7, 2015

As far as I can tell, the current method of dumping arrays is to dump them element-by-element. This is not sufficient for several use cases.

  1. In a design with deep hierarchy with arrays at several levels of hierarchy, the amount of boilerplate required to dump out everything can quickly get out of control.
  2. In a design with parameterized, 2-d arrays, it is simply not possible to loop over both indices of the array. This causes a run time error.

3. In a design with vectors of instances, where each instance contains an array, it is not possible to write a nested loop over the vector of instances and the array indices. This causes a compile time error.

I have provided test cases and error outputs below.

Probably (2) and (3) should be fixed or documented; but even if this is fixed, a commandline switch that causes all arrays to be dumped by default seems to me like a near necessity for any sufficiently complex project that uses arrays.

(Edit to note that (3) is not an Icarus issue, it's a requirement of the language semantics, as @martinwhitaker points out below.)

@kwantam kwantam changed the title add a simple option for dumping *all* arrays in a design please add a simple option for dumping *all* arrays in a design Aug 7, 2015
@martinwhitaker
Copy link
Collaborator

I would agree that the ability to pass an array name to $dumpvars to have it dump the entire array would be useful, but this has previously been discussed and rejected by the other Icarus developers (http://sourceforge.net/p/iverilog/feature-requests/21).

I am less convinced that having an option to automatically dump all arrays would be useful - particularly for a complex project, where you would expect to have many large arrays. At least in VCD, if not the other dump formats, I believe each array word would have to be listed as a separate signal, so the dump file would get very bloated.

For your problem with dumping the words of a multi-dimensional array, I can't reproduce the errors you are seeing. When I put your code sample into an otherwise empty module, it compiles without error. If you are still seeing this with the current head of the master branch, can you provide a complete test case, and the compiler command line options you are using.

N.B. I do see a problem at run time with your test case, which I will look into, but I fear it may not be soluble without reworking the way Icarus handles multi-dimensional arrays. This is probably needed anyway, to fix http://sourceforge.net/p/iverilog/bugs/953.

@kwantam
Copy link
Author

kwantam commented Aug 8, 2015

Sorry that my test case was incomplete. As I've dug into this more, it turns out that I was conflating two separate but similar issues. I have posted test cases for each below.

You are right that dumping everything bloats the dumpfile. But it happens often enough that one finds a problem and doesn't want to have to re-run a simulation to chase it down that the major Verilog vendors have added options to dump everything. Yes, there's a tradeoff here between the time spent re-running a failing simulation after adding one more element to the dump list, versus the slowdown caused by dumping too much data to disk; but one can sometimes avoid both, e.g., by kicking off long-running simulations overnight---if only one can ask the simulator to do something as simple as save everything!

(For example, NCVerilog has $shm_probe(), and frankly it is far and away more useful for sprawling bug chases than anything Icarus provides. In my experience, test bench dump lists tend to dump fewer and fewer signals as they mature; but in the initial stages of development the approach is often to just dump everything.)

The last time this was discussed, Cary R. said

The more I think about this the more I'm unsure we should implement this functionality. It is trivial to make a loop to dump the elements you want from the array and making it so that a user could dump a large array by accident may not be what we want.

But the reality is that it's not trivial to write down a whole bunch of loops over a bunch of arrays buried deep in the hierarchy---and even if it were, the functionality is presently broken! And the idea that a person might invoke a command line option or system task by accident is beyond unconvincing.

But look, I realize this project has a culture around it and I'm just some random dude from the internet. So if can't convince y'all that adding an option is a sensible way to go, no problem. If I end up implementing the functionality before someone else does, I'll submit a PR, and if y'all don't like it, no worries---at least I'll be happily creating my massive bloated fst files 😃

@kwantam
Copy link
Author

kwantam commented Aug 8, 2015

First test case covers (2), above.

module test_counter
   #( parameter ncounters = 8
    , parameter nbanks = 4
    , parameter nbits = 8
   )( input         clk
    , input         rstb
    );

integer i, j;
reg [nbits-1:0] count_reg [nbanks-1:0] [ncounters-1:0];

always @(posedge clk or negedge rstb) begin
    if (~rstb) begin
        for (i = 0; i < nbanks; i = i + 1) begin
            for (j = 0; j < ncounters; j = j + 1) begin
                count_reg[i][j] <= 0;
            end
        end
    end else begin
        for (i = 0; i < nbanks; i = i + 1) begin
            for (j = 0; j < ncounters; j = j + 1) begin
                count_reg[i][j] <= count_reg[i][j] + i * j;
            end
        end
    end
end

endmodule

module test ();

reg clk, rstb;

localparam ncounters = 8;
localparam nbanks = 4;
localparam nbits = 16;

test_counter
   #( .ncounters    (ncounters)
    , .nbanks       (nbanks)
    , .nbits        (nbits)
    ) itcount
    ( .clk          (clk)
    , .rstb         (rstb)
    );

integer i, j;
initial begin
    $dumpfile("test2.fst");
    $dumpvars;
    for (i = 0; i < nbanks; i = i + 1) begin
        for (j = 0; j < ncounters; j = j + 1) begin
            $dumpvars(0, itcount.count_reg[i][j]);
        end
    end

    clk <= 0;
    rstb <= 0;
    #1 clk <= 1;
    rstb <= 1;

    #1000 $finish;
end

always @(clk) begin
    clk <= #1 ~clk;
end

endmodule

Compiling with iverilog -g2012 test.sv works, but running the result of this compilation gives

ERROR: test.sv:53: $dumpvars cannot dump a vpiConstant.

@martinwhitaker
Copy link
Collaborator

A workaround for (2) is to use generate loops, e.g.

reg [7:0] memory[3:0][3:0];

genvar i, j;

for (i = 0; i < 4; i = i + 1) begin
  for (j = 0; j < 4; j = j + 1) begin
    initial $dumpvars(0, memory[i][j]);
  end  
end

This is not ideal, because it bloats the compiler output file with a lot of unnecessary scope declarations, but it's better than nothing. You do then hit the problem that Icarus flattens multi-dimensional arrays into a single dimension, so your dump file contains flattened indices, but again this requires a rework of the way multi-dimensional arrays are handled.

@kwantam
Copy link
Author

kwantam commented Aug 8, 2015

Oh, cool! Thanks for the suggestion.

@martinwhitaker
Copy link
Collaborator

In (3) you've hit a Verilog language limitation. From the standard:

"Names in a hierarchical path name that refer to instance arrays or loop generate blocks may be followed immediately by a constant expression in square brackets."

(my bold). Again a generate loop would solve the problem.

@kwantam
Copy link
Author

kwantam commented Aug 8, 2015

D'oh, you're right.

I should have known that I have to use generate in that case; I've made that mistake before 😃

@martinwhitaker
Copy link
Collaborator

I don't think this one will get implemented, as discussed above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants