Cleanup Named Targets (#1311)

* Add CleanupNamedTargets Transform

Adds a new transform, CleanupNamedTargets, that will convert
ReferenceTargets to InstanceTargets if any ReferenceTargets are found
to be actually pointing at instances.

This fixes an issue where a ComponentName pointing at an instance will
be converted to a ReferenceTarget. Later on, this ReferenceTarget will
not be properly renamed by other transforms, e.g.,
EliminateTargetPaths.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

* Update ScalaTests

Co-authored-by: Jack Koenig <koenig@sifive.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

2 files changed, 85 insertions, 1 deletions

diff --git a/src/test/scala/firrtlTests/LoweringCompilersSpec.scala b/src/test/scala/firrtlTests/LoweringCompilersSpec.scala
index 802596c5..e1de7e34 100644
--- a/src/test/scala/firrtlTests/LoweringCompilersSpec.scala
+++ b/src/test/scala/firrtlTests/LoweringCompilersSpec.scala
@@ -132,7 +132,10 @@ class LoweringCompilersSpec extends AnyFlatSpec with Matchers {
 
   it should "replicate the old order" in {
     val tm = new TransformManager(Forms.MinimalHighForm, Forms.ChirrtlForm)
-    compare(legacyTransforms(new firrtl.ChirrtlToHighFirrtl), tm)
+    val patches = Seq(
+      Add(5, Seq(Dependency[firrtl.annotations.transforms.CleanupNamedTargets]))
+    )
+    compare(legacyTransforms(new firrtl.ChirrtlToHighFirrtl), tm, patches)
   }
 
   behavior of "IRToWorkingIR"
diff --git a/src/test/scala/firrtlTests/annotationTests/CleanupNamedTargetsSpec.scala b/src/test/scala/firrtlTests/annotationTests/CleanupNamedTargetsSpec.scala
new file mode 100644
index 00000000..58cb3d11
--- /dev/null
+++ b/src/test/scala/firrtlTests/annotationTests/CleanupNamedTargetsSpec.scala
@@ -0,0 +1,81 @@
+// See LICENSE for license details.
+
+package firrtlTests.annotationTests
+
+import firrtl.{AnnotationSeq, CircuitState, Parser, UnknownForm}
+import firrtl.annotations.{
+  CircuitName,
+  CircuitTarget,
+  ComponentName,
+  ModuleName,
+  MultiTargetAnnotation,
+  ReferenceTarget,
+  SingleTargetAnnotation,
+  Target}
+import firrtl.annotations.transforms.CleanupNamedTargets
+
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+object CleanupNamedTargetsSpec {
+
+  case class SingleReferenceAnnotation(target: ReferenceTarget) extends SingleTargetAnnotation[ReferenceTarget] {
+    override def duplicate(a: ReferenceTarget) = this.copy(target = a)
+  }
+
+  case class MultiReferenceAnnotation(targets: Seq[Seq[Target]]) extends MultiTargetAnnotation {
+    override def duplicate(a: Seq[Seq[Target]]) = this.copy(targets = a)
+  }
+
+}
+
+class CleanupNamedTargetsSpec extends AnyFlatSpec with Matchers {
+
+  import CleanupNamedTargetsSpec._
+
+  class F {
+
+    val transform = new CleanupNamedTargets
+
+    val circuit =
+      """|circuit Foo:
+         |  module Bar:
+         |    skip
+         |  module Foo:
+         |    inst bar of Bar
+         |    wire baz: UInt<1>
+         |""".stripMargin
+
+    lazy val foo = CircuitTarget("Foo").module("Foo")
+
+    lazy val barTarget = ComponentName("bar", ModuleName("Foo", CircuitName("Foo"))).toTarget
+
+    lazy val bazTarget = ComponentName("baz", ModuleName("Foo", CircuitName("Foo"))).toTarget
+
+    def circuitState(a: AnnotationSeq): CircuitState = CircuitState(Parser.parse(circuit), UnknownForm, a, None)
+
+  }
+
+  behavior of "CleanupNamedTargets"
+
+  it should "convert a SingleTargetAnnotation[ReferenceTarget] of an instance to an InstanceTarget" in new F {
+    val annotations: AnnotationSeq = Seq(SingleReferenceAnnotation(barTarget))
+
+    transform.transform(circuitState(annotations)).renames.get.get(barTarget) should be {
+      Some(Seq(foo.instOf("bar", "Bar")))
+    }
+  }
+
+  it should "convert a MultiTargetAnnotation with a ReferenceTarget of an instance to an InstanceTarget" in new F {
+    val annotations: AnnotationSeq = Seq(MultiReferenceAnnotation(Seq(Seq(barTarget), Seq(bazTarget))))
+
+    val renames = transform.transform(circuitState(annotations)).renames.get
+
+    renames.get(barTarget) should be (Some(Seq(foo.instOf("bar", "Bar"))))
+
+    info("and not touch a true ReferenceAnnotation")
+    renames.get(bazTarget) should be (None)
+
+  }
+
+}