-
Notifications
You must be signed in to change notification settings - Fork 176
Make submodule inputs void in ExpandWhens #706
Conversation
921db1e
to
4350f2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the one comment/question this looks good to me.
case w: DefWire => | ||
netlist ++= (getFemaleRefs(w.name, w.tpe, BIGENDER) map (ref => we(ref) -> WVoid)) | ||
w | ||
case w: DefMemory => | ||
netlist ++= (getFemaleRefs(w.name, MemPortUtils.memType(w), MALE) map (ref => we(ref) -> WVoid)) | ||
w | ||
case w: WDefInstance => | ||
netlist ++= (getFemaleRefs(w.name, w.tpe, MALE).map(ref => we(ref) -> WVoid)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MALE
of getFemaleRefs
here means to get all the inputs to the submodule?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified offline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, similar to how it's used on the DefMemory
above.
Due to chipsalliance/chisel#746 this currently doesn't work with rocket-chip, I am trying to get this fixed tonight |
This is a modification of #706 to preserve the bugfix but preserve the [buggy] API that firrtl was not properly enforcing initialization requirements for submodule inputs
This is a modified form of #706 to preserve the bugfix but preserve the [buggy] 1.0.x API that firrtl was not properly enforcing initialization requirements for submodule inputs
This is a modified form of #706 to preserve the bugfix but preserve the [buggy] 1.0.x API that firrtl was not properly enforcing initialization requirements for submodule inputs. If a submodule port is not properly invalidated, ExpandWhens will print a warning.
…as 712) This is a modified form of #706 to preserve the bugfix but preserve the [buggy] 1.0.x API that firrtl was not properly enforcing initialization requirements for submodule inputs The problem with #706 is that because Firrtl wasn't properly enforcing initialization requirements on submodules, code that used to work no longer does. This alternative version still fixes the incorrect Verilog generation bug but does not enforce the initialization requirement.
Fixes #705
The ports of submodules were not voided in ExpandWhens which means it does not error if they are not properly initialized. Worse still, since they don't have a default value in the netlist, the logic in ExpandWhens just picks the first connection and ignores all the rest.
I've added a test for each of the above cases.