chipsalliance / firrtl

Flexible Intermediate Representation for RTL
https://www.chisel-lang.org/firrtl/
Apache License 2.0
719 stars 176 forks source link

AsyncReset literals check is NOT compatible with chisel's complex literals emission #1044

Open johnsbrew opened 5 years ago

johnsbrew commented 5 years ago

Type of issue: bug report

Steps to reproduce the bug: Compile the following fragment ./firrtl -i bug_asyncreset.fir -tn Test

What is the use case for changing the behavior? Ensuring chisel's firrtl emission compatibility, see the simple example given val valid = RegInit(VecInit(Seq.fill(8)(false.B))) Impact: no functional change

Development Phase: request

Other information

edwardcwang commented 5 years ago

Part of the issue could be that VecInits aren't real literals in Chisel - they are wires that get assigned to, as opposed to "real" literals. See https://github.com/freechipsproject/chisel3/issues/849

johnsbrew commented 5 years ago

@edwardcwang I understand your point about Vec but it not really the issue here since some other chisel constructs could potentially lead to the same behaviour. To fix this issue in a generic way I propose a recursive lookup, see #1045

jackkoenig commented 5 years ago

@johnsbrew thanks for the report and proposed fix. I was hoping to avoid to having to do such a recursive check, but in the absence of support for aggregate literals, I think such an approach is necessary.

edwardcwang commented 5 years ago

What's the status of bundle/aggregate literals in FIRRTL? That could be another way (long term maybe) to avoid this recursive check.