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>

10 files changed, 330 insertions, 26 deletions

diff --git a/src/main/scala/firrtl/Compiler.scala b/src/main/scala/firrtl/Compiler.scala
index 7354b8ee..2017633d 100644
--- a/src/main/scala/firrtl/Compiler.scala
+++ b/src/main/scala/firrtl/Compiler.scala
@@ -14,6 +14,7 @@ import firrtl.Utils.throwInternalError
 import firrtl.annotations.transforms.{EliminateTargetPaths, ResolvePaths}
 import firrtl.options.{Dependency, DependencyAPI, StageUtils, TransformLike}
 import firrtl.stage.Forms
+import firrtl.transforms.DedupAnnotationsTransform
 
 /** Container of all annotations for a Firrtl compiler */
 class AnnotationSeq private (private[firrtl] val underlying: List[Annotation]) {
@@ -396,6 +397,9 @@ trait ResolvedAnnotationPaths {
   override def prepare(state: CircuitState): CircuitState = {
     state.resolvePathsOf(annotationClasses.toSeq: _*)
   }
+
+  // Any transform with this trait invalidates DedupAnnotationsTransform
+  override def invalidates(a: Transform) = a.isInstanceOf[DedupAnnotationsTransform]
 }
 
 /** Defines old API for Emission. Deprecated */
diff --git a/src/main/scala/firrtl/Utils.scala b/src/main/scala/firrtl/Utils.scala
index a58b6997..e29c1a3b 100644
--- a/src/main/scala/firrtl/Utils.scala
+++ b/src/main/scala/firrtl/Utils.scala
@@ -355,6 +355,7 @@ object Utils extends LazyLogging {
     onExp(expression)
     ReferenceTarget(main, module, Nil, ref, tokens.toSeq)
   }
+
   @deprecated("get_flip is fundamentally slow, use to_flip(flow(expr))", "FIRRTL 1.2")
   def get_flip(t: Type, i: Int, f: Orientation): Orientation = {
     if (i >= get_size(t)) throwInternalError(s"get_flip: shouldn't be here - $i >= get_size($t)")
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
diff --git a/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala b/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala
index 13f7b793..8b20d365 100644
--- a/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala
+++ b/src/main/scala/firrtl/backends/verilog/VerilogEmitter.scala
@@ -9,6 +9,7 @@ import firrtl.WrappedExpression._
 import firrtl.traversals.Foreachers._
 import firrtl.annotations.{
   CircuitTarget,
+  MemoryInitAnnotation,
   MemoryLoadFileType,
   MemoryNoSynthInit,
   MemorySynthInit,
@@ -507,6 +508,18 @@ class VerilogEmitter extends SeqTransform with Emitter {
     private val emissionAnnos = annotations.collect {
       case m: SingleTargetAnnotation[ReferenceTarget] @unchecked with EmissionOption => m
     }
+
+    // Check for non-local memory annotations (error if found)
+    emissionAnnos.foreach {
+      case a: MemoryInitAnnotation => {
+        if (!a.target.isLocal)
+          throw new FirrtlUserException(
+            "At least one memory annotation did not deduplicate: got non-local annotation $a from [[DedupAnnotationsTransform]]"
+          )
+      }
+      case _ =>
+    }
+
     // using multiple foreach instead of a single partial function as an Annotation can gather multiple EmissionOptions for simplicity
     emissionAnnos.foreach {
       case a: MemoryEmissionOption => memoryEmissionOption += ((a.target, a))
diff --git a/src/main/scala/firrtl/stage/Forms.scala b/src/main/scala/firrtl/stage/Forms.scala
index 4132f758..c7ae648a 100644
--- a/src/main/scala/firrtl/stage/Forms.scala
+++ b/src/main/scala/firrtl/stage/Forms.scala
@@ -47,7 +47,11 @@ object Forms {
       Dependency[firrtl.transforms.InferResets]
     )
 
-  val Deduped: Seq[TransformDependency] = Resolved :+ Dependency[firrtl.transforms.DedupModules]
+  val Deduped: Seq[TransformDependency] = Resolved ++
+    Seq(
+      Dependency[firrtl.transforms.DedupModules],
+      Dependency[firrtl.transforms.DedupAnnotationsTransform]
+    )
 
   val HighForm: Seq[TransformDependency] = ChirrtlForm ++
     MinimalHighForm ++
diff --git a/src/main/scala/firrtl/transforms/DedupAnnotations.scala b/src/main/scala/firrtl/transforms/DedupAnnotations.scala
new file mode 100644
index 00000000..9355b5c3
--- /dev/null
+++ b/src/main/scala/firrtl/transforms/DedupAnnotations.scala
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package firrtl
+package transforms
+
+import firrtl.ir._
+import firrtl.Mappers._
+import firrtl.options.Dependency
+import firrtl.Utils.BoolType
+import firrtl.annotations.Annotation
+import scala.collection.mutable.Buffer
+import firrtl.annotations.MemoryFileInlineAnnotation
+import firrtl.passes.PassException
+import firrtl.annotations.ReferenceTarget
+import firrtl.annotations._
+import firrtl.analyses.InstanceKeyGraph
+
+import scala.collection.mutable.ArrayBuffer
+
+object DedupAnnotationsTransform {
+
+  final class DifferingModuleAnnotationsException private (msg: String) extends PassException(msg)
+  object DifferingModuleAnnotationsException {
+    def apply(left: ReferenceTarget, right: ReferenceTarget): DifferingModuleAnnotationsException = {
+      val msg = s"${left.serialize} and ${right.serialize} have differing module binaries"
+      new DifferingModuleAnnotationsException(msg)
+    }
+  }
+
+  private case class DedupableRepr(
+    dedupKey:       Any,
+    deduped:        Annotation,
+    original:       Annotation,
+    absoluteTarget: ReferenceTarget)
+  private object DedupableRepr {
+    def apply(annotation: Annotation): Option[DedupableRepr] = annotation.dedup match {
+      case Some((dedupKey, dedupedAnno, absoluteTarget)) =>
+        Some(new DedupableRepr(dedupKey, dedupedAnno, annotation, absoluteTarget))
+      case _ => None
+    }
+  }
+
+  private type InstancePath = Seq[(TargetToken.Instance, TargetToken.OfModule)]
+
+  private def checkInstanceGraph(
+    module:        String,
+    graph:         InstanceKeyGraph,
+    absolutePaths: Seq[InstancePath]
+  ): Boolean = graph.findInstancesInHierarchy(module).size == absolutePaths.size
+
+  def dedupAnnotations(annotations: Seq[Annotation], graph: InstanceKeyGraph): Seq[Annotation] = {
+    val canDedup = ArrayBuffer.empty[DedupableRepr]
+    val outAnnos = ArrayBuffer.empty[Annotation]
+
+    // Extract the annotations which can be deduplicated
+    annotations.foreach { anno =>
+      DedupableRepr(anno) match {
+        case Some(repr) => canDedup += repr
+        case None       => outAnnos += anno
+      }
+    }
+
+    // Partition the dedupable annotations into groups that *should* deduplicate into the same annotation
+    val shouldDedup: Map[Any, ArrayBuffer[DedupableRepr]] = canDedup.groupBy(_.dedupKey)
+    shouldDedup.foreach {
+      case ((target: ReferenceTarget, _), dedupableAnnos) =>
+        val originalAnnos = dedupableAnnos.map(_.original)
+        val uniqueDedupedAnnos = dedupableAnnos.map(_.deduped).distinct
+        // TODO: Extend this to support multi-target annotations
+        val instancePaths = dedupableAnnos.map(_.absoluteTarget.path).toSeq
+        // The annotation deduplication is only legal if it applies to *all* instances of a
+        // deduplicated module -- requires an instance graph check
+        if (uniqueDedupedAnnos.size == 1 && checkInstanceGraph(target.encapsulatingModule, graph, instancePaths))
+          outAnnos += uniqueDedupedAnnos.head
+        else
+          outAnnos ++= originalAnnos
+    }
+
+    outAnnos.toSeq
+  }
+}
+
+/** Deduplicates memory annotations
+  */
+class DedupAnnotationsTransform extends Transform with DependencyAPIMigration {
+
+  override def prerequisites = Nil
+
+  override def optionalPrerequisites = Nil
+
+  override def optionalPrerequisiteOf = Nil
+
+  override def invalidates(a: Transform) = false
+
+  def execute(state: CircuitState): CircuitState = CircuitState(
+    state.circuit,
+    state.form,
+    DedupAnnotationsTransform.dedupAnnotations(state.annotations.underlying, InstanceKeyGraph(state.circuit)),
+    state.renames
+  )
+}
diff --git a/src/test/scala/firrtlTests/MemoryInitSpec.scala b/src/test/scala/firrtlTests/MemoryInitSpec.scala
index 44f0162e..688adcb9 100644
--- a/src/test/scala/firrtlTests/MemoryInitSpec.scala
+++ b/src/test/scala/firrtlTests/MemoryInitSpec.scala
@@ -9,7 +9,11 @@ import firrtl.testutils.FirrtlFlatSpec
 import firrtlTests.execution._
 
 class MemInitSpec extends FirrtlFlatSpec {
-  def input(tpe: String): String =
+  def compile(circuit: String, annos: AnnotationSeq): CircuitState = {
+    (new VerilogCompiler).compileAndEmit(CircuitState(parse(circuit), ChirrtlForm, annos))
+  }
+
+  def basicTest(tpe: String = "UInt<32>"): String =
     s"""
        |circuit MemTest:
        |  module MemTest:
@@ -44,25 +48,22 @@ class MemInitSpec extends FirrtlFlatSpec {
        |""".stripMargin
 
   val mRef = CircuitTarget("MemTest").module("MemTest").ref("m")
-  def compile(annos: AnnotationSeq, tpe: String = "UInt<32>"): CircuitState = {
-    (new VerilogCompiler).compileAndEmit(CircuitState(parse(input(tpe)), ChirrtlForm, annos))
-  }
 
   "NoAnnotation" should "create a randomized initialization" in {
     val annos = Seq()
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLine("    m[initvar] = _RAND_0[31:0];")
   }
 
   "MemoryRandomInitAnnotation" should "create a randomized initialization" in {
     val annos = Seq(MemoryRandomInitAnnotation(mRef))
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLine("    m[initvar] = _RAND_0[31:0];")
   }
 
   "MemoryScalarInitAnnotation w/ 0" should "create an initialization with all zeros" in {
     val annos = Seq(MemoryScalarInitAnnotation(mRef, 0))
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLine("    m[initvar] = 0;")
   }
 
@@ -70,7 +71,7 @@ class MemInitSpec extends FirrtlFlatSpec {
     s"MemoryScalarInitAnnotation w/ $value" should
       s"create an initialization with all values set to $value" in {
       val annos = Seq(MemoryScalarInitAnnotation(mRef, value))
-      val result = compile(annos)
+      val result = compile(basicTest(), annos)
       result should containLine(s"    m[initvar] = $value;")
     }
   }
@@ -78,7 +79,7 @@ class MemInitSpec extends FirrtlFlatSpec {
   "MemoryArrayInitAnnotation" should "initialize all addresses" in {
     val values = Seq.tabulate(32)(ii => 2 * ii + 5).map(BigInt(_))
     val annos = Seq(MemoryArrayInitAnnotation(mRef, values))
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     values.zipWithIndex.foreach {
       case (value, addr) =>
         result should containLine(s"  m[$addr] = $value;")
@@ -87,48 +88,48 @@ class MemInitSpec extends FirrtlFlatSpec {
 
   "MemoryScalarInitAnnotation" should "fail for a negative value" in {
     assertThrows[EmitterException] {
-      compile(Seq(MemoryScalarInitAnnotation(mRef, -1)))
+      compile(basicTest(), Seq(MemoryScalarInitAnnotation(mRef, -1)))
     }
   }
 
   "MemoryScalarInitAnnotation" should "fail for a value that is too large" in {
     assertThrows[EmitterException] {
-      compile(Seq(MemoryScalarInitAnnotation(mRef, BigInt(1) << 32)))
+      compile(basicTest(), Seq(MemoryScalarInitAnnotation(mRef, BigInt(1) << 32)))
     }
   }
 
   "MemoryArrayInitAnnotation" should "fail for a negative value" in {
     assertThrows[EmitterException] {
       val values = Seq.tabulate(32)(_ => BigInt(-1))
-      compile(Seq(MemoryArrayInitAnnotation(mRef, values)))
+      compile(basicTest(), Seq(MemoryArrayInitAnnotation(mRef, values)))
     }
   }
 
   "MemoryArrayInitAnnotation" should "fail for a value that is too large" in {
     assertThrows[EmitterException] {
       val values = Seq.tabulate(32)(_ => BigInt(1) << 32)
-      compile(Seq(MemoryArrayInitAnnotation(mRef, values)))
+      compile(basicTest(), Seq(MemoryArrayInitAnnotation(mRef, values)))
     }
   }
 
   "MemoryArrayInitAnnotation" should "fail if the number of values is too small" in {
     assertThrows[EmitterException] {
       val values = Seq.tabulate(31)(_ => BigInt(1))
-      compile(Seq(MemoryArrayInitAnnotation(mRef, values)))
+      compile(basicTest(), Seq(MemoryArrayInitAnnotation(mRef, values)))
     }
   }
 
   "MemoryArrayInitAnnotation" should "fail if the number of values is too large" in {
     assertThrows[EmitterException] {
       val values = Seq.tabulate(33)(_ => BigInt(1))
-      compile(Seq(MemoryArrayInitAnnotation(mRef, values)))
+      compile(basicTest(), Seq(MemoryArrayInitAnnotation(mRef, values)))
     }
   }
 
   "MemoryScalarInitAnnotation on Memory with Vector type" should "fail" in {
     val caught = intercept[Exception] {
       val annos = Seq(MemoryScalarInitAnnotation(mRef, 0))
-      compile(annos, "UInt<32>[2]")
+      compile(basicTest("UInt<32>[2]"), annos)
     }
     assert(caught.getMessage.endsWith("Cannot initialize memory m of non ground type UInt<32>[2]"))
   }
@@ -136,7 +137,7 @@ class MemInitSpec extends FirrtlFlatSpec {
   "MemoryScalarInitAnnotation on Memory with Bundle type" should "fail" in {
     val caught = intercept[Exception] {
       val annos = Seq(MemoryScalarInitAnnotation(mRef, 0))
-      compile(annos, "{real: SInt<10>, imag: SInt<10>}")
+      compile(basicTest("{real: SInt<10>, imag: SInt<10>}"), annos)
     }
     assert(
       caught.getMessage.endsWith("Cannot initialize memory m of non ground type { real : SInt<10>, imag : SInt<10>}")
@@ -167,19 +168,19 @@ class MemInitSpec extends FirrtlFlatSpec {
 
   "MemoryFileInlineAnnotation" should "emit $readmemh for text.hex" in {
     val annos = Seq(MemoryFileInlineAnnotation(mRef, filename = "text.hex"))
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLine("""$readmemh("text.hex", """ + mRef.name + """);""")
   }
 
   "MemoryFileInlineAnnotation" should "emit $readmemb for text.bin" in {
     val annos = Seq(MemoryFileInlineAnnotation(mRef, filename = "text.bin", hexOrBinary = MemoryLoadFileType.Binary))
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLine("""$readmemb("text.bin", """ + mRef.name + """);""")
   }
 
   "MemoryFileInlineAnnotation" should "fail with blank filename" in {
     assertThrows[Exception] {
-      compile(Seq(MemoryFileInlineAnnotation(mRef, filename = "")))
+      compile(basicTest(), Seq(MemoryFileInlineAnnotation(mRef, filename = "")))
     }
   }
 
@@ -187,7 +188,7 @@ class MemInitSpec extends FirrtlFlatSpec {
     val annos = Seq(
       MemoryFileInlineAnnotation(mRef, filename = "text.hex", hexOrBinary = MemoryLoadFileType.Hex)
     )
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLines(
       """`endif // RANDOMIZE""",
       """$readmemh("text.hex", """ + mRef.name + """);""",
@@ -199,7 +200,7 @@ class MemInitSpec extends FirrtlFlatSpec {
     val annos = Seq(
       MemoryFileInlineAnnotation(mRef, filename = "text.hex", hexOrBinary = MemoryLoadFileType.Hex)
     ) ++ Seq(MemorySynthInit)
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLines(
       """`endif // SYNTHESIS""",
       """initial begin""",
@@ -213,13 +214,146 @@ class MemInitSpec extends FirrtlFlatSpec {
       MemoryFileInlineAnnotation(mRef, filename = "text.hex", hexOrBinary = MemoryLoadFileType.Hex)
     ) ++ Seq(MemoryNoSynthInit)
 
-    val result = compile(annos)
+    val result = compile(basicTest(), annos)
     result should containLines(
       """`endif // RANDOMIZE""",
       """$readmemh("text.hex", """ + mRef.name + """);""",
       """end // initial"""
     )
   }
+
+  /** Firrtl which contains a child memory module instantiated twice
+    * If deduplication occurs, the firrtl for the child module should appear only
+    * once
+    * Any non-local memory annotations bound to the 'm' memories in each child instance
+    * should properly deduplicate in order for them to be emitted in the Verilog
+    */
+  def dedupTest =
+    s"""
+       |circuit Top:
+       |  module Child:
+       |    input clock : Clock
+       |    input rAddr : UInt<5>
+       |    input rEnable : UInt<1>
+       |    input wAddr : UInt<5>
+       |    input wData : UInt<8>
+       |    input wEnable : UInt<1>
+       |    output rData : UInt<8>
+       |
+       |    mem m:
+       |      data-type => UInt<8>
+       |      depth => 32
+       |      reader => r
+       |      writer => w
+       |      read-latency => 1
+       |      write-latency => 1
+       |      read-under-write => new
+       |
+       |    m.r.clk <= clock
+       |    m.r.addr <= rAddr
+       |    m.r.en <= rEnable
+       |    rData <= m.r.data
+       |
+       |    m.w.clk <= clock
+       |    m.w.addr <= wAddr
+       |    m.w.en <= wEnable
+       |    m.w.data <= wData
+       |    m.w.mask is invalid
+       |
+       |  module Top:
+       |    input clock : Clock
+       |    input rAddr : UInt<5>
+       |    input rEnable : UInt<1>
+       |    input wAddr : UInt<5>
+       |    input wData : UInt<8>
+       |    input wEnable : UInt<1>
+       |    output rData : UInt<8>[2]
+       |
+       |    inst c1 of Child
+       |    c1.clock <= clock
+       |    c1.rAddr <= rAddr
+       |    c1.rEnable <= rEnable
+       |    c1.wAddr <= wAddr
+       |    c1.wData <= wData
+       |    c1.wEnable <= wEnable
+       |
+       |    inst c2 of Child
+       |    c2.clock <= clock
+       |    c2.rAddr <= rAddr
+       |    c2.rEnable <= rEnable
+       |    c2.wAddr <= wAddr
+       |    c2.wData <= wData
+       |    c2.wEnable <= wEnable
+       |
+       |    rData[0] <= c1.rData
+       |    rData[1] <= c2.rData
+       |""".stripMargin
+
+  // Absolute references to the memory objects in each child module
+  val child1MRef = CircuitTarget("Top").module("Top").instOf("c1", "Child").ref("m")
+  val child2MRef = CircuitTarget("Top").module("Top").instOf("c2", "Child").ref("m")
+  // Final deduplicated reference
+  val dedupedRef = CircuitTarget("Top").module("Child").ref("m")
+
+  "MemoryRandomInitAnnotation" should "randomize memory in single deduped module" in {
+    val annos = Seq(
+      MemoryRandomInitAnnotation(child1MRef),
+      MemoryRandomInitAnnotation(child2MRef)
+    )
+    val result = compile(dedupTest, annos)
+    result should containLine("      m[initvar] = _RAND_0[7:0];")
+  }
+
+  "MemoryScalarInitAnnotation" should "initialize memory to 0 in deduped module" in {
+    val annos = Seq(
+      MemoryScalarInitAnnotation(child1MRef, value = 0),
+      MemoryScalarInitAnnotation(child2MRef, value = 0)
+    )
+    val result = compile(dedupTest, annos)
+    result should containLine("      m[initvar] = 0;")
+  }
+
+  "MemoryArrayInitAnnotation" should "initialize memory with array of values in deduped module" in {
+    val values = Seq.tabulate(32)(ii => 2 * ii + 5).map(BigInt(_))
+    val annos = Seq(
+      MemoryArrayInitAnnotation(child1MRef, values),
+      MemoryArrayInitAnnotation(child2MRef, values)
+    )
+    val result = compile(dedupTest, annos)
+
+    values.zipWithIndex.foreach {
+      case (value, addr) =>
+        result should containLine(s"      m[$addr] = $value;")
+    }
+  }
+
+  "MemoryFileInlineAnnotation" should "emit $readmemh in deduped module" in {
+    val annos = Seq(
+      MemoryFileInlineAnnotation(child1MRef, filename = "text.hex"),
+      MemoryFileInlineAnnotation(child2MRef, filename = "text.hex")
+    )
+    val result = compile(dedupTest, annos)
+    result should containLine("""$readmemh("text.hex", """ + dedupedRef.name + """);""")
+  }
+
+  "MemoryFileInlineAnnotation" should "fail dedup if not all instances have the annotation" in {
+    val annos = Seq(
+      MemoryFileInlineAnnotation(child1MRef, filename = "text.hex")
+    )
+    assertThrows[FirrtlUserException] {
+      compile(dedupTest, annos)
+    }
+  }
+
+  "MemoryFileInlineAnnotation" should "fail dedup if instances have different init files" in {
+    val annos = Seq(
+      MemoryFileInlineAnnotation(child1MRef, filename = "text.hex"),
+      MemoryFileInlineAnnotation(child2MRef, filename = "text.bin")
+    )
+    assertThrows[FirrtlUserException] {
+      compile(dedupTest, annos)
+    }
+  }
 }
 
 abstract class MemInitExecutionSpec(values: Seq[Int], init: ReferenceTarget => Annotation)