Instance Annotations (#926)

Formerly #865 

Major Code Changes/Features Added:

Added Target trait as replacement for Named
Added TargetToken as token in building Target
Added GenericTarget as a catch-all Target
Added CircuitTarget, ModuleTarget, ReferenceTarget, and InstanceTarget
Added ResolvePaths annotation
Added EliminateTargetPaths (and helper class DuplicationHelper)
Updated Dedup to work with instance annotations
Updated RenameMap to work with instance annotations
DCE & ConstantProp extend ResolveAnnotationPaths

7 files changed, 928 insertions, 68 deletions

diff --git a/src/test/scala/firrtlTests/AnnotationTests.scala b/src/test/scala/firrtlTests/AnnotationTests.scala
index 10414786..6b492148 100644
--- a/src/test/scala/firrtlTests/AnnotationTests.scala
+++ b/src/test/scala/firrtlTests/AnnotationTests.scala
@@ -68,7 +68,6 @@ abstract class AnnotationTests extends AnnotationSpec with Matchers {
     val tname = transform.name
     val inlineAnn = InlineAnnotation(CircuitName("Top"))
     val result = compiler.compile(CircuitState(parse(input), ChirrtlForm, Seq(inlineAnn)), Seq(transform))
-    println(result.annotations.head)
     result.annotations.head should matchPattern {
       case DeletedAnnotation(`tname`, `inlineAnn`) =>
     }
@@ -192,7 +191,7 @@ abstract class AnnotationTests extends AnnotationSpec with Matchers {
       anno("w.a"), anno("w.b[0]"), anno("w.b[1]"),
       anno("r.a"), anno("r.b[0]"), anno("r.b[1]"),
       anno("write.a"), anno("write.b[0]"), anno("write.b[1]"),
-      dontTouch("Top.r"), dontTouch("Top.w")
+      dontTouch("Top.r"), dontTouch("Top.w"), dontTouch("Top.mem")
     )
     val result = compiler.compile(CircuitState(parse(input), ChirrtlForm, annos), Nil)
     val resultAnno = result.annotations.toSeq
diff --git a/src/test/scala/firrtlTests/ClockListTests.scala b/src/test/scala/firrtlTests/ClockListTests.scala
index 20718a71..48d6dfd3 100644
--- a/src/test/scala/firrtlTests/ClockListTests.scala
+++ b/src/test/scala/firrtlTests/ClockListTests.scala
@@ -1,13 +1,11 @@
+// See LICENSE for license details.
+
 package firrtlTests
 
 import java.io._
-import org.scalatest._
-import org.scalatest.prop._
 import firrtl._
 import firrtl.ir.Circuit
 import firrtl.passes._
-import firrtl.Parser.IgnoreInfo
-import annotations._
 import clocklist._
 
 class ClockListTests extends FirrtlFlatSpec {
diff --git a/src/test/scala/firrtlTests/InlineInstancesTests.scala b/src/test/scala/firrtlTests/InlineInstancesTests.scala
index 6d386d48..4affd64d 100644
--- a/src/test/scala/firrtlTests/InlineInstancesTests.scala
+++ b/src/test/scala/firrtlTests/InlineInstancesTests.scala
@@ -346,6 +346,43 @@ class InlineInstancesTests extends LowTransformSpec {
            |    b <= a""".stripMargin
       failingexecute(input, Seq(inline("A")))
    }
+
+  "Jack's Bug" should "not fail" in {
+
+    val input = """circuit Top :
+                  |  module Top :
+                  |    input a : UInt<32>
+                  |    output b : UInt<32>
+                  |    inst i of Inline
+                  |    i.a <= a
+                  |    b <= i.b
+                  |  module Inline :
+                  |    input a : UInt<32>
+                  |    output b : UInt<32>
+                  |    inst child of InlineChild
+                  |    child.a <= a
+                  |    b <= child.b
+                  |  module InlineChild :
+                  |    input a : UInt<32>
+                  |    output b : UInt<32>
+                  |    b <= a""".stripMargin
+    val check = """circuit Top :
+                  |  module Top :
+                  |    input a : UInt<32>
+                  |    output b : UInt<32>
+                  |    wire i_a : UInt<32>
+                  |    wire i_b : UInt<32>
+                  |    inst i_child of InlineChild
+                  |    i_b <= i_child.b
+                  |    i_child.a <= i_a
+                  |    b <= i_b
+                  |    i_a <= a
+                  |  module InlineChild :
+                  |    input a : UInt<32>
+                  |    output b : UInt<32>
+                  |    b <= a""".stripMargin
+    execute(input, check, Seq(inline("Inline")))
+  }
 }
 
 // Execution driven tests for inlining modules
diff --git a/src/test/scala/firrtlTests/RenameMapSpec.scala b/src/test/scala/firrtlTests/RenameMapSpec.scala
index 9e305b70..b063599b 100644
--- a/src/test/scala/firrtlTests/RenameMapSpec.scala
+++ b/src/test/scala/firrtlTests/RenameMapSpec.scala
@@ -4,26 +4,27 @@ package firrtlTests
 
 import firrtl.RenameMap
 import firrtl.FIRRTLException
-import firrtl.annotations.{
-  Named,
-  CircuitName,
-  ModuleName,
-  ComponentName
-}
+import firrtl.RenameMap.{CircularRenameException, IllegalRenameException}
+import firrtl.annotations._
 
 class RenameMapSpec extends FirrtlFlatSpec {
-  val cir = CircuitName("Top")
-  val cir2 = CircuitName("Pot")
-  val cir3 = CircuitName("Cir3")
-  val modA = ModuleName("A", cir)
-  val modA2 = ModuleName("A", cir2)
-  val modB = ModuleName("B", cir)
-  val foo = ComponentName("foo", modA)
-  val foo2 = ComponentName("foo", modA2)
-  val bar = ComponentName("bar", modA)
-  val fizz = ComponentName("fizz", modA)
-  val fooB = ComponentName("foo", modB)
-  val barB = ComponentName("bar", modB)
+  val cir   = CircuitTarget("Top")
+  val cir2  = CircuitTarget("Pot")
+  val cir3  = CircuitTarget("Cir3")
+  val modA  = cir.module("A")
+  val modA2 = cir2.module("A")
+  val modB = cir.module("B")
+  val foo = modA.ref("foo")
+  val foo2 = modA2.ref("foo")
+  val bar = modA.ref("bar")
+  val fizz = modA.ref("fizz")
+  val fooB = modB.ref("foo")
+  val barB = modB.ref("bar")
+
+  val tmb = cir.module("Top").instOf("mid", "Middle").instOf("bot", "Bottom")
+  val tm2b = cir.module("Top").instOf("mid", "Middle2").instOf("bot", "Bottom")
+  val middle = cir.module("Middle")
+  val middle2 = cir.module("Middle2")
 
   behavior of "RenameMap"
 
@@ -35,82 +36,268 @@ class RenameMapSpec extends FirrtlFlatSpec {
 
   it should "return a Seq of renamed things if it does rename something" in {
     val renames = RenameMap()
-    renames.rename(foo, bar)
+    renames.record(foo, bar)
     renames.get(foo) should be (Some(Seq(bar)))
   }
 
   it should "allow something to be renamed to multiple things" in {
     val renames = RenameMap()
-    renames.rename(foo, bar)
-    renames.rename(foo, fizz)
+    renames.record(foo, bar)
+    renames.record(foo, fizz)
     renames.get(foo) should be (Some(Seq(bar, fizz)))
   }
 
   it should "allow something to be renamed to nothing (ie. deleted)" in {
     val renames = RenameMap()
-    renames.rename(foo, Seq())
+    renames.record(foo, Seq())
     renames.get(foo) should be (Some(Seq()))
   }
 
   it should "return None if something is renamed to itself" in {
     val renames = RenameMap()
-    renames.rename(foo, foo)
+    renames.record(foo, foo)
     renames.get(foo) should be (None)
   }
 
-  it should "allow components to change module" in {
+  it should "allow targets to change module" in {
     val renames = RenameMap()
-    renames.rename(foo, fooB)
+    renames.record(foo, fooB)
     renames.get(foo) should be (Some(Seq(fooB)))
   }
 
-  it should "rename components if their module is renamed" in {
+  it should "rename targets if their module is renamed" in {
     val renames = RenameMap()
-    renames.rename(modA, modB)
+    renames.record(modA, modB)
     renames.get(foo) should be (Some(Seq(fooB)))
     renames.get(bar) should be (Some(Seq(barB)))
   }
 
-  it should "rename renamed components if the module of the target component is renamed" in {
+  it should "rename renamed targets if the module of the target is renamed" in {
     val renames = RenameMap()
-    renames.rename(modA, modB)
-    renames.rename(foo, bar)
+    renames.record(modA, modB)
+    renames.record(foo, bar)
     renames.get(foo) should be (Some(Seq(barB)))
   }
 
   it should "rename modules if their circuit is renamed" in {
     val renames = RenameMap()
-    renames.rename(cir, cir2)
+    renames.record(cir, cir2)
     renames.get(modA) should be (Some(Seq(modA2)))
   }
 
-  it should "rename components if their circuit is renamed" in {
+  it should "rename targets if their circuit is renamed" in {
     val renames = RenameMap()
-    renames.rename(cir, cir2)
+    renames.record(cir, cir2)
     renames.get(foo) should be (Some(Seq(foo2)))
   }
 
-  // Renaming `from` to each of the `tos` at the same time should error
-  case class BadRename(from: Named, tos: Seq[Named])
-  val badRenames =
-    Seq(BadRename(foo, Seq(cir)),
-        BadRename(foo, Seq(modA)),
-        BadRename(modA, Seq(foo)),
-        BadRename(modA, Seq(cir)),
-        BadRename(cir, Seq(foo)),
-        BadRename(cir, Seq(modA)),
-        BadRename(cir, Seq(cir2, cir3))
-       )
-  // Run all BadRename tests
-  for (BadRename(from, tos) <- badRenames) {
-    val fromN = from.getClass.getSimpleName
-    val tosN = tos.map(_.getClass.getSimpleName).mkString(", ")
-    it should s"error if a $fromN is renamed to $tosN" in {
+  val TopCircuit = cir
+  val Top = cir.module("Top")
+  val Top_m = Top.instOf("m", "Middle")
+  val Top_m_l = Top_m.instOf("l", "Leaf")
+  val Top_m_l_a = Top_m_l.ref("a")
+  val Top_m_la = Top_m.ref("l").field("a")
+  val Middle = cir.module("Middle")
+  val Middle2 = cir.module("Middle2")
+  val Middle_la = Middle.ref("l").field("a")
+  val Middle_l_a = Middle.instOf("l", "Leaf").ref("a")
+
+  it should "rename targets if modules in the path are renamed" in {
+    val renames = RenameMap()
+    renames.record(Middle, Middle2)
+    renames.get(Top_m) should be (Some(Seq(Top.instOf("m", "Middle2"))))
+  }
+
+  it should "rename targets if instance and module in the path are renamed" in {
+    val renames = RenameMap()
+    renames.record(Middle, Middle2)
+    renames.record(Top.ref("m"), Top.ref("m2"))
+    renames.get(Top_m) should be (Some(Seq(Top.instOf("m2", "Middle2"))))
+  }
+
+  it should "rename targets if instance in the path are renamed" in {
+    val renames = RenameMap()
+    renames.record(Top.ref("m"), Top.ref("m2"))
+    renames.get(Top_m) should be (Some(Seq(Top.instOf("m2", "Middle"))))
+  }
+
+  it should "rename targets if instance and ofmodule in the path are renamed" in {
+    val renames = RenameMap()
+    val Top_m2 = Top.instOf("m2", "Middle2")
+    renames.record(Top_m, Top_m2)
+    renames.get(Top_m) should be (Some(Seq(Top_m2)))
+  }
+
+  it should "properly do nothing if no remaps" in {
+    val renames = RenameMap()
+    renames.get(Top_m_l_a) should be (None)
+  }
+
+  it should "properly rename if leaf is inlined" in {
+    val renames = RenameMap()
+    renames.record(Middle_l_a, Middle_la)
+    renames.get(Top_m_l_a) should be (Some(Seq(Top_m_la)))
+  }
+
+  it should "properly rename if middle is inlined" in {
+    val renames = RenameMap()
+    renames.record(Top_m.ref("l"), Top.ref("m_l"))
+    renames.get(Top_m_l_a) should be (Some(Seq(Top.instOf("m_l", "Leaf").ref("a"))))
+  }
+
+  it should "properly rename if leaf and middle are inlined" in {
+    val renames = RenameMap()
+    val inlined = Top.ref("m_l_a")
+    renames.record(Top_m_l_a, inlined)
+    renames.record(Top_m_l, Nil)
+    renames.record(Top_m, Nil)
+    renames.get(Top_m_l_a) should be (Some(Seq(inlined)))
+  }
+
+  it should "quickly rename a target with a long path" in {
+    (0 until 50 by 10).foreach { endIdx =>
+      val renames = RenameMap()
+      renames.record(TopCircuit.module("Y0"), TopCircuit.module("X0"))
+      val deepTarget = (0 until endIdx).foldLeft(Top: IsModule) { (t, idx) =>
+        t.instOf("a", "A" + idx)
+      }.ref("ref")
+      val (millis, rename) = firrtl.Utils.time(renames.get(deepTarget))
+      println(s"${(deepTarget.tokens.size - 1) / 2} -> $millis")
+      //rename should be(None)
+    }
+  }
+
+  it should "rename with multiple renames" in {
+    val renames = RenameMap()
+    val Middle2 = cir.module("Middle2")
+    renames.record(Middle, Middle2)
+    renames.record(Middle.ref("l"), Middle.ref("lx"))
+    renames.get(Middle.ref("l")) should be (Some(Seq(Middle2.ref("lx"))))
+  }
+
+  it should "rename with fields" in {
+    val Middle_o = Middle.ref("o")
+    val Middle_i = Middle.ref("i")
+    val Middle_o_f = Middle.ref("o").field("f")
+    val Middle_i_f = Middle.ref("i").field("f")
+    val renames = RenameMap()
+    renames.record(Middle_o, Middle_i)
+    renames.get(Middle_o_f) should be (Some(Seq(Middle_i_f)))
+  }
+
+  it should "rename instances with same ofModule" in {
+    val Middle_o = Middle.ref("o")
+    val Middle_i = Middle.ref("i")
+    val renames = RenameMap()
+    renames.record(Middle_o, Middle_i)
+    renames.get(Middle.instOf("o", "O")) should be (Some(Seq(Middle.instOf("i", "O"))))
+  }
+
+  it should "detect circular renames" in {
+    case class BadRename(from: IsMember, tos: Seq[IsMember])
+    val badRenames =
+      Seq(
+        BadRename(foo, Seq(foo.field("bar"))),
+        BadRename(modA, Seq(foo))
+        //BadRename(cir, Seq(foo)),
+        //BadRename(cir, Seq(modA))
+      )
+    // Run all BadRename tests
+    for (BadRename(from, tos) <- badRenames) {
+      val fromN = from
+      val tosN = tos.mkString(", ")
+      //it should s"error if a $fromN is renamed to $tosN" in {
       val renames = RenameMap()
-      for (to <- tos) { renames.rename(from, to) }
-      a [FIRRTLException] shouldBe thrownBy {
-        renames.get(foo)
+      for (to <- tos) {
+        a [IllegalArgumentException] shouldBe thrownBy {
+          renames.record(from, to)
+        }
       }
+      //}
+    }
+  }
+
+  it should "be able to rename weird stuff" in {
+    // Renaming `from` to each of the `tos` at the same time should be ok
+    case class BadRename(from: CompleteTarget, tos: Seq[CompleteTarget])
+    val badRenames =
+      Seq(//BadRename(foo, Seq(cir)),
+        BadRename(foo, Seq(modB)),
+        BadRename(modA, Seq(fooB)),
+        //BadRename(modA, Seq(cir)),
+        //BadRename(cir, Seq(foo)),
+        //BadRename(cir, Seq(modA)),
+        BadRename(cir, Seq(cir2, cir3))
+      )
+    // Run all BadRename tests
+    for (BadRename(from, tos) <- badRenames) {
+      val fromN = from
+      val tosN = tos.mkString(", ")
+      //it should s"error if a $fromN is renamed to $tosN" in {
+        val renames = RenameMap()
+        for (to <- tos) {
+          (from, to) match {
+            case (f: CircuitTarget, t: CircuitTarget) => renames.record(f, t)
+            case (f: IsMember, t: IsMember) => renames.record(f, t)
+          }
+        }
+        //a [FIRRTLException] shouldBe thrownBy {
+        renames.get(from)
+        //}
+      //}
+    }
+  }
+
+  it should "error if a circular rename occurs" in {
+    val renames = RenameMap()
+    val top = CircuitTarget("Top")
+    renames.record(top.module("A"), top.module("B").instOf("c", "C"))
+    renames.record(top.module("B"), top.module("A").instOf("c", "C"))
+    a [CircularRenameException] shouldBe thrownBy {
+      renames.get(top.module("A"))
+    }
+  }
+
+  it should "not error if a swapping rename occurs" in {
+    val renames = RenameMap()
+    val top = CircuitTarget("Top")
+    renames.record(top.module("A"), top.module("B"))
+    renames.record(top.module("B"), top.module("A"))
+    renames.get(top.module("A")) should be (Some(Seq(top.module("B"))))
+    renames.get(top.module("B")) should be (Some(Seq(top.module("A"))))
+  }
+
+  it should "error if a reference is renamed to a module, and then we try to rename the reference's field" in {
+    val renames = RenameMap()
+    val top = CircuitTarget("Top")
+    renames.record(top.module("A").ref("ref"), top.module("B"))
+    renames.get(top.module("A").ref("ref")) should be(Some(Seq(top.module("B"))))
+    a [IllegalRenameException] shouldBe thrownBy {
+      renames.get(top.module("A").ref("ref").field("field"))
+    }
+    a [IllegalRenameException] shouldBe thrownBy {
+      renames.get(top.module("A").instOf("ref", "R"))
+    }
+  }
+
+  it should "error if we rename an instance's ofModule into a non-module" in {
+    val renames = RenameMap()
+    val top = CircuitTarget("Top")
+
+    renames.record(top.module("C"), top.module("D").ref("x"))
+    a [IllegalRenameException] shouldBe thrownBy {
+      renames.get(top.module("A").instOf("c", "C"))
+    }
+  }
+
+  it should "error if path is renamed into a non-path" ignore {
+    val renames = RenameMap()
+    val top = CircuitTarget("Top")
+
+    renames.record(top.module("E").instOf("f", "F"), top.module("E").ref("g"))
+
+    a [IllegalRenameException] shouldBe thrownBy {
+      println(renames.get(top.module("E").instOf("f", "F").ref("g")))
     }
   }
 }
diff --git a/src/test/scala/firrtlTests/annotationTests/EliminateTargetPathsSpec.scala b/src/test/scala/firrtlTests/annotationTests/EliminateTargetPathsSpec.scala
new file mode 100644
index 00000000..de84d79d
--- /dev/null
+++ b/src/test/scala/firrtlTests/annotationTests/EliminateTargetPathsSpec.scala
@@ -0,0 +1,357 @@
+// See LICENSE for license details.
+
+package firrtlTests.annotationTests
+
+import firrtl._
+import firrtl.annotations._
+import firrtl.annotations.analysis.DuplicationHelper
+import firrtl.annotations.transforms.NoSuchTargetException
+import firrtl.transforms.DontTouchAnnotation
+import firrtlTests.{FirrtlMatchers, FirrtlPropSpec}
+
+class EliminateTargetPathsSpec extends FirrtlPropSpec with FirrtlMatchers {
+  val input =
+    """circuit Top:
+      |  module Leaf:
+      |    input i: UInt<1>
+      |    output o: UInt<1>
+      |    o <= i
+      |    node a = i
+      |  module Middle:
+      |    input i: UInt<1>
+      |    output o: UInt<1>
+      |    inst l1 of Leaf
+      |    inst l2 of Leaf
+      |    l1.i <= i
+      |    l2.i <= l1.o
+      |    o <= l2.o
+      |  module Top:
+      |    input i: UInt<1>
+      |    output o: UInt<1>
+      |    inst m1 of Middle
+      |    inst m2 of Middle
+      |    m1.i <= i
+      |    m2.i <= m1.o
+      |    o <= m2.o
+    """.stripMargin
+
+  val TopCircuit = CircuitTarget("Top")
+  val Top = TopCircuit.module("Top")
+  val Middle = TopCircuit.module("Middle")
+  val Leaf = TopCircuit.module("Leaf")
+
+  val Top_m1_l1_a = Top.instOf("m1", "Middle").instOf("l1", "Leaf").ref("a")
+  val Top_m2_l1_a = Top.instOf("m2", "Middle").instOf("l1", "Leaf").ref("a")
+  val Top_m1_l2_a = Top.instOf("m1", "Middle").instOf("l2", "Leaf").ref("a")
+  val Top_m2_l2_a = Top.instOf("m2", "Middle").instOf("l2", "Leaf").ref("a")
+  val Middle_l1_a = Middle.instOf("l1", "Leaf").ref("a")
+  val Middle_l2_a = Middle.instOf("l2", "Leaf").ref("a")
+  val Leaf_a = Leaf.ref("a")
+
+  case class DummyAnnotation(target: Target) extends SingleTargetAnnotation[Target] {
+    override def duplicate(n: Target): Annotation = DummyAnnotation(n)
+  }
+  class DummyTransform() extends Transform with ResolvedAnnotationPaths {
+    override def inputForm: CircuitForm = LowForm
+    override def outputForm: CircuitForm = LowForm
+
+    override val annotationClasses: Traversable[Class[_]] = Seq(classOf[DummyAnnotation])
+
+    override def execute(state: CircuitState): CircuitState = state
+  }
+  val customTransforms = Seq(new DummyTransform())
+
+  val inputState = CircuitState(parse(input), ChirrtlForm)
+  property("Hierarchical tokens should be expanded properly") {
+    val dupMap = new DuplicationHelper(inputState.circuit.modules.map(_.name).toSet)
+
+
+    // Only a few instance references
+    dupMap.expandHierarchy(Top_m1_l1_a)
+    dupMap.expandHierarchy(Top_m2_l1_a)
+    dupMap.expandHierarchy(Middle_l1_a)
+
+    dupMap.makePathless(Top_m1_l1_a).foreach {Set(TopCircuit.module("Leaf___Top_m1_l1").ref("a")) should contain (_)}
+    dupMap.makePathless(Top_m2_l1_a).foreach {Set(TopCircuit.module("Leaf___Top_m2_l1").ref("a")) should contain (_)}
+    dupMap.makePathless(Top_m1_l2_a).foreach {Set(Leaf_a) should contain (_)}
+    dupMap.makePathless(Top_m2_l2_a).foreach {Set(Leaf_a) should contain (_)}
+    dupMap.makePathless(Middle_l1_a).foreach {Set(
+      TopCircuit.module("Leaf___Top_m1_l1").ref("a"),
+      TopCircuit.module("Leaf___Top_m2_l1").ref("a"),
+      TopCircuit.module("Leaf___Middle_l1").ref("a")
+    ) should contain (_) }
+    dupMap.makePathless(Middle_l2_a).foreach {Set(Leaf_a) should contain (_)}
+    dupMap.makePathless(Leaf_a).foreach {Set(
+      TopCircuit.module("Leaf___Top_m1_l1").ref("a"),
+      TopCircuit.module("Leaf___Top_m2_l1").ref("a"),
+      TopCircuit.module("Leaf___Middle_l1").ref("a"),
+      Leaf_a
+    ) should contain (_)}
+    dupMap.makePathless(Top).foreach {Set(Top) should contain (_)}
+    dupMap.makePathless(Middle).foreach {Set(
+      TopCircuit.module("Middle___Top_m1"),
+      TopCircuit.module("Middle___Top_m2"),
+      Middle
+    ) should contain (_)}
+    dupMap.makePathless(Leaf).foreach {Set(
+      TopCircuit.module("Leaf___Top_m1_l1"),
+      TopCircuit.module("Leaf___Top_m2_l1"),
+      TopCircuit.module("Leaf___Middle_l1"),
+      Leaf
+    ) should contain (_) }
+  }
+
+  property("Hierarchical donttouch should be resolved properly") {
+    val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DontTouchAnnotation(Top_m1_l1_a)))
+    val customTransforms = Seq(new LowFirrtlOptimization())
+    val outputState = new LowFirrtlCompiler().compile(inputState, customTransforms)
+    val check =
+      """circuit Top :
+        |  module Leaf___Top_m1_l1 :
+        |    input i : UInt<1>
+        |    output o : UInt<1>
+        |
+        |    node a = i
+        |    o <= i
+        |
+        |  module Leaf :
+        |    input i : UInt<1>
+        |    output o : UInt<1>
+        |
+        |    skip
+        |    o <= i
+        |
+        |  module Middle___Top_m1 :
+        |    input i : UInt<1>
+        |    output o : UInt<1>
+        |
+        |    inst l1 of Leaf___Top_m1_l1
+        |    inst l2 of Leaf
+        |    o <= l2.o
+        |    l1.i <= i
+        |    l2.i <= l1.o
+        |
+        |  module Middle :
+        |    input i : UInt<1>
+        |    output o : UInt<1>
+        |
+        |    inst l1 of Leaf
+        |    inst l2 of Leaf
+        |    o <= l2.o
+        |    l1.i <= i
+        |    l2.i <= l1.o
+        |
+        |  module Top :
+        |    input i : UInt<1>
+        |    output o : UInt<1>
+        |
+        |    inst m1 of Middle___Top_m1
+        |    inst m2 of Middle
+        |    o <= m2.o
+        |    m1.i <= i
+        |    m2.i <= m1.o
+        |
+      """.stripMargin
+    canonicalize(outputState.circuit).serialize should be (canonicalize(parse(check)).serialize)
+    outputState.annotations.collect {
+      case x: DontTouchAnnotation => x.target
+    } should be (Seq(Top.circuitTarget.module("Leaf___Top_m1_l1").ref("a")))
+  }
+
+  property("No name conflicts between old and new modules") {
+    val input =
+      """circuit Top:
+        |  module Middle:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+        |  module Top:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst m1 of Middle
+        |    inst m2 of Middle
+        |    inst x of Middle___Top_m1
+        |    x.i <= i
+        |    m1.i <= i
+        |    m2.i <= m1.o
+        |    o <= m2.o
+        |  module Middle___Top_m1:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+        |    node a = i
+      """.stripMargin
+    val checks =
+      """circuit Top :
+        |  module Middle :
+        |  module Top :
+        |  module Middle___Top_m1 :
+        |  module Middle____Top_m1 :""".stripMargin.split("\n")
+    val Top_m1 = Top.instOf("m1", "Middle")
+    val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DummyAnnotation(Top_m1)))
+    val outputState = new LowFirrtlCompiler().compile(inputState, customTransforms)
+    val outputLines = outputState.circuit.serialize.split("\n")
+    checks.foreach { line =>
+      outputLines should contain (line)
+    }
+  }
+
+  property("Previously unused modules should remain, but newly unused modules should be eliminated") {
+    val input =
+      """circuit Top:
+        |  module Leaf:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+        |    node a = i
+        |  module Middle:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+        |  module Top:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst m1 of Middle
+        |    inst m2 of Middle
+        |    m1.i <= i
+        |    m2.i <= m1.o
+        |    o <= m2.o
+      """.stripMargin
+
+    val checks =
+      """circuit Top :
+        |  module Leaf :
+        |  module Top :
+        |  module Middle___Top_m1 :
+        |  module Middle___Top_m2 :""".stripMargin.split("\n")
+
+    val Top_m1 = Top.instOf("m1", "Middle")
+    val Top_m2 = Top.instOf("m2", "Middle")
+    val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DummyAnnotation(Top_m1), DummyAnnotation(Top_m2)))
+    val outputState = new LowFirrtlCompiler().compile(inputState, customTransforms)
+    val outputLines = outputState.circuit.serialize.split("\n")
+
+    checks.foreach { line =>
+      outputLines should contain (line)
+    }
+    checks.foreach { line =>
+      outputLines should not contain ("  module Middle :")
+    }
+  }
+
+  property("Paths with incorrect names should error") {
+    val input =
+      """circuit Top:
+        |  module Leaf:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+        |    node a = i
+        |  module Middle:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+        |  module Top:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst m1 of Middle
+        |    inst m2 of Middle
+        |    m1.i <= i
+        |    m2.i <= m1.o
+        |    o <= m2.o
+      """.stripMargin
+    intercept[NoSuchTargetException] {
+      val Top_m1 = Top.instOf("m1", "MiddleX")
+      val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DummyAnnotation(Top_m1)))
+      new LowFirrtlCompiler().compile(inputState, customTransforms)
+    }
+    intercept[NoSuchTargetException] {
+      val Top_m2 = Top.instOf("x2", "Middle")
+      val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DummyAnnotation(Top_m2)))
+      new LowFirrtlCompiler().compile(inputState, customTransforms)
+    }
+  }
+
+  property("No name conflicts between two new modules") {
+    val input =
+      """circuit Top:
+        |  module Top:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst m1 of Middle_
+        |    inst m2 of Middle
+        |    m1.i <= i
+        |    m2.i <= m1.o
+        |    o <= m2.o
+        |  module Middle:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst _l of Leaf
+        |    _l.i <= i
+        |    o <= _l.o
+        |  module Middle_:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst l of Leaf
+        |    l.i <= i
+        |    node x = i
+        |    o <= l.o
+        |  module Leaf:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+      """.stripMargin
+    val checks =
+      """circuit Top :
+        |  module Middle :
+        |  module Top :
+        |  module Leaf___Middle__l :
+        |  module Leaf____Middle__l :""".stripMargin.split("\n")
+    val Middle_l1 = CircuitTarget("Top").module("Middle").instOf("_l", "Leaf")
+    val Middle_l2 = CircuitTarget("Top").module("Middle_").instOf("l", "Leaf")
+    val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DummyAnnotation(Middle_l1), DummyAnnotation(Middle_l2)))
+    val outputState = new LowFirrtlCompiler().compile(inputState, customTransforms)
+    val outputLines = outputState.circuit.serialize.split("\n")
+    checks.foreach { line =>
+      outputLines should contain (line)
+    }
+  }
+
+  property("Keep annotations of modules not instantiated") {
+    val input =
+      """circuit Top:
+        |  module Top:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst m1 of Middle
+        |    inst m2 of Middle
+        |    m1.i <= i
+        |    m2.i <= m1.o
+        |    o <= m2.o
+        |  module Middle:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    inst _l of Leaf
+        |    _l.i <= i
+        |    o <= _l.o
+        |  module Middle_:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= UInt(0)
+        |  module Leaf:
+        |    input i: UInt<1>
+        |    output o: UInt<1>
+        |    o <= i
+      """.stripMargin
+    val checks =
+      """circuit Top :
+        |  module Middle_ :""".stripMargin.split("\n")
+    val Middle_ = CircuitTarget("Top").module("Middle_").ref("i")
+    val inputState = CircuitState(parse(input), ChirrtlForm, Seq(DontTouchAnnotation(Middle_)))
+    val outputState = new VerilogCompiler().compile(inputState, customTransforms)
+    val outputLines = outputState.circuit.serialize.split("\n")
+    checks.foreach { line =>
+      outputLines should contain (line)
+    }
+  }
+}
diff --git a/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala b/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala
new file mode 100644
index 00000000..4ae4e036
--- /dev/null
+++ b/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala
@@ -0,0 +1,59 @@
+// See LICENSE for license details.
+
+package firrtlTests.annotationTests
+
+import firrtl.annotations.{CircuitTarget, GenericTarget, ModuleTarget, Target}
+import firrtl.annotations.TargetToken._
+import firrtlTests.FirrtlPropSpec
+
+class TargetSpec extends FirrtlPropSpec {
+  def check(comp: Target): Unit = {
+    val named = Target.convertTarget2Named(comp)
+    println(named)
+    val comp2 = Target.convertNamed2Target(named)
+    assert(comp.toGenericTarget.complete == comp2)
+  }
+  property("Serialization of Targets should work") {
+    val circuit = CircuitTarget("Circuit")
+    val top = circuit.module("Top")
+    val targets: Seq[(Target, String)] =
+      Seq(
+        (circuit, "~Circuit"),
+        (top, "~Circuit|Top"),
+        (top.instOf("i", "I"), "~Circuit|Top/i:I"),
+        (top.ref("r"), "~Circuit|Top>r"),
+        (top.ref("r").index(1).field("hi").clock, "~Circuit|Top>r[1].hi@clock"),
+        (GenericTarget(None, None, Vector(Ref("r"))), "~???|???>r")
+      )
+    targets.foreach { case (t, str) =>
+      assert(t.serialize == str, s"$t does not properly serialize")
+    }
+  }
+  property("Should convert to/from Named") {
+    check(Target(Some("Top"), None, Nil))
+    check(Target(Some("Top"), Some("Top"), Nil))
+    check(Target(Some("Top"), Some("Other"), Nil))
+    val r1 = Seq(Ref("r1"), Field("I"))
+    val r2 = Seq(Ref("r2"), Index(0))
+    check(Target(Some("Top"), Some("Top"), r1))
+    check(Target(Some("Top"), Some("Top"), r2))
+  }
+  property("Should enable creating from API") {
+    val top = ModuleTarget("Top","Top")
+    val x_reg0_data = top.instOf("x", "X").ref("reg0").field("data")
+    top.instOf("x", "x")
+    top.ref("y")
+    println(x_reg0_data)
+  }
+  property("Should serialize and deserialize") {
+    val circuit = CircuitTarget("Circuit")
+    val top = circuit.module("Top")
+    val targets: Seq[Target] =
+      Seq(circuit, top, top.instOf("i", "I"), top.ref("r"),
+        top.ref("r").index(1).field("hi").clock, GenericTarget(None, None, Vector(Ref("r"))))
+    targets.foreach { t =>
+      assert(Target.deserialize(t.serialize) == t, s"$t does not properly serialize/deserialize")
+    }
+  }
+}
+
diff --git a/src/test/scala/firrtlTests/transforms/DedupTests.scala b/src/test/scala/firrtlTests/transforms/DedupTests.scala
index b66f7f9d..5ee2b927 100644
--- a/src/test/scala/firrtlTests/transforms/DedupTests.scala
+++ b/src/test/scala/firrtlTests/transforms/DedupTests.scala
@@ -3,14 +3,24 @@
 package firrtlTests
 package transforms
 
+import firrtl.RenameMap
 import firrtl.annotations._
-import firrtl.transforms.{DedupModules}
+import firrtl.transforms.DedupModules
 
 
 /**
  * Tests inline instances transformation
  */
 class DedupModuleTests extends HighTransformSpec {
+  case class MultiTargetDummyAnnotation(targets: Seq[Target], tag: Int) extends Annotation {
+    override def update(renames: RenameMap): Seq[Annotation] = {
+      val newTargets = targets.flatMap(renames(_))
+      Seq(MultiTargetDummyAnnotation(newTargets, tag))
+    }
+  }
+  case class SingleTargetDummyAnnotation(target: ComponentName) extends SingleTargetAnnotation[ComponentName] {
+    override def duplicate(n: ComponentName): Annotation = SingleTargetDummyAnnotation(n)
+  }
   def transform = new DedupModules
   "The module A" should "be deduped" in {
      val input =
@@ -135,7 +145,7 @@ class DedupModuleTests extends HighTransformSpec {
           """.stripMargin
      execute(input, check, Seq(dontDedup("A")))
   }
-  "The module A and A_" should "be deduped even with different port names and info, and annotations should remap" in {
+  "The module A and A_" should "be deduped even with different port names and info, and annotations should remapped" in {
      val input =
         """circuit Top :
           |  module Top :
@@ -161,16 +171,12 @@ class DedupModuleTests extends HighTransformSpec {
           |    output x: UInt<1> @[yy 2:2]
           |    x <= UInt(1)
         """.stripMargin
-     case class DummyAnnotation(target: ComponentName) extends SingleTargetAnnotation[ComponentName] {
-       override def duplicate(n: ComponentName): Annotation = DummyAnnotation(n)
-     }
 
      val mname = ModuleName("Top", CircuitName("Top"))
-     val finalState = execute(input, check, Seq(DummyAnnotation(ComponentName("a2.y", mname))))
-
-     finalState.annotations.collect({ case d: DummyAnnotation => d }).head should be(DummyAnnotation(ComponentName("a2.x", mname)))
-
+     val finalState = execute(input, check, Seq(SingleTargetDummyAnnotation(ComponentName("a2.y", mname))))
+     finalState.annotations.collect({ case d: SingleTargetDummyAnnotation => d }).head should be(SingleTargetDummyAnnotation(ComponentName("a2.x", mname)))
   }
+
   "Extmodules" should "with the same defname and parameters should dedup" in {
      val input =
         """circuit Top :
@@ -215,6 +221,7 @@ class DedupModuleTests extends HighTransformSpec {
 
      execute(input, check, Seq.empty)
   }
+
   "Extmodules" should "with the different defname or parameters should NOT dedup" in {
      def mkfir(defnames: (String, String), params: (String, String)) =
        s"""circuit Top :
@@ -253,12 +260,188 @@ class DedupModuleTests extends HighTransformSpec {
         |    inst a2 of A_
         |  module A :
         |    output x: UInt<1>
+        |    inst b of B
+        |    x <= b.x
+        |  module A_ :
+        |    output x: UInt<1>
+        |    inst b of B_
+        |    x <= b.x
+        |  module B :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+        |  module B_ :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+      """.stripMargin
+    val check =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A
+        |  module A :
+        |    output x: UInt<1>
+        |    inst b of B
+        |    x <= b.x
+        |  module B :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+      """.stripMargin
+    execute(input, check, Seq.empty)
+  }
+
+  "The module A and A_" should "be deduped with fields that sort of match" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
+        |    wire b: {c: UInt<1>}
+        |    x <= b.c
+        |  module A_ :
+        |    output x: UInt<1>
+        |    wire b: {b: UInt<1>}
+        |    x <= b.b
+      """.stripMargin
+    val check =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A
+        |  module A :
+        |    output x: UInt<1>
+        |    wire b: {c: UInt<1>}
+        |    x <= b.c
+      """.stripMargin
+    execute(input, check, Seq.empty)
+  }
+
+  "The module A and A_" should "not be deduped with different annotation targets" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+        |  module A_ :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+      """.stripMargin
+    val check =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+        |  module A_ :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+      """.stripMargin
+    execute(input, check, Seq(dontTouch("A.b")))
+  }
+
+  "The module A and A_" should "be deduped with same annotation targets" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+        |  module A_ :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+      """.stripMargin
+    val check =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A
+        |  module A :
+        |    output x: UInt<1>
+        |    wire b: UInt<1>
+        |    x <= b
+      """.stripMargin
+    execute(input, check, Seq(dontTouch("A.b"), dontTouch("A_.b")))
+  }
+  "The module A and A_" should "not be deduped with same annotations with same multi-targets, but which have different root modules" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
+        |    inst b of B
+        |    x <= b.x
+        |  module A_ :
+        |    output x: UInt<1>
         |    inst b of B_
         |    x <= b.x
+        |  module B :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+        |  module B_ :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+      """.stripMargin
+    val check =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
+        |    inst b of B
+        |    x <= b.x
         |  module A_ :
         |    output x: UInt<1>
+        |    inst b of B_
+        |    x <= b.x
+        |  module B :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+        |  module B_ :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+      """.stripMargin
+    val Top = CircuitTarget("Top")
+    val A = Top.module("A")
+    val B = Top.module("B")
+    val A_ = Top.module("A_")
+    val B_ = Top.module("B_")
+    val annoAB = MultiTargetDummyAnnotation(Seq(A, B), 0)
+    val annoA_B_ = MultiTargetDummyAnnotation(Seq(A_, B_), 0)
+    val cs = execute(input, check, Seq(annoAB, annoA_B_))
+    cs.annotations.toSeq should contain (annoAB)
+    cs.annotations.toSeq should contain (annoA_B_)
+  }
+  "The module A and A_" should "be deduped with same annotations with same multi-targets, that share roots" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output x: UInt<1>
         |    inst b of B
         |    x <= b.x
+        |  module A_ :
+        |    output x: UInt<1>
+        |    inst b of B_
+        |    x <= b.x
         |  module B :
         |    output x: UInt<1>
         |    x <= UInt(1)
@@ -279,7 +462,47 @@ class DedupModuleTests extends HighTransformSpec {
         |    output x: UInt<1>
         |    x <= UInt(1)
       """.stripMargin
-    execute(input, check, Seq.empty)
+    val Top = CircuitTarget("Top")
+    val A = Top.module("A")
+    val A_ = Top.module("A_")
+    val annoA = MultiTargetDummyAnnotation(Seq(A, A.instOf("b", "B")), 0)
+    val annoA_ = MultiTargetDummyAnnotation(Seq(A_, A_.instOf("b", "B_")), 0)
+    val cs = execute(input, check, Seq(annoA, annoA_))
+    cs.annotations.toSeq should contain (annoA)
+    cs.annotations.toSeq should not contain (annoA_)
+    cs.deletedAnnotations.isEmpty should be (true)
+  }
+  "The deduping module A and A_" should "renamed internal signals that have different names" in {
+    val input =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A_
+        |  module A :
+        |    output y: UInt<1>
+        |    y <= UInt(1)
+        |  module A_ :
+        |    output x: UInt<1>
+        |    x <= UInt(1)
+      """.stripMargin
+    val check =
+      """circuit Top :
+        |  module Top :
+        |    inst a1 of A
+        |    inst a2 of A
+        |  module A :
+        |    output y: UInt<1>
+        |    y <= UInt<1>("h1")
+      """.stripMargin
+    val Top = CircuitTarget("Top")
+    val A = Top.module("A")
+    val A_ = Top.module("A_")
+    val annoA = SingleTargetDummyAnnotation(A.ref("y"))
+    val annoA_ = SingleTargetDummyAnnotation(A_.ref("x"))
+    val cs = execute(input, check, Seq(annoA, annoA_))
+    cs.annotations.toSeq should contain (annoA)
+    cs.annotations.toSeq should not contain (SingleTargetDummyAnnotation(A.ref("x")))
+    cs.deletedAnnotations.isEmpty should be (true)
   }
 }