Skip to content

Commit

Permalink
For HasId.setRef, have first set win (with force override) (#1655)
Browse files Browse the repository at this point in the history
This is a refinement of the assertion added in #1616 then removed in
 #1654. Because Records now set the refs of children upon binding,
later, unbound Records could incorrectly override the refs. The first
set should win.
  • Loading branch information
jackkoenig authored Nov 5, 2020
1 parent 679dd54 commit 8187318
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
4 changes: 2 additions & 2 deletions core/src/main/scala/chisel3/BlackBox.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,8 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param
// Long term solution will be to define BlackBox IO differently as part of
// it not descending from the (current) Module
for ((name, port) <- namedPorts) {
// Override the _ref because it was set during IO binding
port.setRef(ModuleIO(this, _namespace.name(name)))
// We have to force override the _ref because it was set during IO binding
port.setRef(ModuleIO(this, _namespace.name(name)), force = true)
}

// We need to call forceName and onModuleClose on all of the sub-elements
Expand Down
7 changes: 5 additions & 2 deletions core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,11 @@ private[chisel3] trait HasId extends InstanceId {
}

private var _ref: Option[Arg] = None
private[chisel3] def setRef(imm: Arg): Unit = {
_ref = Some(imm)
private[chisel3] def setRef(imm: Arg): Unit = setRef(imm, false)
private[chisel3] def setRef(imm: Arg, force: Boolean): Unit = {
if (_ref.isEmpty || force) {
_ref = Some(imm)
}
}
private[chisel3] def setRef(parent: HasId, name: String): Unit = setRef(Slot(Node(parent), name))
private[chisel3] def setRef(parent: HasId, index: Int): Unit = setRef(Index(Node(parent), ILit(index)))
Expand Down
13 changes: 13 additions & 0 deletions src/test/scala/chiselTests/BundleSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,17 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils {
}
}
}

"Bound Data" should "have priority in setting ref over unbound Data" in {
class MyModule extends RawModule {
val foo = IO(new Bundle {
val x = Output(UInt(8.W))
})
foo.x := 0.U // getRef on foo.x is None.get without fix
val bar = new Bundle {
val y = foo.x
}
}
ChiselStage.emitChirrtl(new MyModule)
}
}

0 comments on commit 8187318

Please sign in to comment.