[ExportVerilog] Avoid inlining unpacked array expressions to declarations for tool compatibility

[uenoku]

This changes emission style for unpacked array declaration. Verilator doesn't support initialization assignments for unpacked arrays.

wire w[1:0] = '{0, 0};

This PR checks the value type and prevents inlining. Ideally it is more desirable to improve verilator but for now I want to avoid inlining unpacked arrays to declaration since it's just a tiny readability optimization.

Fix #6363.

[darthscsi]

Are assigns allowed with embedded (not top level) unpacked arrays? Should the test be fore any unpacked type in the aggregate?

[youngar]

Thanks for fixing this, I didn't realize that the assignment syntax only worked for unpacked arrays.

Are assigns allowed with embedded (not top level) unpacked arrays? Should the test be fore any unpacked type in the aggregate?

I guess in Verilog you can't have a packed<unpacked<..>>, so we should probably add a verifier for this.

I am guessing that you could still inline the assignment by mixing array concat for packed and literal for unpacked, e.g.

wire [1:0] w[1:0] = `{{1, 2}, {3, 4}}

[youngar]

I read too quickly and thought you were saying "packed arrays aren't allowed to have assignment initializations", which I don't immediately see an example of in the spec.

As far as the original statement goes, this is an example right from the SystemVerilog spec, section 10.9.1 Array assignment patterns:

bit unpackedbits [1:0] = '{1,1};

And IIRC there was an issue with inlining this into port declarations in a specific tool, which should also be allowed, if I am understanding what this means correctly, 10.8 Assignment-like contexts includes "A port connection to an input or output port of a module, interface, or program".

[uenoku]

I guess in Verilog you can't have a packed<unpacked<..>>, so we should probably add a verifier for this.

I agree, we should reject these at HW verifier.

As far as the original statement goes, this is an example right from the SystemVerilog spec, section 10.9.1 Array assignment patterns:

Yeah, Interesting. It look we can inline assignments for variable types (logic, bit...). So maybe it's not allowed for only net? Both VCS and verilator rejected the example so it's hard to believe these two don't follow the spec.

[youngar]

After some discussion, it seems like assignment expressions might be considered one of the more esoteric sections of the sverilog spec, which was surprising to me. I think the current suggestion is that we have a lowering flag to split the initialization of these vars/wires for tool compatibility. FWIW wire w[1:0] = '{0, 0}; seems to work fine on VCS 2021.

[darthscsi]

I guess in Verilog you can't have a packed<unpacked<..>>, so we should probably add a verifier for this.

Is this true? If so we can make verifiers for sv types.

[darthscsi]

Can we have a couple examples of verilog here which are tested on standard tools? Perhaps added to the rational doc for export.

[darthscsi]

Looking at SV.10.10.1, it looks like there is assignment?
Per 7.4.3, assignment from an integer is not allowed.
Per 7.6 "A packed array cannot be directly assigned to an unpacked array without an explicit cast."
And also per 7.6 examples:

Similarly, the source of an assignment can be a complex expression involving array slices or concatenations.
For example:
string d[1:5] = '{ "a", "b", "c", "d", "e" };
string p[];
p = { d[1:3], "hello", d[4:5] }; 

An example from 6.20: parameter logic [31:0] P1 [3:0] = '{ 1, 2, 3, 4 } ; // unpacked array

[darthscsi]

But to the packed<unpacked> question, 7.4.1: "Packed arrays can be made of only the single bit data types (bit, logic, reg), enumerated types, and recursively other packed arrays and packed structures." We should fix this in the type system.

[uenoku]

I rebased the PR since this issue actually blocks DPI adaption in verilator. Ideally it is more desirable to improve verilator (and I don't think it's hard) but for now I want to avoid inlining unpacked arrays to declaration since it's just a tiny readability optimization. I considered adding lowering options but I think it's overkill for this one.