Support literals cast to aggregates as async reset reg init values #1225
Conversation
Accomplished by changing the code gen for casting literals to aggregates. Rather than connecting the literal to a wire that is then bit selected from, just bit select from the literal which saves the creation of an intermediate wire and matches FIRRTL's semantics for legal async reset initial values.
There was a problem hiding this comment.
This seems fine as a quickfix for this problem.
But I think there is a larger underlying discussion about how much the frontend should special-case literals, and where functionality is duplicated with FIRRTL, weighing the trade-offs of non-DRY code. The duplication is trivial in this case, so bugs are less likely, but this might not be the last time we need a feature like this...
|
@jackkoenig regarding your comment on chipsalliance/firrtl#1201 I guess you do not expect to fix the issues raised in #1208 test-cases now but rather wait for an in-depth work about proper constant propagation ? val x = WireInit(123.U + 456.U)
withReset(reset.asAsyncReset)(RegInit(x))seen as a valid reset value ? |
|
@johnsbrew, you are correct that this is not intended to address test cases like |
|
Failing CircleCI will be fixed by #1227 |
|
@Mergifyio backport 3.2.x |
…1225) Accomplished by changing the code gen for casting literals to aggregates. Rather than connecting the literal to a wire that is then bit selected from, just bit select from the literal which saves the creation of an intermediate wire and matches FIRRTL's semantics for legal async reset initial values. (cherry picked from commit 92d88ff)
|
Command
Hey, we reacted but our real name is @Mergifyio |
…1225) (#1247) Accomplished by changing the code gen for casting literals to aggregates. Rather than connecting the literal to a wire that is then bit selected from, just bit select from the literal which saves the creation of an intermediate wire and matches FIRRTL's semantics for legal async reset initial values. (cherry picked from commit 92d88ff)
Accomplished by changing the code gen for casting literals to
aggregates. Rather than connecting the literal to a wire that is then
bit selected from, just bit select from the literal which saves the
creation of an intermediate wire and matches FIRRTL's semantics for
legal async reset initial values.
Turned out to be a very clean implementation, most work (as usual) in the testing.
Related issue: chipsalliance/firrtl#1201
Type of change: bug fix
Impact: API addition? (only in that it fixes a bug)
Development Phase: implementation
Release Notes
Fix bug where literals cast to aggregates could not be used as async reset register initial values.