-Wwidth-expand does not detect width mismatches in packed-variable assignments

[jcurtiss8086]

While reviewing LINT errors from an industry tool, we found a real RTL width-mismatch bug that is not reported by Slang. I was expecting that -Wwidth-expand should flag an error for both the param and the logic-var assignments in the below example.

The LHS is 40 bits and the RHS is 30 bits for both the param and the logic-var. The RTL bug was that the 2nd packed dimension was changed from 5:0 to 7:0 without increasing the width from 6-bits to 8-bits in the concatenation values. This resulted in getting unintended values when directly accessing A[1], A[2], etc.

-Wpacked-array-conv also does not trigger.

module top;

    localparam bit [4:0][7:0] A = {
        6'd2,
        6'd4,
        6'd9,
        6'd24,
        6'd24
    };

    logic [4:0][7:0] b;

    always_comb begin
        b = {
            6'd2,
            6'd4,
            6'd9,
            6'd24,
            6'd24
        };

    end

endmodule

[MikePopoloski]

Hmm, that's unfortunate. What's happening here is that since the RHS is fully a constant, Wwidth-expand is suppressed in favor of Wconstant-conversion. In turn, Wconstant-conversion is then suppressed because of the heuristic where the value is representable in the target type and so no information is "lost". Probably that heuristic needs work for multi-dimensional cases, since that does reinterpret the value.

[jcurtiss8086]

Thanks for the explanation of the scenario -- makes complete sense and I had forgotten about Wconstant-conversion, which does catch the inverse case where the array dimensions change from 7:0 -> 5:0, since in that case now there is truncation of real information.

[jcurtiss8086]

What are your thoughts on implementing a new warning to flag if a packed array is assigned with a non-vector expression? e.g. the original example is using a concatenation to assign the full packed-array and would trigger the new warning. This could be applied regardless of variable vs constant. Silencing the warning would require changing the assignment to use a vector-expression of the form A = '{ ... }. This should be a safer coding style when dealing with packed-arrays and the RTL would have still worked correctly in the original scenario even when using the wrong size for each of the vector-elements.

module top;
    logic [1:0][1:0][1:0] b;
    always_comb begin
        b[0] = {  // warn since b[0] is packed-type [1:0][1:0]
           2'h3,
           2'h1
        };
    end
endmodule;

I expect that this warning would be very noisy with many existing code-bases, and the onus would be on the project to clean up packed-array assignments to take advantage of the warning. The practicality of such a task for my own repos is TBD :-)

[MikePopoloski]

Such a warning would be easy to add, but like you say probably super noisy. I agree that the assignment-pattern form is much safer than the concat form though.

My current thinking is that Wpacked-array-conv should have caught the issue; right now it only warns when both sides are multidimensional instead of either side being multidimensional. Concatenations always result in a single-dimensional bit vector and so would always trigger the warning in that case; probably there should be a carve out that suppresses the warning if the rhs is a concat where each element is of the correct width for the resulting dimension. I haven't thought much about Wconstant-conversion yet but maybe it needs a similar catch.

[MikePopoloski]

a80bb18 makes Wpacked-array-conv catch this case.