Counter-intuitive user experience (UX) for simple constant addition

[edwardcwang]

Type of issue: bug report / other enhancement

Impact: unknown

Development Phase: request

Other information:

Food for thought.

Consider the following simple Chisel expression (which is actually present in the bootcamp): 4.U + 4.U. Surprisingly, it evaluates to 0 not 8, since 4.U is assigned a width of 3 by Chisel (i.e. 4.U is identical to 4.U(3.W)), and the default + operator is width-truncating.

We actually need to use the expression 4.U +& 4.U to express 8.U, which can be a bit counter-intuitive. It would be good if 4.U + 4.U could mean 4.U(inf.W) + 4.U(inf.W) --> 8.U(inf.W) rather than 4.U(3.W) + 4.U(3.W), for which the + operator is doing a spectacular job.

(Do width-less UInts actually exist?)

class MyOperators extends Module {
  val io = IO(new Bundle {
    val in      = Input(UInt(4.W))
    val wtf     = Output(UInt())
    val why     = Output(UInt())
    val bbq     = Output(UInt())
  })
  // tail(add(UInt<3>("h04"), UInt<3>("h04")), 1) == 0
  io.wtf := 4.U + 4.U
  // add(UInt<3>("h04"), UInt<3>("h04")) == 8
  io.why := 4.U +& 4.U
  io.bbq := 4.U(4.W) + 4.U(4.W)
}

Generated circuits for reference below:

circuit MyOperators : 
  module MyOperators : 
    input clock : Clock
    input reset : UInt<1>
    output io : {flip in : UInt<4>, wtf : UInt, why : UInt, bbq : UInt}

    node _T = add(UInt<3>("h04"), UInt<3>("h04")) @[cmd23.sc 8:17]
    node _T_1 = tail(_T, 1) @[cmd23.sc 8:17]
    io.wtf <= _T_1 @[cmd23.sc 8:10]
    node _T_2 = add(UInt<3>("h04"), UInt<3>("h04")) @[cmd23.sc 9:17]
    io.why <= _T_2 @[cmd23.sc 9:10]
    node _T_3 = add(UInt<4>("h04"), UInt<4>("h04")) @[cmd23.sc 10:22]
    node _T_4 = tail(_T_3, 1) @[cmd23.sc 10:22]
    io.bbq <= _T_4 @[cmd23.sc 10:10]

module MyOperators(
  input        clock,
  input        reset,
  input  [3:0] io_in,
  output [2:0] io_wtf,
  output [3:0] io_why,
  output [3:0] io_bbq
);
  assign io_wtf = 3'h0; // @[cmd23.sc 8:10]
  assign io_why = 4'h8; // @[cmd23.sc 9:10]
  assign io_bbq = 4'h8; // @[cmd23.sc 10:10]
endmodule

[seldridge]

Apparently this dates back to Chisel 2 design decisions. Some old discussion here: https://github.com/ucb-bar/chisel2-deprecated/issues/672. This is more about suggesting that + should map to +& as opposed to +%.

This situation here is subtly different as the intuitive behavior when adding literals is that width expands and it may be okay if that's different from the default behavior when adding hardware types. No clue how to handle addition that mixes literals and hardware types and to do that in an intuitive way.

There was some discussion a while ago about literals having infinite width or a way of expressing literals differently. Some exploration of that might help.

If anything, at least the current situation has consistent behavior. + always means +%. Perhaps it's worth being very explicit about this, and how literal widths are determined, in the bootcamp.

[mwachs5]

Maybe you should change the bootcamp to explicitly state the widths of UInt literals. Is that what users are expected to do...?

[ducky64]

Infinite width operations were proposed in #867 with a PR in #869. Ultimately, that PR broke backwards compatibility with what I would call a bug, so it kind of stalled. (I vaguely recall other technical issues with infinite width? though not sure)

I'd argue this is less of a truncating/non-truncating plus issue and more of width inference rules, especially for literals without an explicit width. For example, I'd imagine it would be uncontroversial to write:

4.U(3.W) + 4.U(3.W)

and expect zero.

But less intuitive are these cases:

4.U(3.W) + 4.U
4.U + 4.U

even if you knew the width resolution rules for +, you'd need to know the width assigned to the literal, and that 4.U doesn't mean a number, it's actually filling in a concrete width implicitly - which may be the underlying point of confusion.

Also worth considering would be:

val a = RegInit(4.U(3.W))
a := a + 4.U

though perhaps isn't confusing since you're updating a register with a defined width (there's no expected inference on a := a + 4.U).

I don't know the right answer to this (and lots of people will probably have different opinion), but my thoughts:

• Compare to what this should do in Scala-land and Chisel-land - I'd imagine new users to be programming by pattern matching and analogy (probably second only to programming by StackOverflow). In this case, should:

  (4+4).U === 4.U + 4.U

  ?

• What about similar behavior in Verilog and VHDL (is there even literal width inference there at all? - if not, they completely sidestep this problem)? If we expect new users to mainly come from a hardware engineering background, preserving similar behavior might practically result in the least confusion.
• If we want 4.U + 4.U === 8.U, I think infinite width is a good way of conceptually explaining it.

[aswaterman]

Regarding "what should this do in Scala-land":

    BigInt((1 << 30) + (1 << 30)) != BigInt(1 << 30) + BigInt(1 << 30)

Analogous issues exist, it's just that Scala's integral types are more spaced out than ours, so people are a lot less likely to run into them.

[albert-magyar]

What about similar behavior in Verilog and VHDL (is there even literal width inference there at all? - if not, they completely sidestep this problem)? If we expect new users to mainly come from a hardware engineering background, preserving similar behavior might practically result in the least confusion.

I would say that their solution is ... not great.

The number of bits that make up an unsized number (which is a simple decimal number or a number without the size specification) shall be at least 32. -- IEEE 1364-2001

I agree that there are two separate facets here: inferred widths of literals and widths of intermediate expressions. For the inferred widths of literals, Verilog shows its bizarre affinity for 32-bit values. On the "widths of intermediate expressions" side, Verilog has a remarkably complex set of rules that sometimes Does What I Mean but sometimes produces real head-scratchers.

[jackkoenig]

Perhaps the most reasonable approach is documentation and to stylistically encourage use of +& and +% over +, especially in the bootcamp. Then when people encounter behavior they find surprising there is a pretty big hint in the code.

[ducky64]

We discussed this at the dev meeting today. Highlights:

• We considered deprecating + in favor of making people explicitly use +% or +&. Probably not practical, at all.
• In general, backwards compatibility is a barrier to changes to inferred width behavior. (but maybe people have ideas for solutions here that are safe-ish?)
• There was also the idea floated of doing a gotchas section in the Chisel bootcamp, or populating StackOverflow with gotchas. Reasons it might be ineffective: you see it once, forget about it, and still end up slogging through debugging later. Reasons for: every language has gotchas, we might not be able to solve every one cleanly.
• The most practical solution seems to be to detect these antipatterns and generate warnings or errors. The concrete idea was to detect operations involving two unspecified (inferred) width literals that leads to truncation, and warn on that (or have a compile option that turns those into errors). Not a perfect solution (having non-literal intermediates would result in a false negative, among other things), and there would need to be ways to suppress the warning without changing behavior (specify an explicit width should suppress the warning).
• All that being said, since this is the first time this has come up compared to how long Chisel has been around, maybe it's not that big of an issue?

@edwardcwang if you think this is the right solution, you've been volunteered to PR this :trollface: but feel free to delegate or reassign if you'd like.

[ccelio]

If I may add my own two cents; this is something that has (very rarely) come up in my team, and we added this pattern to our code-review process. I.e., if someone writes + 1.U, especially as part of compound statement (x < a + 1), put cognitive attention towards it. Since this shows up in <0.1% of our code base, this isn't onerous (we often rely/prefer wraparound behavior of the default + anyways, and in other cases we write helper functions to manage more complex wraparound semantics). I'd also like to point out that it doesn't really matter what the default behavior is, or even if you completely eliminate + from the language, this cognitive overhead during code review will be ever-present.

It does feel like a sufficiently complex Linter could catch something here (e.g., for x < a + 1, and x and a are the same width, you almost certainly have a problem!).

And yes, I think Chisel could benefit from a style-guide and a code-review guide that should be introduced fairly early in the curriculum.