New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix WIDTH false positive with multidimesional packed arrays #1613
Comments
Original Redmine Comment The rightmost index of dec_len is 31:0, meaning it needs 6 bits to index into. Your cur_bit_len as you indicate is only 4 bits. Perhaps I'm confused, but seems the warning is correct. |
Original Redmine Comment does
|
Original Redmine Comment I think what I said above is consistent with IEEE 1800-2017 7.4.5 which says (among other things), "A subarray is an array that is an element of another array. As in the C language, subarrays are referenced by omitting indices for one or more array dimensions, always omitting the ones that vary most rapidly..." I find that a bit obtuse but in the preceding paragraph it describes "rapidity" of various dimensions which, I think, went something like
|
Original Redmine Comment You're right, an easy cross check:
What looks to be going on is for packed structs Verilator runs these checks twice, once on the code as written then once after the unpacking, the warning is on the second one which makes no sense for the user. Will look into fixing. |
I've just run into this in our codebase in OpenTitan. A simpler example that triggers the same bug: module arrays(input logic [4:0] cnt_i,
input logic [17:0][63:0] data_i,
output logic [63:0] out_o);
assign out_o = data_i[5'd17 - cnt_i];
endmodule The behaviour hasn't changed recently but running with a recent build from master gives:
|
Fortunately disabling WIDTH still gives the right answer, will need to take a further look, meantime adding a test. |
It seems I've hit this one, too, or at least something similar. Here's the minimal reproducer I came up with, parameterized to show different cases:
It seems the WIDTH check for SUB of a packed 2D index is checking the range of the wrong packed index. For VAL_MAX >= 31, MIN_REG >= 1: %Warning-WIDTH: top.sv:23: Operator SUB expects 32 or 6 bits on the LHS, but LHS's VARREF 'rd_index' generates 4 bits. There is no warning for VAL_MAX < 31 or MIN_REG == 0. The number of expected bits seems to depend upon VAL_MAX even though this is indexing into the range [15:MIN_REG], not [VAL_MAX:0]. I'm using Verilator 4.028 2020-02-06 rev v4.026-92-g890cecc1. |
These have the same constraints as array extract. This was noticed by inspection. These operations aren't frequently used, so this probably just hasn't been noticed yet.
Author Name: Tim Allen
Original Redmine Issue: 1613 from https://www.veripool.org
excerpt of file.sv...
The text was updated successfully, but these errors were encountered: