Skip to content
This repository has been archived by the owner on Apr 20, 2024. It is now read-only.

Run ConvertToExtMod before DedupModules #119

Open
tymcauley opened this issue Feb 26, 2022 · 1 comment · May be fixed by #120
Open

Run ConvertToExtMod before DedupModules #119

tymcauley opened this issue Feb 26, 2022 · 1 comment · May be fixed by #120

Comments

@tymcauley
Copy link
Contributor

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 pushed a commit to tymcauley/barstools that referenced this issue Feb 26, 2022
Normally, `DedupModules` spends a lot of time working on modules that
this pass removes. Having this run before `DedupModules` significantly
speeds up FIRRTL compile time for the Harness block in Chipyard.

Resolves ucb-bar#119
@tymcauley tymcauley linked a pull request Feb 26, 2022 that will close this issue
@tymcauley
Copy link
Contributor Author

Added this PR for experimenting with the fix: #120

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant