[sverilog,foreach] Fix nested foreach loops #27030

matutem · 2025-04-22T16:43:45Z

Description

Merged PR #26908 shows a misuse of foreach loops in system verilog, and apparently the underlying confusion was already creeping into the code. This has to do with nested foreach loops, documented in the P1800-2017 SystemVerilog LRM under section 12.7.3. The issue is that when nesting loops over multiple dimensions of an array we should NOT re-iterate over the indices already iterated over by the parent loop.

Notice the foreach loops in question are within constraint blocks, while the examples I show are over loops outside constraint blocks. It is hard to show counterexamples for constraint blocks, but it is very unlikely foreach loops iterate differently in constraint and procedural contexts.

It is unclear what is causing this change, perhaps some linter or maybe xcelium behaves differently?

Consider the following simple code:

module test;
logic [1:0][1:0][1:0] a = '{default: 0};
logic [1:0][1:0][1:0] b = '{default: 0};
initial begin
  // Correct looping
  foreach (a[i, j]) begin
    foreach (a[i][j][k]) begin
      a[i][j][k] = '1;
      $display("a[%0d][%0d][%0d] = 1'b%b", i, j, k, a[i][j][k]);
    end
  end
  // Incorrect looping
  foreach (b[i, j]) begin
    foreach (b[i, j, k]) begin
      b[i][j][k] = '1;
      $display("b[%0d][%0d][%0d] = 1'b%b", i, j, k, b[i][j][k]);
    end
  end
end
endmodule : test

This test's output (shown below) shows that the loop over a makes one assignment per index, while the loop over b performs 4 assignments for each b[i][j]. In this simple case this is just a waste of simulation time, but for any assignment that is not idem-potent it will cause incorrect results.

The log is shown below:

Chronologic VCS simulator copyright 1991-2024
Contains Synopsys proprietary information.
Compiler version W-2024.09-SP1-1_Full64; Runtime version W-2024.09-SP1-1_Full64;  Apr 22 16:26 2025
a[1][1][1] = 1'b1
a[1][1][0] = 1'b1
a[1][0][1] = 1'b1
a[1][0][0] = 1'b1
a[0][1][1] = 1'b1
a[0][1][0] = 1'b1
a[0][0][1] = 1'b1
a[0][0][0] = 1'b1
b[1][1][1] = 1'b1
b[1][1][0] = 1'b1
b[1][0][1] = 1'b1
b[1][0][0] = 1'b1
b[0][1][1] = 1'b1
b[0][1][0] = 1'b1
b[0][0][1] = 1'b1
b[0][0][0] = 1'b1
b[1][1][1] = 1'b1
b[1][1][0] = 1'b1
b[1][0][1] = 1'b1
b[1][0][0] = 1'b1
b[0][1][1] = 1'b1
b[0][1][0] = 1'b1
b[0][0][1] = 1'b1
b[0][0][0] = 1'b1
b[1][1][1] = 1'b1
b[1][1][0] = 1'b1
b[1][0][1] = 1'b1
b[1][0][0] = 1'b1
b[0][1][1] = 1'b1
b[0][1][0] = 1'b1
b[0][0][1] = 1'b1
b[0][0][0] = 1'b1
b[1][1][1] = 1'b1
b[1][1][0] = 1'b1
b[1][0][1] = 1'b1
b[1][0][0] = 1'b1
b[0][1][1] = 1'b1
b[0][1][0] = 1'b1
b[0][0][1] = 1'b1
b[0][0][0] = 1'b1
           V C S   S i m u l a t i o n   R e p o r t 
Time: 0
CPU Time:      0.080 seconds;       Data structure size:   0.0Mb

For an example of non-idempotent assignments consider this:

module test;
int a [1:0][1:0][1:0] = '{default: 0};
int b [1:0][1:0][1:0] = '{default: 0};

initial begin
  foreach (a[i, j]) begin
    foreach (a[i][j][k]) begin
      a[i][j][k]++;
      $display("a[%0d][%0d][%0d] = 1'b%b", i, j, k, a[i][j][k]);
    end
  end
  foreach (b[i, j]) begin
    foreach (b[i, j, k]) begin
      b[i][j][k]++;
      $display("b[%0d][%0d][%0d] = 1'b%b", i, j, k, b[i][j][k]);
    end
  end
end
endmodule : test

The output shows how the multiple loops over b ends up incrementing multiple times:

Chronologic VCS simulator copyright 1991-2024
Contains Synopsys proprietary information.
Compiler version W-2024.09-SP1-1_Full64; Runtime version W-2024.09-SP1-1_Full64;  Apr 22 16:50 2025
a[1][1][1] = 1'b00000000000000000000000000000001
a[1][1][0] = 1'b00000000000000000000000000000001
a[1][0][1] = 1'b00000000000000000000000000000001
a[1][0][0] = 1'b00000000000000000000000000000001
a[0][1][1] = 1'b00000000000000000000000000000001
a[0][1][0] = 1'b00000000000000000000000000000001
a[0][0][1] = 1'b00000000000000000000000000000001
a[0][0][0] = 1'b00000000000000000000000000000001
b[1][1][1] = 1'b00000000000000000000000000000001
b[1][1][0] = 1'b00000000000000000000000000000001
b[1][0][1] = 1'b00000000000000000000000000000001
b[1][0][0] = 1'b00000000000000000000000000000001
b[0][1][1] = 1'b00000000000000000000000000000001
b[0][1][0] = 1'b00000000000000000000000000000001
b[0][0][1] = 1'b00000000000000000000000000000001
b[0][0][0] = 1'b00000000000000000000000000000001
b[1][1][1] = 1'b00000000000000000000000000000010
b[1][1][0] = 1'b00000000000000000000000000000010
b[1][0][1] = 1'b00000000000000000000000000000010
b[1][0][0] = 1'b00000000000000000000000000000010
b[0][1][1] = 1'b00000000000000000000000000000010
b[0][1][0] = 1'b00000000000000000000000000000010
b[0][0][1] = 1'b00000000000000000000000000000010
b[0][0][0] = 1'b00000000000000000000000000000010
b[1][1][1] = 1'b00000000000000000000000000000011
b[1][1][0] = 1'b00000000000000000000000000000011
b[1][0][1] = 1'b00000000000000000000000000000011
b[1][0][0] = 1'b00000000000000000000000000000011
b[0][1][1] = 1'b00000000000000000000000000000011
b[0][1][0] = 1'b00000000000000000000000000000011
b[0][0][1] = 1'b00000000000000000000000000000011
b[0][0][0] = 1'b00000000000000000000000000000011
b[1][1][1] = 1'b00000000000000000000000000000100
b[1][1][0] = 1'b00000000000000000000000000000100
b[1][0][1] = 1'b00000000000000000000000000000100
b[1][0][0] = 1'b00000000000000000000000000000100
b[0][1][1] = 1'b00000000000000000000000000000100
b[0][1][0] = 1'b00000000000000000000000000000100
b[0][0][1] = 1'b00000000000000000000000000000100
b[0][0][0] = 1'b00000000000000000000000000000100
           V C S   S i m u l a t i o n   R e p o r t 
Time: 0

The text was updated successfully, but these errors were encountered:

rswarbrick · 2025-04-23T11:16:23Z

Oops! I'm really sorry for not catching this properly when reviewing that PR. Looking at the first commit in the PR, the old code was:

    foreach (filter_cfg[channel]) {
      foreach (filter_cfg[channel][filter]) {
        soft filter_cfg[channel][filter] == FILTER_CFG_DEFAULTS[filter];
      }
    }

I think the important part of the spec is the first paragraph, that says:

The foreach-loop construct specifies iteration over the elements of an array. Its argument is an identifier that
designates any type of array followed by a comma-separated list of loop variables enclosed in square
brackets. Each loop variable corresponds to one of the dimensions of the array.

In the example, the inner foreach loop has:

"identifier that designates any type of array" = filter_cfg[channel]
"comma-separated list of loop variables enclosed in square brackets" = [filter].

My strong suspicion is that the code wasn't quite correct (because filter_cfg[channel] is an expression, but is not an identifier), but presumably it worked correctly for both VCS and Xcelium.

The change was motivated by a lint check that might well have been right: "Use foreach(x[a,b]) instead of foreach(x[a][b])".

I think the correct next step is to follow on from the PR you flagged and collapse some expressions. For the example above, I think we should collapse to

    foreach (filter_cfg[channel, filter]) {
      soft filter_cfg[channel][filter] == FILTER_CFG_DEFAULTS[filter];
    }

Does that make sense?

matutem assigned antmarzam and rswarbrick Apr 22, 2025

matutem added Component:DV DV issue: testbench, test case, etc. Component:RTL labels Apr 22, 2025

matutem mentioned this issue Apr 22, 2025

[dv, adc] correct foreach syntax #26908

Merged

rswarbrick linked a pull request Apr 23, 2025 that will close this issue

[dv] Iterate more carefully with foreach loops #27034

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sverilog,foreach] Fix nested foreach loops #27030

[sverilog,foreach] Fix nested foreach loops #27030

matutem commented Apr 22, 2025 •

edited by rswarbrick

Loading

rswarbrick commented Apr 23, 2025

[sverilog,foreach] Fix nested foreach loops #27030

[sverilog,foreach] Fix nested foreach loops #27030

Comments

matutem commented Apr 22, 2025 • edited by rswarbrick Loading

Description

rswarbrick commented Apr 23, 2025

matutem commented Apr 22, 2025 •

edited by rswarbrick

Loading