Project

General

Profile

[logo] 
 
Home
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  IPC::Locker
  Parallel::Forker
  Voneline
General Info
  Papers

Multiple comma-separated declaration in one for-loop

Added by Yu Sheng Lin over 1 year ago

Hello.

In the SystemVerilog standard, there can be multiple comma-separated in one for-loop declaration like this.

for (int count = 0, done = 0, j = 0; j * count < 125; j++, count++)
    $display("Value j = %d\n", j );

However, it is not supported by Verilator, at least in 4.004. Is there any plan about supporting this now? Or, can someone provide some instructions about how to write a patch to support this.


Replies (7)

RE: Multiple comma-separated declaration in one for-loop - Added by Wilson Snyder over 1 year ago

It would be excellent to have improvements here.

The reason this is the only format supported is the loop unroller at present only recognizes this form, so performance will die otherwise. So I'd suggest two sets of patches.

First, improve V3Unroll to not look at the initializer list and instead try to expand every loop.

Then with that done you can change the verilog.y to take all statements.

RE: Multiple comma-separated declaration in one for-loop - Added by Yu Sheng Lin over 1 year ago

Hello, I find supporting this is difficult since I have little experience about compilers. I try to work on this but so far I can only make a patch which can detect the comma inside a for-loop. https://github.com/johnjohnlin/verilator_fork/commit/3483c3291f3ae1d33037b43458e47aad31c27d1c

After
%Warning-USERWARN: a.sv:3: Initialization after the first comma is ignored now
%Warning-USERWARN: Use "/* verilator lint_off USERWARN */" and lint_on around source to disable this message.
%Warning-USERWARN: a.sv:3: Assignment after the first comma is ignored now
%Error: Exiting due to 2 warning(s)
%Error: Command Failed /opt/verilator_dev/bin/verilator_bin --cc a.sv
Before
%Error: a.sv:3: syntax error, unexpected ','
%Error: Cannot continue
%Error: Command Failed /usr/bin/verilator_bin --cc a.sv
So I want to ask for some clues about the development. I have inspect the Bison code, and it seems that "for_initialization" returns two types of linked-list (am I correct?).
With type: AstVar->AstAssign->nullptr
No type  : AstAssign->nullptr
Therefore, is it reasonable to change the structure like this?
With type: AstVar->AstAssign->AstVar->AstAssign->...->AstVar->AstAssign->nullptr
No type  : AstAssign->AstAssign->...->AstAssign->nullptr

Finally, you said that I must improve V3Unroll as well, but I cannot understand how this code interact with the data-structure above. Can you also provide explanation about this? Thanks.

RE: Multiple comma-separated declaration in one for-loop - Added by Wilson Snyder over 1 year ago

I applied code similar to your patch to trunk as it seemed a good general improvement. I changed it slightly to match the IEEE grammar and Verilog-Perl's implementation.

As to next steps, you would change where now it prints the error to do '$$=$1; $1->addNext($3);' then if you look at the .tree outputs the AstAssign or AstVar will have a next() that points to the following assignments. Then in V3Unroll handle them, and in later steps if not unrolled you can move those statements to the new format (before or inside the AstWhile that replaces the AstFor.)

RE: Multiple comma-separated declaration in one for-loop - Added by Yu Sheng Lin over 1 year ago

I implement the basic version for this feature based on v4.008. Current code is changed significantly since later assignments can depend on previous ones for (e.g. i = 1, k = N-1-i; see below), which is not very compatible with current code logic. (My understanding is that, in the current code, the Unroller computes one loop-variable, concatenates the loop-body and loop-increment, then replaces all rvalue occurrences of that loop-variable to to a AstConst.)

As a result, I change two things into vectors, including loop-variables, ignored assignments. Also, the assignment is simulated one-by-one now, and the variable update is executed immediately after every RHS update. I use the following example to demonstrate that my implementation basically works. I attach the generated node trees from the code below. For the for-loop with only one variables, the new Unroller generates the same tree as v4.004. For the for-loop with two variables, I manually inspect the generated tree and it seems good. However, since a buggy Unroller can be devastating, this commit still should be reviewed and cleaned-up carefully.

https://github.com/johnjohnlin/verilator_fork/commit/14ea0c26b301f4e5e89e283db2408d12c54be123

module MODULE#(parameter N = 3)(input [N-1:0] x, output [N-1:0] y);
`ifdef NEW
always_comb begin
    for (int i = 0, k = N-1-i; i < 3; i++, k = N-1-i) begin
        y[i] = x[i];
    end
end
`else
always_comb begin
    for (int i = 0; i < N; i++) begin
        y[i] = x[N-1-i];
    end
end
`endif
endmodule
BTW, the new rules in v4.010 seem to accept something like this (loop-variables with different type):
for (int i = 0, double k = N-1-i; i < 3; i++, k = N-1-i) begin
, but this is not a legal in C/C++ so I comment them out in this commit.

MY_TEST.zip (206 KB)

RE: Multiple comma-separated declaration in one for-loop - Added by Wilson Snyder over 1 year ago

Thanks for this work, I need to spend some more time looking at the details of this, but for now I think the big thing would be to see if we can get the 4.010 multiple inits working again, and if you could please add new test_regress tests to your branch that try the new features. Thanks.

RE: Multiple comma-separated declaration in one for-loop - Added by Yu Sheng Lin over 1 year ago

I add 2 small files for this and this regression works fine. Also, t_for_comma_bad regression can be removed later. https://github.com/johnjohnlin/verilator_fork/commit/5e01839c5d4fecd801645eff558d512366fdfe1f

RE: Multiple comma-separated declaration in one for-loop - Added by Wilson Snyder 8 months ago

Sorry, think this got lost, I suggest:

1. File a bug so I don't forget :)

2. The code needs to support the multiple inits.

3. Need to rebase.

4. An alternative might be to have the patch a bit more generally record whatever variables are in the init section only. Then track these through v3simulate replacing each as it goes, if the loop terminates after a reasonable iteration count then do the real expansion. The advantage of this is the loop condition and post-increment can basically be normal code - no need for checking bounds etc any more (most of that algorithm was from before V3Simulate existed).

    (1-7/7)