Fix lazy cloning and apply the optimization in more cases

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

@@ -1218,6 +1218,10 @@ abstract class Record(private[chisel3] implicit val compileOptions: CompileOptio
       s"Internal Error! This should have been implemented by the chisel3-plugin. Please file an issue against chisel3"
     )
   }
+
+  override private[chisel3] lazy val _minId: Long = {
+    this.elementsIterator.map(_._minId).foldLeft(this._id)(_ min _)
+  }
 }
 
 /**
@@ -1405,8 +1409,6 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record wi
     }
   }
 
-  private[chisel3] lazy val hasExternalRef: Boolean = this.elements.exists(_._2._id < _id)
-
   override def cloneType: this.type = {
     val clone = _cloneTypeImpl.asInstanceOf[this.type]
     checkClone(clone)

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

@@ -367,19 +367,20 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
     }
   }
 
-  // must clone a data if
-  // * it has a binding
-  // * its id is older than prevId (not "freshly created")
-  // * it is a bundle with a non-fresh member (external reference)
+  // Must clone a Data if any of the following are true:
+  // * It has a binding
+  // * Its id is older than prevId (not "freshly created")
+  // * It is a Bundle or Record that contains a member older than prevId
   private[chisel3] def mustClone(prevId: Long): Boolean = {
-    if (this.hasBinding || this._id <= prevId) true
-    else
-      this match {
-        case b: Bundle => b.hasExternalRef
-        case _ => false
-      }
+    this.hasBinding || this._minId <= prevId
   }
 
+  /** The minimum (aka "oldest") id that is part of this Data
+    *
+    * @note This is usually just _id except for some Records and Bundles
+    */
+  private[chisel3] def _minId: Long = this._id
+
   override def autoSeed(name: String): this.type = {
     topBindingOpt match {
       // Ports are special in that the autoSeed will keep the first name, not the last name

src/test/scala/chiselTests/AutoClonetypeSpec.scala

@@ -256,7 +256,10 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
 
       elaborate {
         new Module {
-          val io = IO(Output(new BadBundle(UInt(8.W), 1)))
+          // This needs to be constructed before the call to Output, otherwise it won't be cloned
+          // thanks to lazy cloning
+          val gen = new BadBundle(UInt(8.W), 1)
+          val io = IO(Output(gen))
           io.a := 0.U
         }
       }

src/test/scala/chiselTests/LazyCloneSpec.scala

@@ -3,8 +3,10 @@
 package chiselTests
 
 import chisel3._
+import chisel3.experimental.AutoCloneType
 import chisel3.stage.ChiselStage
-import chiselTests.ChiselFlatSpec
+
+import collection.immutable.VectorMap
 
 class LazyCloneSpec extends ChiselFlatSpec {
   object Counter {
@@ -21,6 +23,18 @@ class LazyCloneSpec extends ChiselFlatSpec {
     Counter.count += 1
   }
 
+  class GenRecord(gen: UInt) extends Record with AutoCloneType {
+    lazy val elements = VectorMap("foo" -> gen)
+    Counter.count += 1
+  }
+
+  class NestedGenBundle(gen: UInt) extends Bundle {
+    val foo = new Bundle {
+      val bar = gen
+    }
+    Counter.count += 1
+  }
+
   behavior.of("LazyClone")
 
   it should "not clone" in {
@@ -48,7 +62,7 @@ class LazyCloneSpec extends ChiselFlatSpec {
     Counter.count should be(3L)
   }
 
-  it should "clone because of external ref" in {
+  it should "not clone when ref is external to the Bundle but not the binding operation" in {
     Counter.count = 0L
     class MyModule extends RawModule {
       val io = IO(Flipped(new Bundle {
@@ -57,6 +71,77 @@ class LazyCloneSpec extends ChiselFlatSpec {
       }))
     }
     ChiselStage.elaborate(new MyModule)
+    Counter.count should be(2L)
+  }
+
+  it should "clone Records because of external refs" in {
+    Counter.count = 0L
+    class MyModule extends RawModule {
+      val gen = UInt(8.W)
+      val in = IO(Input(new GenRecord(gen)))
+      val out = IO(Output(new GenRecord(gen)))
+      out := in
+    }
+    ChiselStage.elaborate(new MyModule)
+    Counter.count should be(4L)
+  }
+
+  it should "not clone when ref is external to the Record but not the binding operation" in {
+    Counter.count = 0L
+    class MyModule extends RawModule {
+      val in = IO(Input(new GenRecord(UInt(8.W))))
+      val out = IO(Output(new GenRecord(UInt(8.W))))
+      out := in
+    }
+    ChiselStage.elaborate(new MyModule)
+    Counter.count should be(2L)
+  }
+
+  it should "clone because of nested external ref" in {
+    Counter.count = 0L
+    class MyModule extends RawModule {
+      val gen = UInt(8.W)
+      val in = IO(Input(new NestedGenBundle(gen)))
+      val out = IO(Output(new NestedGenBundle(gen)))
+      out := in
+    }
+    ChiselStage.elaborate(new MyModule)
     Counter.count should be(4L)
   }
+
+  it should "not clone when nested ref is external to the Bundle but not the binding operation" in {
+    Counter.count = 0L
+    class MyModule extends RawModule {
+      val in = IO(Input(new NestedGenBundle(UInt(8.W))))
+      val out = IO(Output(new NestedGenBundle(UInt(8.W))))
+      out := in
+    }
+    ChiselStage.elaborate(new MyModule)
+    Counter.count should be(2L)
+  }
+
+  it should "not clone Vecs (but Vecs always clone their gen 1 + size times)" in {
+    Counter.count = 0L
+    class MyModule extends RawModule {
+      val in = IO(Input(Vec(2, new Foo)))
+      val out = IO(Output(Vec(2, new Foo)))
+      out := in
+    }
+    ChiselStage.elaborate(new MyModule)
+    // Each Vec has 3 clones of Foo + the original Foo, then the Vec isn't cloned
+    Counter.count should be(8L)
+  }
+
+  it should "not clone Vecs even with external refs (but Vecs always clone their gen 1 + size times)" in {
+    Counter.count = 0L
+    class MyModule extends RawModule {
+      val gen = new Foo
+      val in = IO(Input(Vec(2, gen)))
+      val out = IO(Output(Vec(2, gen)))
+      out := in
+    }
+    ChiselStage.elaborate(new MyModule)
+    // Each Vec has 3 clones of Foo + the original Foo, then the Vec isn't cloned
+    Counter.count should be(7L)
+  }
 }