Optimize _ids (#2866)

* Don't copy _ids to a Seq just to Iterate on them

Saves a copy of a large data structure.

* Only add Data to _ids when the binding is constrained

ConstrainedBinding means that the bound Data is scoped to a module.
Since _ids are used for naming of signals within a module, there is no
reason to add Unconstrained Data to the _ids of the Module they were
constructed during. Unconstrained Data include things like literals and
children of Aggregates (which sort of are constrained to a Module but
only via their parent, they are not directly named by the module).

core/src/main/scala/chisel3/Aggregate.scala

@@ -184,7 +184,7 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int) extend
   }
 
   private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
-    _parent.foreach(_.addId(this))
+    this.maybeAddToParentIds(target)
     binding = target
 
     val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
@@ -987,7 +987,7 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
   }
 
   private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
-    _parent.foreach(_.addId(this))
+    this.maybeAddToParentIds(target)
     binding = target
 
     val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)

core/src/main/scala/chisel3/Data.scala

@@ -450,6 +450,14 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
     */
   private[chisel3] def bind(target: Binding, parentDirection: SpecifiedDirection = SpecifiedDirection.Unspecified): Unit
 
+  /** Adds this `Data` to its parents _ids if it should be added */
+  private[chisel3] def maybeAddToParentIds(target: Binding): Unit = {
+    // ConstrainedBinding means the thing actually corresponds to a Module, no need to add to _ids otherwise
+    if (target.isInstanceOf[ConstrainedBinding]) {
+      _parent.foreach(_.addId(this))
+    }
+  }
+
   // Both _direction and _resolvedUserDirection are saved versions of computed variables (for
   // efficiency, avoid expensive recomputation of frequent operations).
   // Both are only valid after binding is set.

core/src/main/scala/chisel3/Element.scala

@@ -18,7 +18,7 @@ abstract class Element extends Data {
   def name:                               String = getRef.name
 
   private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
-    _parent.foreach(_.addId(this))
+    this.maybeAddToParentIds(target)
     binding = target
     val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
     direction = ActualDirection.fromSpecified(resolvedDirection)

core/src/main/scala/chisel3/Module.scala

@@ -326,14 +326,15 @@ package experimental {
     // Returns the last id contained within a Module
     private[chisel3] def _lastId: Long = _ids.last match {
       case mod: BaseModule => mod._lastId
-      case _ =>
-        // Ideally we could just take last._id, but Records store and thus bind their Data in reverse order
-        _ids.maxBy(_._id)._id
+      case agg: Aggregate  =>
+        // Ideally we could just take .last._id, but Records store their elements in reverse order
+        getRecursiveFields.lazily(agg, "").map(_._1._id).max
+      case other => other._id
     }
 
-    private[chisel3] def getIds = {
+    private[chisel3] def getIds: Iterable[HasId] = {
       require(_closed, "Can't get ids before module close")
-      _ids.toSeq
+      _ids
     }
 
     private val _ports = new ArrayBuffer[(Data, SourceInfo)]()

core/src/main/scala/chisel3/experimental/Analog.scala

@@ -55,7 +55,7 @@ final class Analog private (private[chisel3] val width: Width) extends Element {
   // Define setter/getter pairing
   // Analog can only be bound to Ports and Wires (and Unbound)
   private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
-    _parent.foreach(_.addId(this))
+    this.maybeAddToParentIds(target)
     SpecifiedDirection.fromParent(parentDirection, specifiedDirection) match {
       case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip =>
       case x                                                        => throwException(s"Analog may not have explicit direction, got '$x'")

core/src/main/scala/chisel3/experimental/dataview/DataProduct.scala

@@ -68,7 +68,7 @@ object DataProduct extends LowPriorityDataProduct {
     def dataIterator(a: BaseModule, path: String): Iterator[(Data, String)] = {
       a.getIds.iterator.flatMap {
         case d: Data if d.getOptionRef.isDefined => // Using ref to decide if it's truly hardware in the module
-          Seq(d -> s"${path}.${d.instanceName}")
+          implicitly[DataProduct[Data]].dataIterator(d, s"${path}.${d.instanceName}")
         case b: BaseModule => dataIterator(b, s"$path.${b.instanceName}")
         case _ => Seq.empty
       }

src/test/scala/chiselTests/experimental/ModuleDataProductSpec.scala

@@ -10,7 +10,7 @@ import chiselTests.ChiselFlatSpec
 object ModuleDataProductSpec {
   class MyBundle extends Bundle {
     val foo = UInt(8.W)
-    val bar = UInt(8.W)
+    val bar = Vec(1, UInt(8.W))
   }
   trait MyIntf extends BaseModule {
     val in = IO(Input(new MyBundle))
@@ -49,12 +49,15 @@ class ModuleDataProductSpec extends ChiselFlatSpec {
       m.in -> "m.in",
       m.in.foo -> "m.in.foo",
       m.in.bar -> "m.in.bar",
+      m.in.bar(0) -> "m.in.bar(0)",
       m.out -> "m.out",
       m.out.foo -> "m.out.foo",
       m.out.bar -> "m.out.bar",
+      m.out.bar(0) -> "m.out.bar(0)",
       m.r -> "m.r",
       m.r.foo -> "m.r.foo",
       m.r.bar -> "m.r.bar",
+      m.r.bar(0) -> "m.r.bar(0)",
       m.inst.in -> "m.inst.in",
       m.inst.out -> "m.inst.out"
     )
@@ -72,11 +75,13 @@ class ModuleDataProductSpec extends ChiselFlatSpec {
     val m = elaborateAndGetModule(new MyExtModuleWrapper).inst
     val expected = Seq(
       m.in -> "m.in",
-      m.in.foo -> "m.in.foo",
       m.in.bar -> "m.in.bar",
+      m.in.bar(0) -> "m.in.bar(0)",
+      m.in.foo -> "m.in.foo",
       m.out -> "m.out",
-      m.out.foo -> "m.out.foo",
-      m.out.bar -> "m.out.bar"
+      m.out.bar -> "m.out.bar",
+      m.out.bar(0) -> "m.out.bar(0)",
+      m.out.foo -> "m.out.foo"
     )
 
     val impl = implicitly[DataProduct[MyExtModule]]