Error when using loop index in outer dimension of multi-dimensional packed arrays

[udog14]

The code below sets bit 3 of all members of array a. I should print '8888' (tested with vcs). However, I got this error running with "iverilog -g2012"

mda.v:8: error: A reference to a wire or reg (`i') is not allowed in a constant expression. mda.v:8: error: Array index expressions must be constant here.

module test;

  logic [3:0][3:0] a;

  initial begin
    a = 0;
    for (int i=0; i<4; i++)
      a[i][3] = 1;
    $display("%h", a);
    $finish;
  end

endmodule

[martinwhitaker]

According to this comment in the code:

/*
 * Evaluate the prefix indices. All but the final index in a
 * chain of indices must be a single value and must evaluate
 * to constants at compile time. For example:
 *    [x]          - OK
 *    [1][2][x]    - OK
 *    [1][x:y]     - OK
 *    [2:0][x]     - BAD
 *    [y][x]       - BAD
 * Leave the last index for special handling.
 */

this was a deliberate restriction, but I can't find any justification for it in the IEEE standard. Maybe Steve can remember why, although it was written 9 years ago, so maybe not...

[jgaztelu]

Hi, I have encountered the same issue when trying to verify some existing code. It looks like the issue with the outer prefixes happens only inside of always blocks, and not when using for .. generate loops (I assume these are handled by a completely different part of the code).

So, in my experience, this works:

 generate
        for (genvar i=0; i<par_lines_p; i++) begin
            assign r_bit[i] = i_red[i][i_fetch_bit];
            assign g_bit[i] = i_green[i][i_fetch_bit];
            assign b_bit[i] = i_blue[i][i_fetch_bit];
            assign red_row_sh[i] = {red_row[i][width_p-2:0],r_bit[i]};
        end
 endgenerate

While this doesn't:

always_ff @(posedge clk) begin : prefetch_proc
      red_row[i] <= {red_row[i][width_p-2:0],r_bit[i]};
end

I can confirm this code works with the Vivado simulator and AFAIK it should be legal Systemverilog.

Anyway, not sure if it helps much but it might be helpful if somebody stumble upon the same issue 😊

[udog14]

I've forgotten this. @martinwhitaker I agree that this would be illegal a[2:0][x] = ... because the bus is not contiguous (and thus not really a bus).

But a[y][x] is definitely legal.

[jmlzone]

Seeing a similar issue but with structures. This notation works in other simulators. I just did a git pull and rebuilt to ensure I was up to date: Icarus Verilog version 13.0 (devel) (s20221226-331-g77d7f0b8f)

iverilog -I ../rtl -g2012 -o bist.vvp -c bist.vc ../rtl/marchcmbist.sv:81: sorry: constant selects in always processes are not currently supported (all bits will be included). ../rtl/marchc_mbist.sv:195: error: A reference to a net or variable (`r') is not allowed in a constant expression. ../rtl/marchc_mbist.sv:195: error: Array index expressions must be constant here. ivl: netmisc.cc:1619: NetExpr collapse_array_indices(Design, NetScope, NetNet*, const std::__cxx11::list&): Assertion `rc' failed. Aborted

The For loop on line 189: for(int r=0; r<NUM_RAMS; r++) is unrolled and synthesizable and simulatable.

On 195 I reference into the structure. Full RTL below.

/*-------------------------------------------------------------------------------- (C) Copyright 2004-2023 James M. Lee

$Id:$ $Source$

A perpetual non-exclusive license to use is hereby granted to any consulting customer of James M. Lee

Revision History

Date Who Description

8/26/2004 James Lee Initial Creation 1/12/2007 James Lee Adaptation and simplification 10/20/2023 James Lee Move into SystemVerilog package

a simple march memory bist controller

ref:
https://www.technoarete.org/common_abstract/pdf/IJERECE/v4/i10/Ext_98031.pdf patterns 1) write all zeros to all memories incrementing the address. (no read) 2) read back the zeros and write 1's incrementing the address. 3) read back the ones and write 0's incrementing the address. 4) read back the zeros and write 1's decrementing the address. 5) read back the ones and write 0's decrementing the address. 6) read back the zeros decrementing the address. (no write) --------------------------------------------------------------------------------/ import tap_pkg::; module march_mbist #(parameter MAX_ADDR=4095, DATA_WIDTH=32, PIPE_DEPTH=1, NUM_RAMS=1, ADDR_BITS=$clog2(MAX_ADDR)-1 ) ( input clock, input reset_n, input start, input [DATA_WIDTH-1:0] dataout [0:(NUM_RAMS-1)], output logic we_n, output logic [ADDR_BITS:0] addr, output logic [DATA_WIDTH-1:0] datain, output logic running, output logic fail );

typedef struct packed { logic check; logic expval; logic [ADDR_BITS:0] addr; } CHECK_T;

typedef struct packed { logic failed; logic [DATA_WIDTH-1:0] bits; logic [ADDR_BITS:0] addr; } FAIL_T;

CHECK_T check_nxt; CHECK_T [0:PIPE_DEPTH] check_pipe; FAIL_T [0:(NUM_RAMS-1)] fail_status; FAIL_T [0:(NUM_RAMS-1)] fail_nxt; //FAIL_T fail_status; //FAIL_T fail_nxt; logic [ADDR_BITS:0] addr_nxt;

localparam IDLE = 3'h0; localparam INIT0 = 3'h1; localparam INCR0W1 = 3'h2; localparam INCR1W0 = 3'h3; localparam DECR0W1 = 3'h4; localparam DECR1W0 = 3'h5; localparam DECR0 = 3'h6; logic [2:0] phase, phase_nxt;

localparam READ = 1'b1; localparam WRITE = 1'b0;

logic rw_state, rw_nxt; logic datain_nxt; //-------------------------------------------------------------------------------- // stimulus always_comb begin running = (phase != IDLE) | check_pipe[(PIPE_DEPTH-1)].check; // next phase logic case(phase) IDLE: if(start) phase_nxt = INIT0; else phase_nxt = IDLE; INIT0: if(addr==MAX_ADDR) phase_nxt = INCR0W1; else phase_nxt = INIT0; INCR0W1: if((addr==MAX_ADDR) & (rw_state==WRITE)) phase_nxt = INCR1W0; else phase_nxt = INCR0W1; INCR1W0: if((addr==MAX_ADDR) & (rw_state==WRITE)) phase_nxt = DECR0W1; else phase_nxt = INCR1W0; DECR0W1: if((addr=='0) & (rw_state==WRITE)) phase_nxt = DECR1W0; else phase_nxt = DECR0W1; DECR1W0: if((addr=='0) & (rw_state==WRITE)) phase_nxt = DECR0; else phase_nxt = DECR1W0; DECR0: if(addr=='0) phase_nxt = IDLE; else phase_nxt = DECR0; default: phase_nxt = IDLE; endcase // case (phase) // RW logic case(phase_nxt) IDLE, DECR0: rw_nxt = READ; INIT0: rw_nxt = WRITE; INCR0W1, INCR1W0, DECR0W1, DECR1W0: if(rw_state == WRITE) rw_nxt = READ; else rw_nxt = WRITE; default: rw_nxt = READ; endcase // case (phase) // address logic case(phase) IDLE: addr_nxt = '0; INIT0, INCR0W1, INCR1W0: if((addr != MAX_ADDR) & (rw_state == WRITE)) addr_nxt = addr + 1'b1; else if((addr == MAX_ADDR) & (rw_state == WRITE) & (phase !=INCR1W0)) addr_nxt = 0; else addr_nxt = addr; DECR0W1, DECR1W0, DECR0: if(addr != 0 &((rw_state == WRITE)||(phase==DECR0))) addr_nxt = addr - 1'b1; else if((addr == 0) & (rw_state == WRITE) & (phase !=DECR0)) addr_nxt = MAX_ADDR; else addr_nxt = addr; default: addr_nxt = '0; endcase // case (phase) // write data case(phase) IDLE, DECR0, INIT0, INCR1W0, DECR1W0: datain_nxt = 1'b0; INCR0W1, DECR0W1: datain_nxt = 1'b1; default: datain_nxt = 1'b0; endcase // case (phase) // what to check case(phase) IDLE, INIT0: begin check_nxt.check = 1'b0; check_nxt.expval = 1'b0; check_nxt.addr = '0; end // case: IDLE, INIT0 INCR1W0, DECR1W0: begin check_nxt.check = (rw_state==READ); check_nxt.expval = 1'b1; check_nxt.addr = addr_nxt; end INCR0W1, DECR0W1, DECR0: begin check_nxt.check = (rw_state==READ); check_nxt.expval = 1'b0; check_nxt.addr = addr_nxt; end default:
begin check_nxt.check = 1'b0; check_nxt.expval = 1'b0; check_nxt.addr = '0; end endcase // case (phase) we_n = ~(rw_state == WRITE); end always_ff @(posedge clock or negedge reset_n) if(!reset_n) begin phase <= IDLE; rw_state <= READ; addr <= '0; datain <= '0; for(int r=1; r<PIPE_DEPTH; r++) check_pipe[r] <= '0; for(int r=0; r<NUM_RAMS; r++) fail_status[r] <= '0; end else begin phase <= phase_nxt; rw_state <= rw_nxt; addr <= addr_nxt; datain <= {DATA_WIDTH{datain_nxt}}; for(int i=1; i<=PIPE_DEPTH; i++) check_pipe[i] <= check_pipe[i-1]; check_pipe[0] <= check_nxt; for(int r=0; r<NUM_RAMS; r++) fail_status[r] <= fail_nxt[r]; end // else: !if(!reset_n) //-------------------------------------------------------------------------------- // checker logic logic [(DATA_WIDTH-1):0] expect_data, ram_dout; logic check, already_failed; always_comb begin expect_data = {DATA_WIDTH{check_pipe[PIPE_DEPTH].expval}}; check = check_pipe[PIPE_DEPTH].check; if((phase == IDLE) & start) begin fail_nxt = '0; end // if ((phase == IDLE) & start) else for(int r=0; r<NUM_RAMS; r++) begin ram_dout = dataout[r];//[r]; already_failed = fail_status[r].failed; if(~already_failed & check & (ram_dout != expect_data)) begin fail_nxt[r].failed = 1'b1; fail_nxt[r].addr = check_pipe[PIPE_DEPTH].addr; fail_nxt[r].bits = ram_dout ^ expect_data; end // if (~already_failed & check & (ram_dout !=expect_data)) else begin fail_nxt[r] = fail_status[r]; end // else: !if(~already_failed & check & (ram_dout !=expect_data)) end fail=0; for(int f=0; f<NUM_RAMS; f++) fail = fail | fail_status[f].failed; end endmodule // march_mbist

[jhallen]

I wish this was fixed..

this was a deliberate restriction, but I can't find any justification for it in the IEEE standard. Maybe Steve can remember why, although it was written 9 years ago, so maybe not...

I remember commercial simulators had similar restrictions: for example you can't have vec[j+7:j] (maybe too hard to prove that the slice width is always constant), so they added vec[j :+ 8]. If this can work, I think it's reasonable to expect var[j][7:0] to also work.

If it's on the right side, you can use a temporary variable as a work-around: tmp = vec[j]; tmp[7]...

[cousteaulecommandant]

For the record, this issue only happens on multidimensional packed arrays. You can do x[i][j][k] just fine if the first 2 dimensions of x are unpacked.

---

I remember commercial simulators had similar restrictions: for example you can't have vec[j+7:j] (maybe too hard to prove that the slice width is always constant), so they added vec[j :+ 8].

I understand that this is a limitation of standard Verilog. A range expression is either constant_expr : constant_expr or expr +: constant_expr (or -:), but it cannot be expr : expr with two variable expressions (which makes sense because that'd allow variable width ranges). This applies even to the case where both expressions are a fixed value apart (e.g. j+7 : j), probably because it's hard to determine that at compile time. In any case, I don't think the bug described in this issue is related to ranges.

If it's on the right side, you can use a temporary variable as a work-around: tmp = vec[j]; tmp[7]...

This workaround might be worth a try, thanks! I'll have it in mind in case I encounter this issue in the future. (Then again, I'd prefer not to build my IP code for compatibility with a certain simulator, except perhaps the testbench part.)