From 6426b4afe6ef57797340b235c774baec05862869 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 12 Feb 2020 16:19:54 -0500 Subject: Record self-renames in RenameMap, distinct renames Change the behavior of RenameMap.completeTarget so that self-renames do not silently *not* happen. Previously, requests to self-rename would be ignored unless they were packaged in a sequences of renames that included a self-rename. Change renames to be recorded distinctly so that multiple requests to rename to the same thing will now deduplicate. Previously, these renames would be recorded multiple times. This change was required because allowing self-renames exposed a bug in InferWidthsAnnosSpec due to multiple renames. These changes benefit the situation where you rightly want to do a self-rename. Namely, when doing module duplication (with the EliminateTargetPaths transform). Signed-off-by: Schuyler Eldridge Do not record the same rename multiple times Signed-off-by: Schuyler Eldridge --- src/main/scala/firrtl/RenameMap.scala | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/main/scala/firrtl/RenameMap.scala b/src/main/scala/firrtl/RenameMap.scala index 03f01991..ffb7acc2 100644 --- a/src/main/scala/firrtl/RenameMap.scala +++ b/src/main/scala/firrtl/RenameMap.scala @@ -484,19 +484,15 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S * @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]] */ -- cgit v1.2.3 From 94c84eb1708233454cde7a752483fa1f87513ccc Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 12 Feb 2020 17:19:04 -0500 Subject: Update RenameMap Scaladoc for self-rename, distinc Signed-off-by: Schuyler Eldridge --- src/main/scala/firrtl/RenameMap.scala | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/main/scala/firrtl/RenameMap.scala b/src/main/scala/firrtl/RenameMap.scala index ffb7acc2..bdcb785a 100644 --- 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,6 +56,8 @@ 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)) @@ -59,6 +65,8 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S * [[firrtl.annotations.CircuitTarget CircuitTarget]]s * @param from * @param tos + * $noteSelfRename + * $noteDistinct */ def record(from: CircuitTarget, tos: Seq[CircuitTarget]): Unit = completeRename(from, tos) @@ -66,6 +74,8 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S * IsMember]] * @param from * @param to + * $noteSelfRename + * $noteDistinct */ def record(from: IsMember, to: IsMember): Unit = completeRename(from, Seq(to)) @@ -73,6 +83,8 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S * [[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,7 +493,7 @@ final class RenameMap private (val underlying: mutable.HashMap[CompleteTarget, S } } - /** Fully renames from to tos + /** Fully rename `from` to `tos` * @param from * @param tos */ -- cgit v1.2.3 From 00e736fb1dffd7fa1cd9986dbfb3dcdb4b273fbc Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 12 Feb 2020 16:52:35 -0500 Subject: Add test of RenameMap self-renaming Signed-off-by: Schuyler Eldridge --- src/test/scala/firrtlTests/RenameMapSpec.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'src') diff --git a/src/test/scala/firrtlTests/RenameMapSpec.scala b/src/test/scala/firrtlTests/RenameMapSpec.scala index 2da10b7f..bbe0255f 100644 --- a/src/test/scala/firrtlTests/RenameMapSpec.scala +++ b/src/test/scala/firrtlTests/RenameMapSpec.scala @@ -752,4 +752,18 @@ 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) + } } -- cgit v1.2.3 From 1909e216b8e7748d97ac34d91b18ec0f54fde46a Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Wed, 12 Feb 2020 16:52:53 -0500 Subject: Add test of RenameMap not recording same rename Signed-off-by: Schuyler Eldridge --- src/test/scala/firrtlTests/RenameMapSpec.scala | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'src') diff --git a/src/test/scala/firrtlTests/RenameMapSpec.scala b/src/test/scala/firrtlTests/RenameMapSpec.scala index bbe0255f..dc091b0a 100644 --- a/src/test/scala/firrtlTests/RenameMapSpec.scala +++ b/src/test/scala/firrtlTests/RenameMapSpec.scala @@ -766,4 +766,18 @@ class RenameMapSpec extends FirrtlFlatSpec { 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) + } } -- cgit v1.2.3