Corrected bit width of indexes into wires in verilog

[JackDavidson]

Currently, verilog generated by CHISEL explicitly sets the width of bits used to index into wires/registers.

However, rather than the bit width of the index being set by the width of the wire being indexed into, the bit width is simply set to the minimum that it needs to be in order to hold the index number.

for example: wire io_addr[31:0] assign T5 = io_addr[2'h2:1'h0]; // <= should be [5'h2 : 5'h0] OR ['h2 : 'h0] OR [2 : 0]

Note that there are actually three ways the issue could be solved:

1. Do not explicitly set bit widths of indices ('h2 instead of 2'h2)
2. Use base-10 numbers (2 instead of 2'h2)
3. Use the proper bit-withs, log_2(with_of_wire), rounded up.

This pull request fixes the issue by using option number 3, simply using the correct bit-widths. However, there is no reason we need to explicitly set a width at all.

I am submitting this pull request for now, but if anyone thinks that solving the issue using options 1 or 2 would be better, I am completely willing to redo the changes in either way.

see also: https://github.com/ucb-bar/chisel/pull/681

only either this (hex) or the other (base 10) should be accepted

[ghost]

Can one of the admins verify this patch?

[aswaterman]

(1) requires SystemVerilog. I guess I'd favor (2), but (3) works, too.

[ucbjrl]

ok to test

[ucbjrl]

This does seem like a verilator bug based on the fact that vcs doesn't complain. I don't think one can argue that the bit widths of the literals are incorrect, but I agree we shouldn't annoy verilator if a simple change will satisfy it. Are there any downstream tools that require explicit bit widths for literals? If not, I second the suggestion to use solution (2).

[aswaterman]

@ucbjrl since not specifying the widths on bit-extract is the normal idiom for hand-written Verilog, I'd expect all the backend tools to be OK with this.

[sdtwigg]

Does the Verilog spec require that bit-extraction widths either stay unspecified (although I speculate the spec ensures the eventual size is >=32 given how often Verilog assumes integers are 32-bit numbers) or be >= the minimum addressable width for that net?

[JackDavidson]

I actually also agree that solution (2) is the most ideal choice, as long as it does not break anything. I think it makes the generated code slightly more readable.

As far as the verilog spec goes, I don't think that there are any requirements, though I'm not certain. This is more about working nicely with verilator and simply having a nice and proper verilog output :)

[ucbjrl]

obsoleted by #681