-
Notifications
You must be signed in to change notification settings - Fork 574
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
Counter-intuitive user experience (UX) for simple constant addition #1144
Comments
Apparently this dates back to Chisel 2 design decisions. Some old discussion here: ucb-bar/chisel2-deprecated#672. This is more about suggesting that 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. |
Maybe you should change the bootcamp to explicitly state the widths of UInt literals. Is that what users are expected to do...? |
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 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 I don't know the right answer to this (and lots of people will probably have different opinion), but my thoughts:
|
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. |
I would say that their solution is ... not great.
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. |
Perhaps the most reasonable approach is documentation and to stylistically encourage use of |
We discussed this at the dev meeting today. Highlights:
@edwardcwang if you think this is the right solution, you've been volunteered to PR this |
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 It does feel like a sufficiently complex Linter could catch something here (e.g., for 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. |
Fix FileUtils.getLines, add simple FileUtils tests
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 to0
not8
, since4.U
is assigned a width of3
by Chisel (i.e.4.U
is identical to4.U(3.W)
), and the default+
operator is width-truncating.We actually need to use the expression
4.U +& 4.U
to express8.U
, which can be a bit counter-intuitive. It would be good if4.U + 4.U
could mean4.U(inf.W) + 4.U(inf.W) --> 8.U(inf.W)
rather than4.U(3.W) + 4.U(3.W)
, for which the+
operator is doing a spectacular job.(Do width-less UInts actually exist?)
Generated circuits for reference below:
The text was updated successfully, but these errors were encountered: