Merge pull request #1388 from freechipsproject/allow-self-renames

Allow self renames
chipsalliance · Feb 13, 2020 · dd6bbd4 · dd6bbd4
2 parents eabc385 + 1909e21
commit dd6bbd4
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 14 deletions.
diff --git a/src/main/scala/firrtl/RenameMap.scala b/src/main/scala/firrtl/RenameMap.scala
@@ -33,12 +33,16 @@ object RenameMap {
   *
   * Transforms that modify names should return a [[RenameMap]] with the [[CircuitState]]
   * These are mutable datastructures for convenience
+  * @define noteSelfRename @note Self renames *will* be recorded
+  * @define noteDistinct @note Rename to/tos will be made distinct
   */
 // TODO This should probably be refactored into immutable and mutable versions
 final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, Seq[CompleteTarget]] = mutable.HashMap[CompleteTarget, Seq[CompleteTarget]](), val chained: Option[RenameMap] = None) {
 
   /** Chain a [[RenameMap]] with this [[RenameMap]]
     * @param next the map to chain with this map
+    * $noteSelfRename
+    * $noteDistinct
     */
   def andThen(next: RenameMap): RenameMap = {
     if (next.chained.isEmpty) {
@@ -52,27 +56,35 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S
     * [[firrtl.annotations.CircuitTarget CircuitTarget]]
     * @param from
     * @param to
+    * $noteSelfRename
+    * $noteDistinct
     */
   def record(from: CircuitTarget, to: CircuitTarget): Unit = completeRename(from, Seq(to))
 
   /** Record that the from [[firrtl.annotations.CircuitTarget CircuitTarget]] is renamed to another sequence of
     * [[firrtl.annotations.CircuitTarget CircuitTarget]]s
     * @param from
     * @param tos
+    * $noteSelfRename
+    * $noteDistinct
     */
   def record(from: CircuitTarget, tos: Seq[CircuitTarget]): Unit = completeRename(from, tos)
 
   /** Record that the from [[firrtl.annotations.IsMember Member]] is renamed to another [[firrtl.annotations.IsMember
     * IsMember]]
     * @param from
     * @param to
+    * $noteSelfRename
+    * $noteDistinct
     */
   def record(from: IsMember, to: IsMember): Unit = completeRename(from, Seq(to))
 
   /** Record that the from [[firrtl.annotations.IsMember IsMember]] is renamed to another sequence of
     * [[firrtl.annotations.IsMember IsMember]]s
     * @param from
     * @param tos
+    * $noteSelfRename
+    * $noteDistinct
     */
   def record(from: IsMember, tos: Seq[IsMember]): Unit = completeRename(from, tos)
 
@@ -81,6 +93,8 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S
     * and ([[firrtl.annotations.IsMember IsMember]] -> Seq[ [[firrtl.annotations.IsMember IsMember]] ]) key/value
     * allowed
     * @param map
+    * $noteSelfRename
+    * $noteDistinct
     */
   def recordAll(map: collection.Map[CompleteTarget, Seq[CompleteTarget]]): Unit =
     map.foreach{
@@ -479,24 +493,20 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S
     }
   }
 
-  /** Fully renames from to tos
+  /** Fully rename `from` to `tos`
     * @param from
     * @param tos
     */
   private def completeRename(from: CompleteTarget, tos: Seq[CompleteTarget]): Unit = {
-    (from, tos) match {
-      case (x, Seq(y)) if x == y =>
-      case _ =>
-        tos.foreach{recordSensitivity(from, _)}
-        val existing = underlying.getOrElse(from, Vector.empty)
-        val updated = existing ++ tos
-        underlying(from) = updated
-        getCache.clear()
-        traverseTokensCache.clear()
-        traverseHierarchyCache.clear()
-        traverseLeftCache.clear()
-        traverseRightCache.clear()
-    }
+    tos.foreach{recordSensitivity(from, _)}
+    val existing = underlying.getOrElse(from, Vector.empty)
+    val updated = (existing ++ tos).distinct
+    underlying(from) = updated
+    getCache.clear()
+    traverseTokensCache.clear()
+    traverseHierarchyCache.clear()
+    traverseLeftCache.clear()
+    traverseRightCache.clear()
   }
 
   /* DEPRECATED ACCESSOR/SETTOR METHODS WITH [[firrtl.ir.Named Named]] */

diff --git a/src/test/scala/firrtlTests/RenameMapSpec.scala b/src/test/scala/firrtlTests/RenameMapSpec.scala
@@ -752,4 +752,32 @@ class RenameMapSpec extends FirrtlFlatSpec {
       Some(Seq(bar2))
     }
   }
+
+  it should "record a self-rename" in {
+    val top = CircuitTarget("Top").module("Top")
+    val foo = top.instOf("foo", "Mod")
+    val bar = top.instOf("bar", "Mod")
+
+    val r = RenameMap()
+
+    r.record(foo, bar)
+    r.record(foo, foo)
+
+    r.get(foo) should not be (empty)
+    r.get(foo).get should contain allOf (foo, bar)
+  }
+
+  it should "not record the same rename multiple times" in {
+    val top = CircuitTarget("Top").module("Top")
+    val foo = top.instOf("foo", "Mod")
+    val bar = top.instOf("bar", "Mod")
+
+    val r = RenameMap()
+
+    r.record(foo, bar)
+    r.record(foo, bar)
+
+    r.get(foo) should not be (empty)
+    r.get(foo).get should contain theSameElementsAs Seq(bar)
+  }
 }