verilog ND-array generation should be packed arrays instead of unpacked arrays.

[rdaly525]

Currently the verilog for commonlib.MuxN is generated like the following:

module commonlib_muxn__N2__width9 (
    input [8:0] in_data [1:0],
    input [0:0] in_sel,
    output [8:0] out
);
wire [8:0] _join_out;
coreir_mux #(
    .width(9)
) _join (
    .in0(in_data[0]),
    .in1(in_data[1]),
    .sel(in_sel[0]),
    .out(_join_out)
);
assign out = _join_out;
endmodule

Some verilog tools cannot handle the unpacked input [8:0] in_data [1:0] for ports.

We should default ND-arrays to use packed arrays:

module commonlib_muxn__N2__width9 (
    input [1:0][8:0] in_data,
    input [0:0] in_sel,
    output [8:0] out
);
wire [8:0] _join_out;
coreir_mux #(
    .width(9)
) _join (
    .in0(in_data[0]),
    .in1(in_data[1]),
    .sel(in_sel[0]),
    .out(_join_out)
);
assign out = _join_out;
endmodule

Or set up some verilog generation flags to choose these options.

[leonardt]

Is this high priority? From my conversations with @rsetaluri it seems that the synthesis flows we're using at AHA handles these fine, so does the FB flow. From your other issue, it seems like the tool will support it if you use a flag or .sv extension.

Ideally we could have it be configurable as to whether we use packed or unpacked, but this is quite a lot of work. I think the code in the verilog backend originally used packed, so it's not so bad to add there, but for verification, handling packed arrays (e.g. in fault) is quite tedious (for verilator). We have to map multi-dimensional array interactions into packed C integers for verilator (so we need to generate masking code). For system-verilog, it's fine since you can use the same packed array syntax in the test bench, but we'd need to be aware at the magma level whether a multi-dimensional array will be generated using packed or unpacked.

[rdaly525]

Not high priority as the workaround is to just use System Verilog. But it would be nice if we could tag verilog code generation according to what version of verilog/systemverilog it supports in order to early-out with these types of compatibility errors.

[leonardt]

Just noting this here in case it comes up (raj used the same workaround). If verilog compatability is required or unpacked arrays are to avoided for whatever reason (PD had some problems with this), one can use the disable_ndarray option in magma and disable-ndarray flag for flattentypes in coreir which will flatten ndarrays and generate verilog compatible code (no unpacked arrays)