diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 3e0362e797d..d6a447083df 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -149,21 +149,6 @@ private[chisel3] trait HasId extends InstanceId { * @return the name, if it can be computed */ def computeName(defaultPrefix: Option[String], defaultSeed: Option[String]): Option[String] = { - // Recursively builds a name if referenced fields of an aggregate type - def buildAggName(id: HasId): Option[String] = { - def recArg(node: Arg): Option[String] = node match { - case Slot(imm, name) => recArg(imm).map(_ + "_" + name) - case Index(imm, ILit(num)) => recArg(imm).map(_ + "_" + num) - case Index(imm, n: LitArg) => recArg(imm).map(_ + "_" + n.num) - case Index(imm, _: Node) => recArg(imm) - case Node(id) => recArg(id.getOptionRef.get) - case Ref(name) => Some(name) - case ModuleIO(mod, name) if _parent.contains(mod) => Some(name) - case ModuleIO(mod, name) => recArg(mod.getRef).map(_ + "_" + name) - } - id.getOptionRef.flatMap(recArg) - } - /** Computes a name of this signal, given the seed and prefix * @param seed * @param prefix @@ -173,18 +158,13 @@ private[chisel3] trait HasId extends InstanceId { val builder = new StringBuilder() prefix.foreach { case Left(s: String) => builder ++= s + "_" - case Right(d: HasId) => - buildAggName(d) match { - case Some(n) => builder ++= n + "_" - case _ => - } - case _ => + case other => Builder.exception(s"Only Strings should exist in Prefixes, got $other") } builder ++= seed builder.toString } - if(hasSeed) { + if (hasSeed) { Some(buildName(seedOpt.get, prefix_seed)) } else { defaultSeed.map { default => @@ -379,19 +359,48 @@ private[chisel3] object Builder { def annotations: ArrayBuffer[ChiselAnnotation] = dynamicContext.annotations def namingStack: NamingStack = dynamicContext.namingStack - // Puts either a prefix string or hasId onto the prefix stack - def pushPrefix(d: Either[String, HasId]): Unit = { - chiselContext.get().prefixStack += d - } - // Puts a prefix string onto the prefix stack def pushPrefix(d: String): Unit = { chiselContext.get().prefixStack += Left(d) } - // Puts a prefix data onto the prefix stack - def pushPrefix(d: HasId): Unit = { - chiselContext.get().prefixStack += Right(d) + /** Pushes the current name of a data onto the prefix stack + * + * @param d data to derive prefix from + * @return whether anything was pushed to the stack + */ + def pushPrefix(d: HasId): Boolean = { + def buildAggName(id: HasId): Option[String] = { + // TODO This is slow, can we store this information upon binding? + def getSubName(field: Data, parent: Data): Option[String] = parent match { + case vec: Vec[_] => + val idx = vec.indexOf(field) + if (idx >= 0) Some(idx.toString) else None + case rec: Record => rec.elements.collectFirst { case (name, d) if d == field => name } + case _ => Builder.exception(s"Shouldn't see non-Aggregate $parent as parent in ChildBinding!") + } + def map2[A, B](a: Option[A], b: Option[A])(f: (A, A) => B): Option[B] = + a.flatMap(ax => b.map(f(ax, _))) + // If the binding is None, this is an illegal connection and later logic will error + def recData(data: Data): Option[String] = data.binding.flatMap { + case (_: WireBinding | _: RegBinding | _: MemoryPortBinding | _: OpBinding) => data.seedOpt + case ChildBinding(parent) => recData(parent).map { p => + // And name of the field if we have one, we don't for dynamic indexing of Vecs + getSubName(data, parent).map(p + "_" + _).getOrElse(p) + } + case SampleElementBinding(parent) => recData(parent) + case PortBinding(mod) if Builder.currentModule.contains(mod) => data.seedOpt + case PortBinding(mod) => map2(mod.seedOpt, data.seedOpt)(_ + "_" + _) + case (_: LitBinding | _: DontCareBinding) => None + } + id match { + case d: Data => recData(d) + case _ => None + } + } + buildAggName(d).map { name => + chiselContext.get().prefixStack += Left(name) + }.isDefined } // Remove a prefix from top of the stack @@ -598,7 +607,7 @@ private[chisel3] object Builder { * @param m exception message */ @throws(classOf[ChiselException]) - def exception(m: => String): Unit = { + def exception(m: => String): Nothing = { error(m) throwException(m) } diff --git a/core/src/main/scala/chisel3/internal/prefix.scala b/core/src/main/scala/chisel3/internal/prefix.scala index bbe3122616f..9dc1490164a 100644 --- a/core/src/main/scala/chisel3/internal/prefix.scala +++ b/core/src/main/scala/chisel3/internal/prefix.scala @@ -28,9 +28,11 @@ private[chisel3] object prefix { // scalastyle:ignore * @return The return value of the provided function */ def apply[T](name: HasId)(f: => T): T = { - Builder.pushPrefix(name) + val pushed = Builder.pushPrefix(name) val ret = f - Builder.popPrefix() + if (pushed) { + Builder.popPrefix() + } ret } diff --git a/src/test/scala/chiselTests/naming/PrefixSpec.scala b/src/test/scala/chiselTests/naming/PrefixSpec.scala index 888ce1ba851..27a9fd39750 100644 --- a/src/test/scala/chiselTests/naming/PrefixSpec.scala +++ b/src/test/scala/chiselTests/naming/PrefixSpec.scala @@ -69,8 +69,6 @@ class PrefixSpec extends ChiselPropSpec with Utils { property("Prefixing seeded with signal") { class Test extends MultiIOModule { - @treedump - @dump def builder(): UInt = { val wire = Wire(UInt(3.W)) wire := 3.U @@ -345,4 +343,59 @@ class PrefixSpec extends ChiselPropSpec with Utils { )) } } + + property("Connections should use the non-prefixed name of the connected Data") { + class Test extends MultiIOModule { + prefix("foo") { + val x = Wire(UInt(8.W)) + x := { + val w = Wire(UInt(8.W)) + w := 3.U + w + 1.U + } + } + } + aspectTest(() => new Test) { + top: Test => + Select.wires(top).map(_.instanceName) should be (List("foo_x", "foo_x_w")) + } + } + + property("Connections to aggregate fields should use the non-prefixed aggregate name") { + class Test extends MultiIOModule { + prefix("foo") { + val x = Wire(new Bundle { val bar = UInt(8.W) }) + x.bar := { + val w = Wire(new Bundle { val fizz = UInt(8.W) }) + w.fizz := 3.U + w.fizz + 1.U + } + } + } + aspectTest(() => new Test) { + top: Test => + Select.wires(top).map(_.instanceName) should be (List("foo_x", "foo_x_bar_w")) + } + } + + + property("Prefixing with wires in recursive functions should grow linearly") { + class Test extends MultiIOModule { + def func(bools: Seq[Bool]): Bool = { + if (bools.isEmpty) true.B + else { + val w = Wire(Bool()) + w := bools.head && func(bools.tail) + w + } + } + val in = IO(Input(Vec(4, Bool()))) + val x = func(in) + } + aspectTest(() => new Test) { + top: Test => + Select.wires(top).map(_.instanceName) should be (List("x", "x_w_w", "x_w_w_w", "x_w_w_w_w")) + } + + } }