Fix non-power-of-2, multi-dimensional array direct assignment

[MrPoloGit]

I'm encountering a bug while running Verilator, specifically the generated C++ code.

When D is set to numbers that are powers of 2, the provided example works; however, when D is set to something like 2560, an error is received resulting from C++ code. However if instead one uses temporary variables, non powers of 2 will then work. Does this have to do how Verilator handles direct assignments in multi-dimensional unpacked arrays, which then leads incorrect array indexing for the C++ code?

Versions:

Linux Mint 21.2
g++ (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
Verilator 5.039 devel rev v5.038-64-g460bfbf18

I created a minimal example while preserving the bug.

Example:

module matrix_test;

parameter  int D                     = 2560;
localparam int MatmulParallelism     = 64;
localparam int MatmulTransfersPerRow = D / MatmulParallelism;
logic          rst_ni                = 0;
logic          clk_i;
typedef logic [MatmulTransfersPerRow-1:0][MatmulParallelism-1:0] matrix_t [];

function automatic matrix_t assign_matrix();
    matrix_t out = new [D];
    logic current_cell = 0;

    for (int i = 0; i < (D*D); i++) begin
        int out_i = i / D;
        int out_j = (i % D) / MatmulParallelism;
        int out_k = (i % D) % MatmulParallelism;

        // Bug with Verilator
        out[out_i][out_j][out_k] = current_cell;
        // logic [MatmulTransfersPerRow-1:0][MatmulParallelism-1:0] row = out[out_i];
        // row[out_j][out_k] = current_cell;
        // out[out_i] = row;
      
    end
    return out;
endfunction
    
always @(posedge clk_i) begin
    matrix_t matrix_stream;

    if (!rst_ni) @(posedge rst_ni);

    matrix_stream = assign_matrix();
end

endmodule

verilator --binary -Wno-lint matrix_test.sv

The resulting error:

Vmatrix_test___024root__DepSet_h92de4ffc__0.cpp: In function ‘VlCoroutine Vmatrix_test___024root___eval_initial__TOP__Vtiming__0(Vmatrix_test___024root*)’:
Vmatrix_test___024root__DepSet_h92de4ffc__0.cpp:59:14: error: lvalue required as left operand of assignment
   57 |             ((0x9ffU >= (0xfffU & (VL_SHIFTL_III(12,32,32, __Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_j, 6U)
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   58 |                                    + (0x3fU & __Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_k))))
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   59 |              && (1U & (__Vfunc_matrix_test__DOT__assign_matrix__0__out.atWrite(__Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_i)[
      |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   60 |                        (0x7fU & ((VL_SHIFTL_III(12,32,32, __Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_j, 6U)
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   61 |                                   + (0x3fU & __Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_k))
      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   62 |                                  >> 5U))] >> (0x1fU
      |                                  ~~~~~~~~~~~~~~~~~~
   63 |                                               & (VL_SHIFTL_III(12,32,32, __Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_j, 6U)
      |                                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   64 |                                                  +
      |                                                  ~
   65 |                                                  (0x3fU
      |                                                  ~~~~~~
   66 |                                                   & __Vfunc_matrix_test__DOT__assign_matrix__0__unnamedblk1__DOT__unnamedblk2__DOT__out_k))))))

Should I stick with the temporary assignment method?

Thanks!

[wsnyder]

Probably V3Unknown has a bug, it's trying to protect against out-of-bounds array assignment and gets confused.

Might you be willing to look into fixing this?

[MrPoloGit]

Sure, I can give it a try.

[paul-demo]

@MrPoloGit I think your report is valid and this is a Verilator limitation, but also you could just write this algorithm with synthesizable verilog (ie, not using new[D]) and Verilator would support that correctly. Do you really need to have one dimension of your 3-dimensional array be dynamically sized?

Issue seems to be the combination of atWrite and some shifts/additions for indexing, which result in a temporary value on the left side which cannot be assigned-to in C++.

It's like this where once you do the right-shift, it becomes an rvalue so you can't assign value to it.

out.atWrite(i)[(idx >> 5)] >> (idx & 0x1f) = value;