Register size inference

[schoeberl]

The following code should size a register for 4 bits by using an UInt constant:

val reg = Reg(UInt(15)) reg := reg + UInt(1) io.result := reg

However, the resulting register is a single bit. Both in simulation and in the Verilog code.

Find the test case at: https://github.com/schoeberl/chisel-examples/blob/master/examples/src/main/scala/questions/RegSize.scala

run the simulation with: sbt "run-main questions.RegSizeTester" gtkwave RegSize.vcd or the Verilog generation with sbt "run-main questions.RegSize"

[aswaterman]

Reg(t) creates a Reg with the same type as t. For these purposes, UInt(15) and UInt() are the same. Whether or not this is desirable behavior is debatable, but as of this moment it's technically correct.

On Tue, Feb 9, 2016 at 10:22 PM, Martin Schoeberl notifications@github.com wrote:

The following code should size a register for 4 bits by using an UInt constant:

val reg = Reg(UInt(15)) reg := reg + UInt(1) io.result := reg

However, the resulting register is a single bit. Both in simulation and in the Verilog code.

Find the test case at: https://github.com/schoeberl/chisel-examples/blob/master/examples/src/main/scala/questions/RegSize.scala

run the simulation with: sbt "run-main questions.RegSizeTester" gtkwave RegSize.vcd or the Verilog generation with sbt "run-main questions.RegSize"

— Reply to this email directly or view it on GitHub https://github.com/ucb-bar/chisel/issues/654.

[schoeberl]

If the bit width of an UInt is not part of the type (has no meaning in Reg(UInt(n))) why is the register width than 1? Could be 0, or anything? Has Reg(T) without a init or next value any meaning then? If not I would drop that constructor.

[aswaterman]

The width of reg can be thought of as the minimal value that solves the constraint problem

width(reg) >= width(reg + UInt(1))

where

width(reg + UInt(1)) = max(width(reg), width(UInt(1)))

so width(reg) = width(UInt(1)) = 1. Perhaps not intuitive, but definitely not arbitrary.

On Tue, Feb 9, 2016 at 11:20 PM, Martin Schoeberl notifications@github.com wrote:

If the bit width of an UInt is not part of the type (has no meaning in Reg(UInt(n))) why is the register width than 1? Could be 0, or anything? Has Reg(T) without a init or next value any meaning then? If not I would drop that constructor.

— Reply to this email directly or view it on GitHub https://github.com/ucb-bar/chisel/issues/654#issuecomment-182235402.

[schoeberl]

Is there a use case for creating a register with Reg(T) when the size is reduced to a single bit?

The current behavior is at least surprising, in the worst it can lead to hard to detect bugs in user code. I would call this than as "code to avoid". Something we need to teach when using VHDL due to VHDL language issues. Something I would love to see minimized with Chisel.

I would argue to get the width right from the UInt constant or drop this single parameter constructor (factory). I remember at some version of Chisel Reg(v) was a shorthand for Reg(next = v). Why was this dropped?

[shunshou]

Just noticed this, but I (and at least one other person) ran into similar issues a couple of months ago. I think you should just not make Reg accessible to Chisel users, because I was using Reg(x), assuming it'd act like Reg(next=x) and nothing behaved the way "I expected." RegNext and RegInit are sufficient...

Takeaway: don't support constructs that might be misleading (unless people have a thorough understanding of the Chisel internals -- but requiring that would be bad for language adoption IMO), because debugging is frustrating. :(

[aswaterman]

On Sat, Mar 5, 2016 at 12:54 PM, Angie Wang notifications@github.com wrote:

Just noticed this, but I (and at least one other person) ran into similar issues a couple of months ago. I think you should just not make Reg accessible to Chisel users, because I was using Reg(x), assuming it'd act like Reg(next=x) and nothing behaved the way "I expected." RegNext and RegInit are sufficient...

In this proposal, how do you create a register with no reset value that is only conditionally updated?

Takeaway: don't support constructs that might be misleading (unless people have a thorough understanding of the Chisel internals -- but requiring that would be bad for language adoption IMO), because debugging is frustrating. :(

— Reply to this email directly or view it on GitHub https://github.com/ucb-bar/chisel/issues/654#issuecomment-192733920.

[shunshou]

Conditional update is essentially just a DFF reg with input mux. Either you conditionally update to a new value or you pass the output back to the input. (I'm not really sure how tools are able to infer clock gates from enables, so whether this works for that, I don't know...)

As long as the value passed into the register doesn't depend on the register's previous value (i.e. not a counter), you just assume that the register will have !@#$ until the enable is set at least once. Then the state is always defined after that... I think that's a fair assumption. If you wanted a counter, then you'd have to specify the initial value with RegInit. Something like:

val out = UInt(OUTPUT,width=5) [assuming an IO] val next = Mux(io.cond,io.in,io.out) io.out := RegNext(next)

• I'm using a custom RegNext that behaves as follows:

val temp = Reg{T}(Some(next), Some(next), None, Some(clock)).toBits (square brackets are giving me problems in markdown...) val out = chiselCast(temp){next.cloneType}

Basically, think of the logic as a signal flow graph, with operations and nodes. Fundamentally, a Reg is just converted to a DFF (operation) in hardware... At the most basic level, you just need to spec the data in, data out, and clk (nodes).

How it works is exactly spelled out, and it'll error out if you try something that shouldn't be supported. I'd rather get a ton of compile-time errors than think something worked, only to find out that the output was mathematically incorrect because of some intermediate width inference (or other Chisel assumption) I wasn't familiar with. And I've been running into the latter problem frequently enough with various aspects of Chisel...

[shunshou]

Also, I haven't tried using when instead of Mux, without defining a default case i.e. the feedback. I assume it will derp just like it derps when you don't specify default values for when's w/ combinational logic.

Maybe defining a default condition for registers is too much additional extra coding for people, b/c Verilog + VHDL properly infer it, but I'd sacrifice verbosity for always getting the obvious result.

[ccelio]

On Sat, Mar 5, 2016 at 12:54 PM, Angie Wang notifications@github.com wrote:

Just noticed this, but I (and at least one other person) ran into similar issues a couple of months ago. I think you should just not make Reg accessible to Chisel users, because I was using Reg(x), assuming it'd act like Reg(next=x) and nothing behaved the way "I expected." RegNext and RegInit are sufficient...

In this proposal, how do you create a register with no reset value that is only conditionally updated?

I'd really love for a compiler warning (error?) that catches use of Reg(x), where x is not a type, but a literal.

Reg(UInt(0))

Should be invalid, when the user really wants is:

Reg(UInt())

I'd also prefer that if users want to get the type of a signal, they explicitly ask for it:

Reg(io.a) // bad, easily confused with RegNext(io.a)

Reg(io.a.clone()) // intention is clearer (not sure .clone() does what I want but you get the idea).

I admit I don't know if my proposal is doable, or even desirable. But I've been looking through some code, and I've found a number of Reg(io.something.bits), and I find that construct very misleading.

[shunshou]

At least for me, I definitely typed something like Reg(io.a) expecting it to behave like RegNext(io.a)... I assume that's actually a fairly high frequency mistake. If I did something like Reg(io.a.type) <-- or in my case, I'd use something like Reg(io.a.cloneType), I'd really hope that any width information associated with io.a is also passed along with the type.

For RegNext, it makes sense to do that, b/c the width of the input/output of a reg shouldn't be different (regardless of how many bits of the output you actually use down the chain).

[shunshou]

As a side note, again should probably go in a new issue, but I think the default + might be better off as the current +&. If you wanted to use functional programming i.e. fold to add a chain of #'s together, if you don't properly spec out the max bitwidth of the output and make sure that at least one of the inputs has that bitwidth (padding or similar), you'll likely get an incorrect value. And it's also one of those hard to debug problems... But really, you want + to just work and get the right sum...

There is still some value to keeping the + functionality (I keep it as +% to remind the user that it'll wrap on overflow).

Obviously, +& is sub-optimal as far as resource utilization is concerned, and that's when range inference becomes important... But first and foremost, something should be functionally correct.

[JackDavidson]

So it looks like this is essentially the same issue as https://github.com/ucb-bar/chisel/issues/668

I posed a proposed solution that will change what is currently done by Reg(UInt(##)) into Reg(UInt), and make Reg(UInt(##)) do what most users are expecting