barstools icon indicating copy to clipboard operation
barstools copied to clipboard

Run ConvertToExtMod before DedupModules

Open tymcauley opened this issue 3 years ago • 1 comments

I've noticed that a few FIRRTL transforms take a good amount of time to run (depending on the design, of course). One of the most expensive ones is consistently DedupModules, which runs before High FIRRTL is emitted.

Now, since GenerateTopAndHarness run the FIRRTL compiler twice (once for the top, once for the harness), I was hoping that the harness run would be much quicker, since there's not a lot of complexity to the harness circuit compared to most top designs. However, it looks like DedupModules takes about the same amount of time for both runs. It appears that the root cause of this is that ConvertToExtMod is running after DedupModules, rather than before. Thus, all of the top circuit components are still being de-duplicated, even though they won't be emitted by the harness invocation of the FIRRTL compiler.

Looking at the pre-reqs for ConvertToExtMod, it takes in High FIRRTL, which means this will run after DedupModules. Is there any reason we can't move ConvertToExtMod earlier in the transform order? Is there any feature of High FIRRTL that it depends on? I'm happy to submit a PR once we come to a solution.

FWIW, I ran a quick experiment with this change:

diff --git a/src/main/scala/barstools/tapeout/transforms/ConvertToExtModPass.scala b/src/main/scala/barstools/tapeout/transforms/ConvertToExtModPass.scala
index a81937a..9ceb27c 100644
--- a/src/main/scala/barstools/tapeout/transforms/ConvertToExtModPass.scala
+++ b/src/main/scala/barstools/tapeout/transforms/ConvertToExtModPass.scala
@@ -19,10 +19,10 @@ case class ConvertToExtModAnnotation(target: ModuleTarget) extends SingleTargetA
 // otherwise it's left alone.
 class ConvertToExtMod extends Transform with DependencyAPIMigration {

-  override def prerequisites:         Seq[TransformDependency] = Forms.HighForm
+  override def prerequisites:         Seq[TransformDependency] = Seq.empty
   override def optionalPrerequisites: Seq[TransformDependency] = Seq.empty
   override def optionalPrerequisiteOf: Seq[TransformDependency] = {
-    Forms.HighEmitters ++ Seq(Dependency[RemoveUnusedModules], Dependency[ReplSeqMem])
+    Forms.HighEmitters ++ Seq(Dependency[RemoveUnusedModules], Dependency[ReplSeqMem], Dependency[firrtl.transforms.DedupModules])
   }
   override def invalidates(a: Transform): Boolean = false

This drastically sped up DedupModules for me.

tymcauley avatar Feb 26 '22 00:02 tymcauley

Added this PR for experimenting with the fix: https://github.com/ucb-bar/barstools/pull/120

tymcauley avatar Feb 26 '22 19:02 tymcauley