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

Icarus Verilog creates huge in-memory arrays for shifts with large rhs #13

Closed
cliffordwolf opened this issue Feb 17, 2014 · 7 comments
Closed

Comments

@cliffordwolf
Copy link
Contributor

Icarus Verilog allocates 16 GB of memory when processing the following statement:

module issue_023;
  localparam [4:0] p = 1'b1 << ~30'b0;
endmodule

Adding another 10 bits to the RHS triggers an assert: ivl: verinum.cc:370:
verinum::V verinum::set(unsigned int, verinum::V): Assertion 'idx < nbits_'
failed:

module issue_023;
  localparam [4:0] p = 1'b1 << ~40'b0;
endmodule
@martinwhitaker
Copy link
Collaborator

Steve, can you comment on this. The underlying problem is that the operators in the verinum class operate losslessly, even when operating on sized vectors. Is this still needed? I think that, generally, when we have sized vectors, we want to follow the standard Verilog rules for expression widths.

@steveicarus
Copy link
Owner

I do not understand now that shift creates huge amounts of memory. Worst case, it would allocate space for 41 bit4_t bits. The result should then be truncated to 5 bits when assigned to p, so what is the problem?

@martinwhitaker
Copy link
Collaborator

~40'b0 = 2**40 - 1.

Granted this is an artificial test case, but I guess we should try to stop the compiler blowing up - even if we just output a "did you really mean this?" error message.

@steveicarus
Copy link
Owner

doh!

I'd certainly go with the "did you really mean this?" warning at the very least, because, well, that's nuts. Beyond that, yeah Verilog has rules for this: the left operand of the expression is sized so the expression should not be processed losslessly.

@caryr
Copy link
Collaborator

caryr commented Feb 18, 2014

For the case that we have an actual L-value width or left operand width can we truncate the shift at the appropriate place relative to this width to make this specific code example work correctly? One example that I believe cannot be fixed following the Icarus rules is localparam p = 1 << ~40'b0; since both p and the left numeric constant are not sized.

@martinwhitaker
Copy link
Collaborator

I've just pushed a fix for this to the git master branch. The arithmetic operators in the verinum class now follow the standard Verilog rules for calculating the result size if their operand(s) are sized. If any operand is unsized, they produce lossless results as before, so as Cary
notes, the expression 1 << ~40'b0 will still cause problems. I guess this needs the "did you really mean this?" warning.

@martinwhitaker
Copy link
Collaborator

I've now implemented a cap on the width of unsized expressions. If an unsized expression width exceeds the cap value and also exceeds the context width (if any), a warning message is output and the expression width is capped at that value. I've set the default cap value to 65536 bits, but it can be changed by the user in the same way as the integer width can be changed (note to self - haven't tested changing the cap width yet).

Feel free to argue for a different default setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants