Skip to content
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

verilator %Warning-WIDTH false positive #1613

Open
veripoolbot opened this issue Nov 22, 2019 · 4 comments
Open

verilator %Warning-WIDTH false positive #1613

veripoolbot opened this issue Nov 22, 2019 · 4 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Nov 22, 2019


Author Name: Tim Allen
Original Redmine Issue: 1613 from https://www.veripool.org


excerpt of file.sv...

logic   [31:0]          distance;
logic   [31:0]          bfield;
logic   [3:0]           cur_bit_len;
typedef struct packed {         // 10 bits total
     bit [8:0]          max_tbl_size;
     bit [15:0][31:0]   dec_len;
     bit [15:0][31:0]   dec_pos;
     bit [31:0]         q_bits;
} decode_table_t;

distance <= bfield - o_dec_table.dec_len[cur_bit_len - 4'd1];
    ^          ^                     ^         ^         ^
//32bits    32bits                32bits    4bits     4bits

yields the following false positive warnings
%Warning-WIDTH: file.sv:123: Operator SUB expects 32 or 6 bits on the LHS, but LHS's VARREF 'cur_bit_len' generates 4 bits.
%Warning-WIDTH: file.sv:123: Operator SUB expects 32 or 6 bits on the RHS, but RHS's CONST '4'h1' generates 4 bits.

I hope this is clear enough. AFAIK 

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 22, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-11-22T17:21:39Z


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.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 22, 2019


Original Redmine Comment
Author Name: Tim Allen
Original Date: 2019-11-22T19:44:46Z


does bit [15:0][31:0] dec_len; not mean 16, 32 bit dec_lens for a total of 512 bits? So if I say something like ```dec_len[3]

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 22, 2019


Original Redmine Comment
Author Name: Tim Allen
Original Date: 2019-11-22T20:35:12Z


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

logic [2nd][1st] var [4th][3rd]

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Nov 22, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-11-22T23:13:10Z


You're right, an easy cross check:

  $display("%d", $size(o_dec_table.dec_len));

  16

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.