Width should grow when adding

[da-steve101]

The following example fails as the width of the output of the add is 3 not 4

class Tmp extends Module {
 val io = new Bundle {
   val a = UInt(INPUT, 3)
   val b = UInt(INPUT, 3)
   val c = UInt(OUTPUT, 4)
 }
 io.c := io.a + io.b
}

class TmpTests( c : Tmp ) extends Tester(c) {
 poke( c.io.a, 3 )
 poke( c.io.b, 3 )
 step(1)
 expect( c.io.c, 6)
 poke( c.io.a, 5 )
 poke( c.io.b, 3 )
 step(1)
 expect( c.io.c, 8)
 poke( c.io.a, 6)
 poke( c.io.b, 6)
 step(1)
 expect( c.io.c, 12)
}

[sdtwigg]

This is intentional behavior in the current specification. If you want bit-width extension for addition then you have to manually extend one of the inputs yourself.

[sdtwigg]

Reopening, renaming, and retagging since you it is a more debatable issue whether that should be the current specification

[shunshou]

This is what I was commenting on in my last post @ #654. There's a new add operation in Chisel (even in Chisel 2, I think) that is +&, which will always bit extend, so you'll always get the right result.

I think that the default + should always use +& for functional correctness. BUT, this is overly conservative if you know for a fact that the sum will never require the extra bit. In this case, having a range inference mechanism is important, which is what I've tried doing in ChiselDSP.

The annoying, unintuitive thing about bit extending the inputs (which works) is that, as a first-time Chisel designer (sometime last year), I thought that specifying the output to have enough bits would be enough. I thought it would automatically bit extend the inputs if my output was clearly 1 bit wider, but that was apparently a terrible assumption to make. I'm pretty sure that this is a common mistake for people just starting out with Chisel... and unfortunately, in order to know that this wouldn't work, you'd have to be comfortable enough going through Chisel internals (or I guess the cheatsheet, with its hints on how bitwidth inference is made) to know...

== again, terrible for usability; bad for language adoption.

I had this discussion some months ago; I recall that the default Verilog/VHDL behavior is to bit extend add. But again, this is overly conservative, as you can't expect the tools to know more specifically whether only a range of [0 to 5] might ever be fed into a 3 bit input, unless you provide ranging info from your own high-level insights.

My custom data types use the following:

• + (does range inference and checks to see if the output needs to be bit extended)
• +& (which always bit extends)
• +% (which always wraps on overflow -- which is sometimes useful)

[da-steve101]

I think the better approach is to extend the range, If it is not needed it can be optimized out by the synthesizer. I imagine changing the default to not extend would break a few things. When was this changed? Is it in the release?

[sdtwigg]

The behavior has essentially always been not to extend for addition.

[shunshou]

Yeah, it was a poor choice for defaults... And depending on how complicated your circuit is, the synthesizer will not always be smart enough to optimize the unused bits.

[aswaterman]

As an addendum, it was never for performance reasons. It's largely because the language was designed and adopted by processor architects, where modular arithmetic is almost always the desired behavior, than by DSP folks, who prefer mathematical addition for the obvious reasons.

On Wed, Mar 9, 2016 at 11:12 PM, Stephen Twigg notifications@github.com wrote:

The behavior has essentially always been not to extend for addition.

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

[shunshou]

Yeah, I figured as much. Chisel just needs to be extended a bit for DSP. :P Depending on what you're architecting, it might be better to extend or not. It might also be better to represent stuff with bitwidths in one case and ranges in another. The particular functionality just needs to separated out/clarified.

[da-steve101]

Ok, ill need to look over my code then :)

[aswaterman]

"extended a bit for DSP" -- nice one, @shunshou :p

[seldridge]

My two cents: at present, Chisel and FIRRTL seem to be in disagreement about this. The FIRRTL spec defines an add as always increasing the bit width by one, while Chisel does not. I realize that these two don't have to be agreement and that both Chisel and FIRRTL are moving targets, but it would seem to be like a good idea if the convention (whatever that may happen to be) is the same throughout.

[CuppoJava]

Just to chime in: The FIRRTL spec is defined in such a way to ensure that both behaviors can be encoded using FIRRTL's primitive constructs. This way, Chisel developers can choose which behaviour they want by simply emitting different FIRRTL code and it does not require any changes to FIRRTL. It also allows other FIRRTL frontends (e.g. perhaps one that specializes on DSP) to emit FIRRTL code that corresponds nicely to DSP conventions.

-Patrick

On Thu, Mar 10, 2016 at 6:50 AM, Schuyler Eldridge <notifications@github.com

wrote:

My two cents: at present, Chisel and FIRRTL seem to be in disagreement about this. The FIRRTL spec defines an add as always increasing the bit width by one, while Chisel does not. I realize that these two don't have to be agreement and that both Chisel and FIRRTL are moving targets, but it would seem to be like a good idea if the convention (whatever that may happen to be) is the same throughout.

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

[shunshou]

I think that's the right approach, mostly because you can always trim out the extra bit when it's not needed, but if you don't have the MSB in the first place, you can't get access to carry out unless you but extend the inputs, which is not as intuitive...

On Thursday, March 10, 2016, CuppoJava notifications@github.com wrote:

Just to chime in: The FIRRTL spec is defined in such a way to ensure that both behaviors can be encoded using FIRRTL's primitive constructs. This way, Chisel developers can choose which behaviour they want by simply emitting different FIRRTL code and it does not require any changes to FIRRTL. It also allows other FIRRTL frontends (e.g. perhaps one that specializes on DSP) to emit FIRRTL code that corresponds nicely to DSP conventions.

-Patrick

On Thu, Mar 10, 2016 at 6:50 AM, Schuyler Eldridge < notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');

wrote:

My two cents: at present, Chisel and FIRRTL seem to be in disagreement about this. The FIRRTL spec defines an add as always increasing the bit width by one, while Chisel does not. I realize that these two don't have to be agreement and that both Chisel and FIRRTL are moving targets, but it would seem to be like a good idea if the convention (whatever that may happen to be) is the same throughout.

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

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