Inconsistent LITENDIAN warnings on arrays
module endian; logic e_big[3:0]; logic e_lit[0:3]; logic eu_big_big[3:0][3:0]; logic eu_lit_big[0:3][3:0]; logic eu_big_lit[3:0][0:3]; logic eu_lit_lit[0:3][0:3]; logic [3:0] epu_big_big[3:0]; logic [0:3] epu_lit_big[3:0]; // warn logic [3:0] epu_big_lit[0:3]; logic [0:3] epu_lit_lit[0:3]; // warn logic [3:0][3:0] ep_big_big; logic [0:3][3:0] ep_lit_big; // warn logic [3:0][0:3] ep_big_lit; // warn logic [0:3][0:3] ep_lit_lit; // warn endmoduleVerilator warns as shown. This is in accordance with the resolution to #614 which was to warn for ascending dimensions when packed but not when unpacked. But what's really the issue here is whether the dimension is of a bit vector or not, i.e. would define the bit indexes into a contiguous bit vector that ends up as a single C object. And that to me looks independent of whether it's packed or unpacked - it has much more to do with whether the dimension is the first dimension or not. It just happens that often in cases like
logic [63:0] arr[0:3];the first dimension is a bit index and the second one isn't - so #644's rule that says to warn about packed but not unpacked turns out to do what we want. In cases like
logic [3:0] arr[0:3];it's less clear - perhaps Verilator would treat this as a 4x4 bit matrix and pack it into one word. But either way, whether it's packed or unpacked makes no difference. It's the order of the dimensions, and the overall size of the object as each dimension is applied, which determines whether objects might be packed into a bit vector. I'd suggest a different rule:
- process dimensions left to right (ignoring any distinction between packed/unpacked i.e. before/after the name)
- keep track of the accumulated size of the object as we multiply in the latest dimension
- while the object is still packable into a bit vector (size < 64 maybe?) warn if the latest dimension is ascending
This also does the right thing for arrays of structures. It makes the warning much more dependent on how objects are or aren't represented as data, and much less on how they are syntactically expressed in the source.
#1 Updated by Wilson Snyder 5 months ago
- Status changed from New to AskedReporter
The warning is intended to find common Verilog bugs, not warn of constructs that may have confusing mappings to C++. Perhaps add a new warning that strictly warns about any big endianness?
As to warning about anything more than 64 bits this seems strange. Presumably the user knows the size of the operations they are using, how is that a mistake?
Well the 64-bit cutoff was just an idea, to achieve the same effect as the current packed/unpacked heuristic but in a more principled way. The idea would be to stop warning about ascending dimensions once the total object gets larger than 64 bits (or some other size that stops it being represented in one word).So you would get a warning for either of
local a[0:3]; local [0:3] a;but neither of
somebigobject a[0:3]; somebigobject [0:3] a;The principle being that the warning depends on the dimensions and the size of the object being dimensioned, not on whether the dimension happens to occur before or after the name.
Also available in: Atom