Fix memory annotation deduplication (#2286)

* Add transform to deduplicate memory annotations
* Add annotation deduplication to Dedup stage
* ResolveAnnotationPaths and EliminateTargetPaths now invalidate the dedup annotations transform
* Verilog emitter now throws exception when memory annotations fail to dedup

Co-authored-by: Jack Koenig <koenig@sifive.com>

4 files changed, 49 insertions, 2 deletions

diff --git a/src/main/scala/firrtl/annotations/Annotation.scala b/src/main/scala/firrtl/annotations/Annotation.scala
index 08555a84..f4d6ee55 100644
--- a/src/main/scala/firrtl/annotations/Annotation.scala
+++ b/src/main/scala/firrtl/annotations/Annotation.scala
@@ -38,6 +38,21 @@ trait Annotation extends Product {
     * @return
     */
   def getTargets: Seq[Target] = extractComponents(productIterator.toIterable).toSeq
+
+  /** Returns a deduplicable representation of this [[Annotation]]: a 3-tuple of the
+    * deduplicated annotation's "dedup key", the deduplicated [[Annotation]], and the
+    * [[firrtl.annotations.ReferenceTarget ReferenceTarget]](s) to the annotated objects.
+    *
+    * If two absolute instances of this [[Annotation]] would deduplicate to the same
+    * local form, both of their "dedup key"s must be equivalent.
+    *
+    * A deduplication key is typically taken to be a 2-tuple of the pathless target and
+    * the annotation's value.
+    *
+    * Returning None signifies this annotation will not deduplicate.
+    * @return
+    */
+  private[firrtl] def dedup: Option[(Any, Annotation, ReferenceTarget)] = None
 }
 
 /** If an Annotation does not target any [[Named]] thing in the circuit, then all updates just
diff --git a/src/main/scala/firrtl/annotations/MemoryInitAnnotation.scala b/src/main/scala/firrtl/annotations/MemoryInitAnnotation.scala
index 1e81301d..e87066fb 100644
--- a/src/main/scala/firrtl/annotations/MemoryInitAnnotation.scala
+++ b/src/main/scala/firrtl/annotations/MemoryInitAnnotation.scala
@@ -8,7 +8,8 @@ import firrtl.{
   MemoryFileInlineInit,
   MemoryInitValue,
   MemoryRandomInit,
-  MemoryScalarInit
+  MemoryScalarInit,
+  Utils
 }
 
 /**
@@ -25,6 +26,9 @@ case class MemoryRandomInitAnnotation(target: ReferenceTarget) extends MemoryIni
   override def duplicate(n: ReferenceTarget): Annotation = copy(n)
   override def initValue:    MemoryInitValue = MemoryRandomInit
   override def isRandomInit: Boolean = true
+  override private[firrtl] def dedup: Option[(Any, Annotation, ReferenceTarget)] = Some(
+    ((target.pathlessTarget, Nil), copy(target = target.pathlessTarget), target)
+  )
 }
 
 /** Initialize all entries of the `target` memory with the scalar `value`. */
@@ -32,6 +36,9 @@ case class MemoryScalarInitAnnotation(target: ReferenceTarget, value: BigInt) ex
   override def duplicate(n: ReferenceTarget): Annotation = copy(n)
   override def initValue:    MemoryInitValue = MemoryScalarInit(value)
   override def isRandomInit: Boolean = false
+  override private[firrtl] def dedup: Option[(Any, Annotation, ReferenceTarget)] = Some(
+    ((target.pathlessTarget, value), copy(target = target.pathlessTarget), target)
+  )
 }
 
 /** Initialize the `target` memory with the array of `values` which must be the same size as the memory depth. */
@@ -39,6 +46,9 @@ case class MemoryArrayInitAnnotation(target: ReferenceTarget, values: Seq[BigInt
   override def duplicate(n: ReferenceTarget): Annotation = copy(n)
   override def initValue:    MemoryInitValue = MemoryArrayInit(values)
   override def isRandomInit: Boolean = false
+  override private[firrtl] def dedup: Option[(Any, Annotation, ReferenceTarget)] = Some(
+    ((target.pathlessTarget, values), copy(target = target.pathlessTarget), target)
+  )
 }
 
 /** Initialize the `target` memory with inline readmem[hb] statement. */
@@ -51,6 +61,9 @@ case class MemoryFileInlineAnnotation(
   override def duplicate(n: ReferenceTarget): Annotation = copy(n)
   override def initValue:    MemoryInitValue = MemoryFileInlineInit(filename, hexOrBinary)
   override def isRandomInit: Boolean = false
+  override private[firrtl] def dedup: Option[(Any, Annotation, ReferenceTarget)] = Some(
+    ((target.pathlessTarget, filename), copy(target = target.pathlessTarget), target)
+  )
 }
 
 /** Initializes the memory inside the `ifndef SYNTHESIS` block (default) */
diff --git a/src/main/scala/firrtl/annotations/Target.scala b/src/main/scala/firrtl/annotations/Target.scala
index 92339946..02ec42b8 100644
--- a/src/main/scala/firrtl/annotations/Target.scala
+++ b/src/main/scala/firrtl/annotations/Target.scala
@@ -695,6 +695,15 @@ case class ReferenceTarget(
     }
   }
 
+  /** Returns the local form of this [[ReferenceTarget]]
+    *
+    * For example, given `~Top|Top/foo:Foo/bar:Bar>x`,
+    *
+    * `.pathlessTarget` returns `~Top|Bar>x`
+    *
+    * This is useful for cases in which annotations must point to the module itself rather than
+    *   an absolute *instance* of the module (e.g. deduplication).
+    */
   override def pathlessTarget: ReferenceTarget = ReferenceTarget(circuit, encapsulatingModule, Nil, ref, component)
 
   override def setPathTarget(newPath: IsModule): ReferenceTarget =
@@ -789,6 +798,15 @@ case class InstanceTarget(
 
   override def asPath: Seq[(Instance, OfModule)] = path :+ ((Instance(instance), OfModule(ofModule)))
 
+  /** Returns the local form of this [[InstanceTarget]]
+    *
+    * For example, given `~Top|Top/foo:Foo/bar:Bar`,
+    *
+    * `.pathlessTarget` returns `~Top|Foo/bar:Bar`
+    *
+    * This is useful for cases in which annotations must point to the module itself rather than
+    *   an absolute *instance* of the module (e.g. deduplication).
+    */
   override def pathlessTarget: InstanceTarget = InstanceTarget(circuit, encapsulatingModule, Nil, instance, ofModule)
 
   override def notPath = Seq(Instance(instance), OfModule(ofModule))
diff --git a/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala b/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala
index b63dd13e..104aafc3 100644
--- a/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala
+++ b/src/main/scala/firrtl/annotations/transforms/EliminateTargetPaths.scala
@@ -12,6 +12,7 @@ import firrtl.ir._
 import firrtl.{AnnotationSeq, CircuitState, DependencyAPIMigration, FirrtlInternalException, RenameMap, Transform}
 import firrtl.stage.Forms
 import firrtl.transforms.DedupedResult
+import firrtl.transforms.DedupAnnotationsTransform
 
 import scala.collection.mutable
 
@@ -105,7 +106,7 @@ class EliminateTargetPaths extends Transform with DependencyAPIMigration {
   override def prerequisites = Forms.MinimalHighForm
   override def optionalPrerequisites = Seq.empty
   override def optionalPrerequisiteOf = Seq.empty
-  override def invalidates(a: Transform) = false
+  override def invalidates(a: Transform) = a.isInstanceOf[DedupAnnotationsTransform]
 
   /** Replaces old ofModules with new ofModules by calling dupMap methods
     * Updates oldUsedOfModules, newUsedOfModules