Remove implicit width truncation / Don't repeat Verilog's assignment issue in Chisel

[schoeberl]

I just realized that assignment of a larger width signal to a small width signal results in a silent truncation in Chisel. This is one of the worst issues in Verilog and modern synthesize tools should issue a warning as such an assignment is usually a bug.

Please, please, do not allow this in Chisel! The current behavior is even worse than Verilog as the generated Verilog code will NOT generate the warning.

Just had a meeting with a HW design company about presenting Chisel to them as they might need to switch language after a merger. And this Verilog assignment issue was their main argument against Verilog! So they would then not accept Chisel as well.

Type of issue: bug report

Impact: API modification (but a reasonable one)

[ducky64]

Although this never made it to a GitHub issue, there was some discussion during the 4/19 chisel-dev meeting about stricter truncation rules.

Ideas brainstormed:

• Need to ensure backwards compatibility somehow.
• Opt-in options: scope? WithStrictConnect mixin? separate strong-connect operator? import chisel3.strict? aspects?
• Implementation: possibly a strict connect operation in FIRRTL?

For what it's worth, the resolution was for someone to write a RFC, which apparently never happened.

[seldridge]

So, the super cool thing about Chisel is that you're not locked into this behavior. Consider the following:

class Foo extends RawModule {
  val a = IO(Input(UInt(4.W)))
  val b = IO(Output(UInt(3.W)))
  b := a
}

This gives you the following Mid FIRRTL:

circuit Foo :
  module Foo :
    input a : UInt<4>
    output b : UInt<3>

    skip
    b <= a

And the following Low FIRRTL (with the silent truncation):

circuit Foo :
  module Foo :
    input a : UInt<4>
    output b : UInt<3>

    b <= bits(a, 2, 0)

If this was something that a company really cared about, it is totally reasonable to write a custom FIRRTL transform (or for the Chisel/FIRRTL devs to make this available that can be optionally used) that will either print a warning or throw an error. So, you get the best of all worlds. With Verilog, it's whatever the standard says and whatever the vendor decides to implement.

[aswaterman]

I think there's general agreement that was a short-sighted decision, but it's too pervasive to change. I don't think mandatory warnings are a practical approach either, also because of pervasiveness in existing code.

chisel3.strict and/or a Firrtl transform both seem like viable solutions.

[ccelio]

I agree that this is an issue and some sort of opt-in system (I really like importing chisel3.strict) is needed. The more lint-like things we can move into chisel-time the better we are over verilog! =)

[jackkoenig]

As much as I hate the similarity to use strict in Perl and Javascript, I also vote for import chisel3.strict, I think before adding it we want to do a really good audit to see what things should be included.

My thought on this from the FIRRTL perspective is that we should just change FIRRTL to make <= strict so that when other frontends emit FIRRTL they get the more desirable behavior. From the Chisel perspective, we just emit <- for everything. Since import chisel3._ already splits connections out into leaf-level <= connections, we can just switch all of them to <- and thus the normal chisel3._ connection semantics (strict in the sense of fields much match, weak in the sense of implicit truncation) remain the same.

[jackkoenig]

(Or perhaps if we want to add this sooner, we make it import chisel3.experimental.strict so that users just accept that new things may be added 😈 )

[seldridge]

Width inference should relegate this to a FIRRTL-time check, yes?

Changing FIRRTL semantics to be strict seems to make sense to me. Similarly, enabling a new check that runs by default would necessitate a change to the spec. I do get a little nervous about allowing (compiler) front-end developers to control middle-end checks based on IR emission, however. I'd prefer explicit middle-end options/annotations.

Basically, I'd like the following behavior:

firrtl -Wall -Werror             # turn on all warnings, all warnings are errors
firrtl -Werror=silent-truncation # turn on silent truncation warnings
firrtl -Wno-silent-truncation    # turn off silent truncation warnings

The set of checks enabled by default would be defined by the spec. Warnings would be other things indicating potential user error and can be enabled/disabled with the above CLI that is backed by execution of specific transforms.

Concretely, the above options amount to porcelains around the following:

• RunFirrtlTransformAnnotation(new CheckSilentTruncation)
• CheckSilentTruncationOptions(fatal: Boolean)

I've done some playing around with the dependency API and turning the existing checks on/off or running checks more aggressively (e.g., I would like to be able to either (1) not run CheckTypes, (2) run CheckTypes only once, or (3) run CheckTypes after every InferTypes). I got stuck with that, but I think @jackkoenig's testing of the dependency API led to a bug fix that makes this possible.

[schoeberl]

On 22 Aug, 2019, at 23:34, Andrew Waterman notifications@github.com wrote:

I think there's general agreement that was a short-sighted decision, but it's too pervasive to change. I don't think mandatory warnings are a practical approach either, also because of pervasiveness in existing code.

Is it really so pervasive? Do people really write such code? I would consider this a design error in user code. What about checking the Rocket code base how often this really happens? I think this braking change would be easy to fix in user code and would improve the code quality.

BTW, partial assignment was also dropped form Chisel 2 to Chisel 3 with a argument that this is wrong code, but that broke some of my code (which I considered not wrong code ;-)

chisel3.strict and/or a Firrtl transform both seem like viable solutions.

Agree chisel3.strict would be a solution (work around) as this is easy for the Chisel user. A Firrtl transformation sounds a little bit technical for the user.

Cheers, Martin

[jackkoenig]

Is it really so pervasive? Do people really write such code? I would consider this a design error in user code. What about checking the Rocket code base how often this really happens? I think this braking change would be easy to fix in user code and would improve the code quality.

I did check (well SiFive's codebase including rocket-chip under our normal regression parameterizations) and it is pervasive. It's not an easy fix because it's not something that is caught by the Scala type system; it's only caught if you pick the right parameterization. I wrote tools to collect where it was occurring (in those parameterizations) and did an audit of the code using it, including asking the writers of those lines--they said it was intentional.

BTW, partial assignment was also dropped form Chisel 2 to Chisel 3 with a argument that this is wrong code, but that broke some of my code (which I considered not wrong code ;-)

We should have dropped implicit truncation between Chisel 2 and Chisel 3, but we didn't. This is about API stability; once we cracked 3.0.0 there is a duty to users not to break their code. I am personally very against implicit truncation, but am even more against breaking backwards compatibility. I don't think import chisel3.strict is great, but in my opinion, it's the best solution presented so far.

[schoeberl]

On 24 Aug, 2019, at 3:05, Jack Koenig notifications@github.com wrote:

Is it really so pervasive? Do people really write such code? I would consider this a design error in user code. What about checking the Rocket code base how often this really happens? I think this braking change would be easy to fix in user code and would improve the code quality.

I did check (well SiFive's codebase including rocket-chip under our normal regression parameterizations) and it is pervasive. It's not an easy fix because it's not something that is caught by the Scala type system; it's only caught if you pick the right parameterization. I wrote tools to collect where it was occurring (in those parameterizations) and did an audit of the code using it, including asking the writers of those lines--they said it was intentional.

Thanks for clarifying and checking. I am very surprised that someone is using truncation intentional. I can hardly come up with a use case. And if so, using explicit subfield expression would better show the intention. Ok, I accept that SiFive’s codebase rules here ;-)

I know that Scala’s type system is not so much used in Chisel. Maybe intentional, maybe because it would be too restrictive. But I don’t know the history of Chisel enough.

BTW, partial assignment was also dropped form Chisel 2 to Chisel 3 with a argument that this is wrong code, but that broke some of my code (which I considered not wrong code ;-)

We should have dropped implicit truncation between Chisel 2 and Chisel 3, but we didn't. This is about API stability; once we cracked 3.0.0 there is a duty to users not to break their code. I am personally very against implicit truncation, but am even more against breaking backwards compatibility. I don't think import chisel3.strict is great, but in my opinion, it's the best solution presented so far.

+1 for chisel.strict. Would be a default in all my Chisel files.

Cheers, Martin

[ccelio]

Somewhat related, we found an issue raised by a Verilog linter with myvec(idx) where idx was purposefully one bit larger than log(myvec.size). The wrap-around case was a carefully crafted DontCare, but it would be nice for these types of issues to be caught earlier before making it to a Verilog linter and the designer should specify "I really meant to be clever here!" (why do Verilog linters cost $$$??! :rage:)

Back on topic, I don't think I have a strong opinion on whether this should be a code import, a chisel compiler flag, or a firrtl flag. I can see an argument being made that creating a chisel3.strict will simply move the goal posts on "legacy" and we'll have to re-litigate this argument on the next language design ~mistake~ issue we run into (although I kind of like it when the designer tells me in his code what his assumptions where when he wrote the code).

C has -std, -Wall, etc., in their compiler flags, but that's pretty heavy-handed across everything being compiled, no? It seems we need individual module/file control on the compiler flags (which is already a chisel capability that we use for things like strict IO assignments). Do other languages/linter tools provide individual file/class/module control outside of the code file itself (although yuck on a waiver file!)?

[aswaterman]

I agree it can’t be a global setting because it that would preclude mixing code written under the old regime while getting better error checking for new code.

Re: other languages: It’s not unusual for large C projects to have some parts compiled with stricter flags than other parts, because large C projects are often decomposed into separately compiled libraries.

On Sat, Aug 24, 2019 at 5:40 PM Christopher Celio notifications@github.com wrote:

Somewhat related, we found an issue raised by a Verilog linter with myvec(idx) where idx was purposefully one bit larger than log(myvec.size). The wrap-around case was a carefully crafted DontCare, but it would be nice for these types of issues to be caught earlier before making it to a Verilog linter and the designer should specify "I really meant to be clever here!" (why do Verilog linters cost $$$??! 😡)

Back on topic, I don't think I have a strong opinion on whether this should be a code import, a chisel compiler flag, or a firrtl flag. I can see an argument being made that creating a chisel3.strict will simply move the goal posts on "legacy" and we'll have to re-litigate this argument on the next language design mistake issue we run into (although I kind of like it when the designer tells me in his code what his assumptions where when he wrote the code).

C has -std, -Wall, etc., in their compiler flags, but that's pretty heavy-handed across everything being compiled, no? It seems we need individual module/file control on the compiler flags (which is already a chisel capability that we use for things like strict IO assignments). Do other languages/linter tools provide individual file/class/module control outside of the code file itself (although yuck on a waiver file!)?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/1163?email_source=notifications&email_token=AAH3XQWWUWWNV5ANNWPPWYLQGHIGFA5CNFSM4IOZGJQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5CJ5SQ#issuecomment-524590794, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH3XQVY3KY5Q2AB632CJYLQGHIGFANCNFSM4IOZGJQQ .

[seldridge]

Note: that because options are just a means to generate Annotations, it's really the Annotations that are controlling this in any stage/phase/transform downstream of when the Annotation was created. We then have at least a language for expressing strictness at arbitrary granularities that is preserved across circuit transformations (though fine granularity would benefit from Chisel-level annotators as opposed to baroque options generating annotations).

[ducky64]

This was discussed at the dev meeting, with ideas and concerns but no resolution.

• General feeling from this thread: we want to do this, but backwards code compatibility is an issue.
• chisel3.strict seems fine, but there is the concern that the next time we make a change, we'll need to create a bunch of new imports, eg chisel3.strict2, chise3.strict3, ad nauseum. As a solution, it doesn't scale, unless we're sure this is the last breaking change. This also potentially raises discoverability issues - the best and newest is essentially hidden instead of being the default.
• There was discussion on whether strict flags should be at the file-level (import) or project-level (compiler flags). A potential implementation path may be the CompileOptions infrastructure.
• An alternate solution might be more like Scala's approach, which is that API changes may break code, but there is a compatibility option (like -Xsource=...). Essentially, when you bump your chisel version, you might get a bunch of code breakages, in which case you can either fix them, or add a compatibility flag.
• Another similar idea was just for users not to upgrade the version of chisel they're building against, but it's probably not a good idea since we generally don't support old versions or do backports.
• The FIRRTL implementation is also an open question. Is it a good idea to have this be a transform, since it kind of affects circuit correctness?

[aswaterman]

I think we want a solution that allows people to mix both loose and strict code, so that people can write new code in the new regime while being able to use older libraries that conform to the old regime.

On Mon, Aug 26, 2019 at 2:38 PM Richard Lin notifications@github.com wrote:

This was discussed at the dev meeting, with ideas and concerns but no resolution.

• General feeling from this thread: we want to do this, but backwards code compatibility is an issue.
• chisel3.strict seems fine, but there is the concern that the next time we make a change, we'll need to create a bunch of new imports, eg chisel3.strict2, chise3.strict3, ad nauseum. As a solution, it doesn't scale, unless we're sure this is the last breaking change. This also potentially raises discoverability issues - the best and newest is essentially hidden instead of being the default.
• There was discussion on whether strict flags should be at the file-level (import) or project-level (compiler flags). A potential implementation path may be the CompileOptions infrastructure.
• An alternate solution might be more like Scala's approach, which is that API changes may break code, but there is a compatibility option (like -Xsource=...). Essentially, when you bump your chisel version, you might get a bunch of code breakages, in which case you can either fix them, or add a compatibility flag.
• Another similar idea was just for users not to upgrade the version of chisel they're building against, but it's probably not a good idea since we generally don't support old versions or do backports.
• The FIRRTL implementation is also an open question. Is it a good idea to have this be a transform, since it kind of affects circuit correctness?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/1163?email_source=notifications&email_token=AAH3XQTVKEVHAHRFDTDE3MDQGREMLA5CNFSM4IOZGJQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5FYOKA#issuecomment-525043496, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH3XQQ6FJDQVC5RGXVKR63QGREMLANCNFSM4IOZGJQQ .

[ccelio]

Thanks for the summary.

chisel3.strict seems fine [...] unless we're sure this is the last breaking change.

This is definitely an issue. I believe we are going to continue to find new ways to write broken code for quite a while and we will want to add additional protections to strict.

A potential implementation path may be the CompileOptions infrastructure.

Are there any concerns about CompileOptions? That's the current method for controlling strictness and it's a great way to slowly port legacy code to new standards.

I would really like to avoid any mechanism that 1) encourages legacy code to stay stuck in legacy, or 2) puts additional burden on the build system to manage libs/dependencies/compatibility-versioning (i.e., breaking apart a project into compatibility islands of strictness modes and linking together later).

The CompileOptions seems to have worked well so far for mixing Chisel2, looser Chisel3, and stricter Chisel3 semantics together in one build system.

[ducky64]

I'm not sure how it would be implemented yet, but the idea for chisel3. being latest and you have to specifically add compatibility flags would mean the flags need to apply on a per-project (think sbt project, not top chip design) basis. We can certainly do a per file basis (change all import chisel3. to import chisel3.compatibility_3_1 or something) but that's potentially a lot of code changes (even if trivially automate-able) per version bump.

I don't think there have been much concerns about CompileOptions as an infrastructure. I don't particularly want to expose it as a user-visible API though, since I'm not a fan of mix-and-match styles, but it should be fine as an implementation strategy for chisel3.strict or chisel3.compatibility.

I'm not sure what the solution for legacy code staying in legacy is, the general feeling I get is that refactoring for cleanliness generally isn't prioritized. I don't think anyone is actively campaigning that they like the old semantics, it more that no one wants to put resources into clean up. I think some Berkeley folks put a small effort into porting parts of rocket to chisel3, but that died from lack of interest? @chick might know more about that particular story?

[schoeberl]

On 27 Aug, 2019, at 0:00, Christopher Celio notifications@github.com wrote: Are there any concerns about CompileOptions? That's the current method for controlling strictness and it's a great way to slowly port legacy code to new standards.

Yes, it makes the build process more heavyweight, especially for beginners. I am really like import chisel3.strict. And adding later more restrictions to strict is OK, I think. When you use strict, you know that you use the latest, greatest save version of Chisel to protect you.

Martin

[albert-magyar]

And HDL developers will already have lots of experience with use strict; use warnings :)

[Martoni]

I strongly agree with @schoeberl . Import chisel3.strict should be a good solution. Is there a roadmap to integrate this functionality ?

[ducky64]

One issue with chisel3.strict is how that would scale as we realize other HDL constructs are antipatterns. Would chisel3.strict continually break code as we disallow more unsafe things? Or is it a one-shot thing (which seems short-sighted)? Or is there some middle ground?

[Martoni]

Would chisel3.strict continually break code as we disallow more unsafe things?

I think yes, but it's my opinion ;) If we don't want to "break" old code with chisel3.strict we don't use it.

[schoeberl]

On 18 Dec, 2019, at 20:38, Richard Lin notifications@github.com wrote:

One issue with chisel3.strict is how that would scale as we realize other HDL constructs are antipatterns. Would chisel3.strict continually break code as we disallow more unsafe things? Or is it a one-shot thing (which seems short-sighted)? Or is there some middle ground?

I am fine with breaking code when adding restrictions. But we should discuss those additions ;-)

Martin

[schoeberl]

As of telco 2/3: reduce operations (e.g., andR, orR) with zero width wires (signals) shall result in a runtime excpetion.

[aswaterman]

I'd recommend against that course of action. They're perfectly well-defined, and adding constraints like this increases boilerplate in generator code.

[schoeberl]

This is part of the strict discussion. That means this error comes only when including chisel3.strict, not in plain Chisel code.

[jackkoenig]

I'm not sure I agree it makes sense for strict either though, as Andrew said, it is perfectly well-defined and IMO intuitive since it matches the definition of the same concepts on empty collections

println("empty forall = " + Seq().forall { _ => false })
println("empty exists = " + Seq().exists { _ => true })

prints

empty forall = true
empty exists = false

[seldridge]

I don't like the argument of forall or exists being the reason why this should work. My concern is that this is a "reduction" and reductions (Scala's reduce or Haskell's fold1) error when given an empty list. @aswaterman has brought up a separate point that Seq.empty[Int].sum == 0. So, :woman_shrugging:.

I'd then argue that it's confusing that "reduction" in Chisel3 is really "fold with an initial value". I'm fine with this as it makes sense for generators, but I'm not big on the name as it is incongruous with what software engineers expect (and I realize that Chisel3 API decisions regarding default non-expanding addition was made such that it is what hardware engineers expect).

I'd probably propose the following:

1. Reduction operators in FIRRTL need to be explicitly defined on how they operate on zero-width wires.
2. I'd be in favor of having FIRRTL reduction operators error on zero-width wires.
3. The Chisel3 API for reduction operators should use FIRRTL reductions as opposed to custom reduction chains or muxes.
4. Chisel3 API may implement the reductions as folds with initial values and we document it as such.
5. I'd propose then that chisel3.strict doesn't include andR and instead uses foldAndTrue and foldOrFalse.

For (4), what I mean is val foo = bar.andR could emit as:

node foo = andr(cat(UInt<1>(1), bar))

[schoeberl]

What would be the result of an empty Wire on xorR?

I understand that some generators might emit zero-width wires. Is this a good generator? In most cases, I think a zero width wire (or register) is an error. Therefore, I would like that the strict annotation will catch this.

[aswaterman]

In general, the reductions should adhere to the axiom red(a ## b) === red(red(a) ## red(b)), so xorR should return 0 in this case.

[seldridge]

@schoeberl: Concretely, the implementations are the following:

implicit class Bitwise(a: Seq[Boolean]) {
  def andR: Boolean = a.foldLeft(true )(_ & _)
  def orR:  Boolean = a.foldLeft(false)(_ | _)
  def xorR: Boolean = a.foldLeft(false)(_ ^ _)
}

[jwright6323]

I think the boolean reductions returning their respective identities (1 for andR, 0 for orR and xorR) is most consistent with what both hardware and software engineers would expect.

For software: @seldridge mentioned @aswaterman has pointed out that sum on an empty Seq[Int] will return the additive identity, 0. Seq.empty[Int].product will similarly return the multiplicative identity (1). I would certainly expect other reduction operations to return the above (given A & 1 == A, A | 0 == A, and A ^ 0 == A).

For hardware: If I want to make a state machine that advances state when a compile-time number of widgets is finished coming out of reset, I can have that state transition trigger on the and reduction:

when(widgets.map(_.resetDone).andR) { state := nextState }

If you have none of those widgets, then I would expect that this condition is met (i.e. the andR = 1). Similar arguments could be made about a state machine that is taking produced data from a compile-time-programmable number of producers:

when(producers.map(_.hasData).orR) { state := processData }

These are obviously cartoons, but I think they get the point across.

[schoeberl]

On 3 Feb, 2020, at 22:57, Schuyler Eldridge notifications@github.com wrote:

@schoeberl https://github.com/schoeberl: Concretely, the implementations are the following:

def xorR: Boolean = a.foldLeft(false)( ^ )

I guess default false is an arbitrary decision. Default true would not be worse for xor. Maybe it would actually be more intuitive to be true, it is a form of OR anyway. No, I take this back. There is no useful default for XOR reduce, I think.

+1 for disallowing this with chisel.strict

Let’s make Chisel one of the safest and still efficient/productive hardware description languages ;-)

Martin

[aswaterman]

There are axioms that reductions should adhere to, and making xorR return 1 for a 0-width wire violates at least one of them. It's not arbitrary.

[jwright6323]

xorR is 1-bit add, so it follows that {}.xorR would return the additive identity, 0. I agree that it is not arbitrary.

[schoeberl]

I do not completely understand what you write ;-) However, I probably agree with you. When thinking back long time ago, I remember that there is an algebra (commutative rings or so?) where there is a ‘1’ element that does not change the result when applied. Therefore, I agree that the one element for the xor thing shall be ‘0’ ;-)

Still in favor to disallow reduction operations on zero width wires. Or better disallow zero width wires at all. +1 for chisel3.strict.

Martin

On 3 Feb, 2020, at 23:14, Andrew Waterman notifications@github.com wrote:

There are axioms that reductions should adhere to as associative operators, and making xorR return 1 for a 0-width wire violates at least one of them. It's not arbitrary.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/1163?email_source=notifications&email_token=AAE63GD3QZ62EQDJKRW7YJ3RBCJLTA5CNFSM4IOZGJQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKVTD4I#issuecomment-581644785, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE63GBIN6KWZONEWYTMSFLRBCJLTANCNFSM4IOZGJQQ.

[schoeberl]

Ok, we are getting into the philosophical world of logic ;-) In my world logical operations are defined for two operators. They are not defined for a “none/undefined” input. So following operation in my world resulting in undefined: undefined & value = undefined. I assume that VHDL does this as well. v being a value and X being undefined, I assume that following expressions are true in VHDL:

v and X = X v or X = X v xor X = X

Don’t know enough Verilog, but would it be different? Would it be

v xor X = v?

Current Chisel semantics would make this true, right?

Cheers, Martin

On 3 Feb, 2020, at 23:17, John Wright notifications@github.com wrote:

xorR is 1-bit add, so it follows that {}.xorR would return the additive identity, 0. I agree that it is not arbitrary.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/1163?email_source=notifications&email_token=AAE63GGUFKVDO3P36PEC2KDRBCJZLA5CNFSM4IOZGJQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKVTOVI#issuecomment-581646165, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE63GDANINQMQD6UJW5BVDRBCJZLANCNFSM4IOZGJQQ.

[schoeberl]

OT: Is the ## operator still recommended in Chisel? I really liked ## for concatenations, but it seemed to be deprecated. I don’t see it in any documentation. Still good and valid?

Cheers, Martin

On 3 Feb, 2020, at 22:54, Andrew Waterman notifications@github.com wrote:

In general, the reductions should adhere to the axiom red(a ## b) === red(red(a) ## red(b)), so xorR should return 0 in this case.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/freechipsproject/chisel3/issues/1163?email_source=notifications&email_token=AAE63GG2KTZCTJ25PWPG2ADRBCHDFA5CNFSM4IOZGJQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKVRKWI#issuecomment-581637465, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE63GH5ZJ7UO2QPS6OQGVDRBCHDFANCNFSM4IOZGJQQ.

[aswaterman]

I think that might be a doc bug. ## is what the chisel3 core provides; Cat (which was demoted to chisel3.util) is implemented in terms of ##.

[jwright6323]

Ok, we are getting into the philosophical world of logic ;-) In my world logical operations are defined for two operators. They are not defined for a “none/undefined” input. So following operation in my world resulting in undefined: undefined & value = undefined. I assume that VHDL does this as well. v being a value and X being undefined, I assume that following expressions are true in VHDL: v and X = X v or X = X v xor X = X Don’t know enough Verilog, but would it be different? Would it be v xor X = v? Current Chisel semantics would make this true, right? Cheers, Martin

This seems to be a separate discussion- I think "nothing" (e.g. an empty set) is distinct from "something that is unknown" (e.g. a set with one unknown element).

Also, v and X is not always X in Verilog: (1'b0 & 1'bx) == 1'b0. Similarly (1'b1 | 1'bx) == 1'b1. Anything xor'ed with X is X, but that to me is a property unrelated to the xor operation's identity element.

Getting back to zero-width wires: I like to use them and find that their existence lets me use many functional programming constructs that would otherwise require annoying if guards to check for emptiness. If they were disallowed in chisel3.strict I'd probably find myself not using chisel3.strict at all, which would be a shame, as I would like to avoid the truncation problem that started this thread.

[Martoni]

What is the status of this proposal ? Has there been any progress since the beginning of the year?

[jiegec]

It's 2022 now, any progress on this proposal?

[erlingrj]

I have spent many frustrating hours debugging waveforms only to find that the culprit is a missed ".W" in a UInt instantiation. I.e. 0.U(8.W) vs 0.U(8). When hardcoding the width it is quite easy to spot but often I am using a more complicated statement e.g. "0,U(log2Ceil(config.property.length)) it is quite easy to overlook the missing "W".

I have a routine where I search for ".U(" in my project to track down these typos and I often find them. A warning of some sorts if you pass an Int to the UInt factory instead of a Width would be really great

[nrother]

Interestingly, there is a comment describing exactly this (unwanted) behavior. Despite from that, the method U is implemented without a parameter list...

    * Implementation note: the empty parameter list (like `U()`) is necessary to prevent
    * interpreting calls that have a non-Width parameter as a chained apply, otherwise things like
    * `0.asUInt(16)` (instead of `16.W`) compile without error and produce undesired results.

BTW: I guess this topic is unrelated to the original issue, or am I missing something?

[mwachs5]

The 9.U(0) issue is a known one that has worked its way to the top of the queue right now... but agree that is different than what this issue is talking about.

[dbear496]

If implicit width truncation is going to cause a warning, then there should be a way to make explicit truncations. Currently, I do not see a way to do this cleanly (please correct me if I'm wrong). Being able to explicitly set the width of a wire is especially important when working with the ## operator to avoid stray bits at the melding point.

The & operator comes somewhat close, but it will not reduce the width. The asTypeOf method I think might achieve the goal, but the documentation indicates that changing the width with this method is frowned upon.

To address this, there could be a method in class UInt (or a super class) that returns a copy of this UInt with a defined width (supporting both narrowing and widening conversions). Perhaps there can be a version of asUInt and related methods that take a Width argument.

[ekiwi]

To address this, there could be a method in class UInt (or a super class) that returns a copy of this UInt with a defined width (supporting both narrowing and widening conversions).

You can get a narrowing of a UInt using, e.g., myValue(5,0) to get the lower 6 bits. Alternatively there are also the head(..) and tail(..) methods which are defined on UInt. To get a widened UInt or SInt, you can use the pad(..) method.

[dbear496]

You can get a narrowing of a UInt using, e.g., myValue(5,0) to get the lower 6 bits. Alternatively there are also the head(..) and tail(..) methods which are defined on UInt. To get a widened UInt or SInt, you can use the pad(..) method.

None of these methods on their own work if I don't know in advance whether I need a widening or a narrowing conversion.

The myValue(5,0) method doesn't support widening conversions, so I'd have to compare the current width to the desired width before doing this, and that could get complicated by implicit widths.

Also, I find the myValue(5,0) method annoying since the upper bound is inclusive, so I have to constantly subtract 1 from the desired width. And who knows what to do here if I want a zero-width (if that is ever going to be supported).

The tail method is also not very helpful here because it only changes the width relative to the current width. And this could go wrong especially in the presence of implicit widths.

The head method takes most-significant bits, which is usually not what I need. And it could also go wrong in the presence of implicit widths.

The pad method will only do a widening conversion, whereas I usually need a narrowing conversion.

I suppose I could combine methods by doing myValue.pad(width)(width-1,0), and that would cover all the bases, but it's kinda ugly how I have to specify width twice, which can get messy if the width is an expression: myValue.pad(log2Ceil(config.property.length + 1))(log2Ceil(config.property.length + 1) - 1, 0). Why all that when all I want to do is set the width?

[mwachs5]

Connectable .squeeze is the current solution here. Use the newer operators :<>= and :#= to get the strict behavior. .squeeze if you want to relax it (on whichever side(s) you want to relax it on).