Generate better names for nodes (#190)

For Chisel nodes defined in Module class-level values of type Option or
Iterable, we can still use reflection to assign names based on the name
of the value. This works for arbitrary nesting of Option and Iterable so
long as the innermost type is HasId. Note that this excludes Maps which
always have an innermost type of Tuple2[_,_].

2 files changed, 122 insertions, 3 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
index cde7032d..ca91c5f8 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
@@ -103,10 +103,28 @@ extends HasId {
     val valNames = getValNames(this.getClass)
     def isPublicVal(m: java.lang.reflect.Method) =
       m.getParameterTypes.isEmpty && valNames.contains(m.getName)
+
+    /** Recursively suggests names to supported "container" classes
+      * Arbitrary nestings of supported classes are allowed so long as the
+      * innermost element is of type HasId
+      * Currently supported:
+      *   - Iterable
+      *   - Option
+      * (Note that Map is Iterable[Tuple2[_,_]] and thus excluded)
+      */
+    def nameRecursively(prefix: String, nameMe: Any): Unit =
+      nameMe match {
+        case (id: HasId) => id.suggestName(prefix)
+        case Some(elt) => nameRecursively(prefix, elt)
+        case (iter: Iterable[_]) if iter.hasDefiniteSize =>
+          for ((elt, i) <- iter.zipWithIndex) {
+            nameRecursively(s"${prefix}_${i}", elt)
+          }
+        case _ => // Do nothing
+      }
     val methods = getClass.getMethods.sortWith(_.getName > _.getName)
-    for (m <- methods; if isPublicVal(m)) m.invoke(this) match {
-      case (id: HasId) => id.suggestName(m.getName)
-      case _ =>
+    for (m <- methods if isPublicVal(m)) {
+      nameRecursively(m.getName, m.invoke(this))
     }
 
     // For Module instances we haven't named, suggest the name of the Module
diff --git a/src/test/scala/chiselTests/BetterNamingTests.scala b/src/test/scala/chiselTests/BetterNamingTests.scala
new file mode 100644
index 00000000..44fc542a
--- /dev/null
+++ b/src/test/scala/chiselTests/BetterNamingTests.scala
@@ -0,0 +1,101 @@
+package chiselTests
+
+import org.scalatest.{FlatSpec, Matchers}
+import collection.mutable
+
+import Chisel._
+
+
+// Defined outside of the class so we don't get $ in name
+class Other(w: Int) extends Module {
+  val io = new Bundle {
+    val a = UInt(width = w)
+  }
+}
+class PerNameIndexing(count: Int) extends Module {
+  val io = new Bundle { }
+
+  val wires = Seq.tabulate(count) { i => Module(new Other(i)) }
+  val queues = Seq.tabulate(count) { i => Module(new Queue(UInt(width = i), 16)) }
+}
+
+// Note this only checks Iterable[Chisel.Data] which excludes Maps
+class IterableNaming extends Module {
+  val io = new Bundle { }
+
+  val seq = Seq.tabulate(3) { i =>
+    Seq.tabulate(2) { j => Wire(init = (i * j).U) }
+  }
+  val optSet = Some(Set(Wire(init = 0.U),
+                        Wire(init = 1.U),
+                        Wire(init = 2.U),
+                        Wire(init = 3.U)))
+
+  val stack = mutable.Stack[Module]()
+  for (i <- 0 until 4) {
+    stack push Module(new Other(i))
+  }
+
+  def streamFrom(x: Int): Stream[Module] =
+    Module(new Other(x)) #:: streamFrom(x + 1)
+  val stream = streamFrom(0) // Check that we don't get into infinite loop
+  val list = stream.take(8).toList
+}
+
+/* Better Naming Tests
+ *
+ * These tests are intended to validate that Chisel picks better names
+ */
+class BetterNamingTests extends FlatSpec {
+
+  behavior of "Better Naming"
+
+  it should "provide unique counters for each name" in {
+    val verilog = Driver.emit(() => new PerNameIndexing(4))
+    val ModuleDef = """\s*module\s+(\S+)\s+:\s*""".r
+    val expectedModules = Set("PerNameIndexing",
+      "Queue", "Queue_1", "Queue_2", "Queue_3",
+      "Other", "Other_1", "Other_2", "Other_3")
+    val foundModules = for {
+      ModuleDef(name) <- verilog.split("\n").toSeq
+    } yield name
+    assert(foundModules.toSet === expectedModules)
+  }
+
+  it should "provide names for things defined in Iterable[HasId] and Option[HasId]" in {
+    val verilog = Driver.emit(() => new IterableNaming)
+
+    val lines = verilog.split("\n").toSeq
+
+    val SeqDef = """\s*wire\s+seq_(\d+)_(\d+)\s+:\s+UInt\s*""".r
+    val seqs = for {
+      i <- (0 until 3)
+      j <- (0 until 2)
+    } yield (i.toString, j.toString)
+    val foundSeqs = for {
+      SeqDef(i, j) <- lines
+    } yield (i, j)
+    assert(foundSeqs.toSet === seqs.toSet)
+
+    val OptSetDef = """\s*wire\s+optSet_(\d+)\s+:\s+UInt\s*""".r
+    val optSets = (0 until 4) map (_.toString)
+    val foundOptSets = for {
+      OptSetDef(i) <- lines
+    } yield i
+    assert(foundOptSets.toSet === optSets.toSet)
+
+    val StackDef = """\s*inst\s+stack_(\d+)\s+of\s+Other.*""".r
+    val stacks = (0 until 4) map (_.toString)
+    val foundStacks = for {
+      StackDef(i) <- lines
+    } yield i
+    assert(foundStacks.toSet === stacks.toSet)
+
+    val ListDef = """\s*inst\s+list_(\d+)\s+of\s+Other.*""".r
+    val lists = (0 until 8) map (_.toString)
+    val foundLists = for {
+      ListDef(i) <- lines
+    } yield i
+    assert(foundLists.toSet === lists.toSet)
+  }
+}