Literals set the ref so that they no longer get named #826
Conversation
Fixes #763 This has a few implications * Constructing a literal no longer increments _T_ suffixes * Internally, wrapping a literal Bits in Node(...) will work * Literal Bools work in withReset/withClockAndReset * Data.ref is potentially redundant
There was a problem hiding this comment.
Also, are there any implications for usage with the bundle literals framework (#820)? That PR moves litArg into the binding and alters the behavior of the ref accessors somewhat. So this probably won't be directly compatible, and it might make more sense for ref to be set on a binding operation? It would also probably be safer and cleaner to have refs initialized with binding?
Test cases look sane though.
| @@ -142,6 +142,18 @@ class MultiClockSpec extends ChiselFlatSpec { | |||
| assert(withReset(this.reset) { 5 } == 5) | |||
| }) | |||
| } | |||
| it should "support literal Bools" in { | |||
There was a problem hiding this comment.
This test seems kind of random, I assume this fix is a side effect of the above change?
There was a problem hiding this comment.
#763 was the motivating issue so I added a test for it :) In looking into that issue, I realized that literal Bools not working in withReset was a side effect of the weirdness that this PR fixes
|
@ducky64 Yeah I think this will need to be done slightly differently in #820. Since the current LitBinding doesn't have any information about the literal value, I think this implementation is the right one in the meantime. I agree though that with Bundle literals it makes more sense to set the ref as part of the Binding. |
Have literals set their ref so that a name isn't allocated
This has a few implications
_T_suffixesRelated issue: Fixes #763.
@azidar beat me to fixing #472, but this would also have fixed that issue (I added a test in this PR as well)
Type of change: other enhancement
Impact: no functional change
Development Phase: implementation
Release Notes
Literal Bools work for reset in withClockAndReset and withReset
Literals no longer increment temporary names suffix