From 23db44a44b10aaa6f34f75732266178e6c714aba Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 23 Apr 2020 22:33:14 -0400 Subject: [PATCH 1/3] Expose ChiselStage's PhaseManager, rm extra wraps Signed-off-by: Schuyler Eldridge --- src/main/scala/chisel3/stage/ChiselStage.scala | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/scala/chisel3/stage/ChiselStage.scala b/src/main/scala/chisel3/stage/ChiselStage.scala index 0068d86feaf..e4ff77d08f3 100644 --- a/src/main/scala/chisel3/stage/ChiselStage.scala +++ b/src/main/scala/chisel3/stage/ChiselStage.scala @@ -28,11 +28,12 @@ class ChiselStage extends Stage with PreservesAll[Phase] { Dependency[chisel3.stage.phases.Convert], Dependency[chisel3.stage.phases.MaybeFirrtlStage] ) + final lazy val phaseManager = new PhaseManager(targets) { + override val wrappers = Seq( (a: Phase) => DeletedWrapper(a) ) + } + def run(annotations: AnnotationSeq): AnnotationSeq = try { - new PhaseManager(targets) { override val wrappers = Seq( (a: Phase) => DeletedWrapper(a) ) } - .transformOrder - .map(firrtl.options.phases.DeletedWrapper(_)) - .foldLeft(annotations)( (a, f) => f.transform(a) ) + phaseManager.transform(annotations) } catch { case ce: ChiselException => val stackTrace = if (!view[ChiselOptions](annotations).printFullStackTrace) { From bfb5b3542948246a0941cdc53f48acceddfeda03 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Thu, 23 Apr 2020 22:07:23 -0400 Subject: [PATCH 2/3] Test that Elaborate only runs once Signed-off-by: Schuyler Eldridge --- .../scala/chiselTests/stage/ChiselStageSpec.scala | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/test/scala/chiselTests/stage/ChiselStageSpec.scala b/src/test/scala/chiselTests/stage/ChiselStageSpec.scala index a7a405f11dc..21beb48f35d 100644 --- a/src/test/scala/chiselTests/stage/ChiselStageSpec.scala +++ b/src/test/scala/chiselTests/stage/ChiselStageSpec.scala @@ -8,6 +8,8 @@ import chisel3.stage.ChiselStage import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers +import firrtl.options.Dependency + object ChiselStageSpec { class Foo extends MultiIOModule { @@ -57,4 +59,15 @@ class ChiselStageSpec extends AnyFlatSpec with Matchers { ChiselStage.convert(new Foo) } + behavior of "ChiselStage phase ordering" + + it should "only run elaboration once" in new ChiselStageFixture { + info("Phase order is:\n" + stage.phaseManager.prettyPrint(" ")) + + val order = stage.phaseManager.flattenedTransformOrder.map(Dependency.fromTransform) + + info("Elaborate only runs once") + exactly (1, order) should be (Dependency[chisel3.stage.phases.Elaborate]) + } + } From 5e2ca44a31969291d2ad27869f7b443ce46a8654 Mon Sep 17 00:00:00 2001 From: Schuyler Eldridge Date: Mon, 4 May 2020 17:54:24 -0400 Subject: [PATCH 3/3] Fix double elaboration Remove the requirement that FirrtlStage runs elaboration (this should be implicit) and remove the unneeded invalidation of elaboration by the Emitter. Due to Convert currently NOT invalidating Elaborate (when it should), add an optionalPrerequisiteOf to ensure that the Emitter runs before the Convert phase. Co-authored-by: David Biancolin Co-authored-by: Schuyler Eldridge Signed-off-by: Schuyler Eldridge --- src/main/scala/chisel3/stage/ChiselStage.scala | 1 - src/main/scala/chisel3/stage/phases/Emitter.scala | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/scala/chisel3/stage/ChiselStage.scala b/src/main/scala/chisel3/stage/ChiselStage.scala index e4ff77d08f3..2dbb5b9d96c 100644 --- a/src/main/scala/chisel3/stage/ChiselStage.scala +++ b/src/main/scala/chisel3/stage/ChiselStage.scala @@ -20,7 +20,6 @@ class ChiselStage extends Stage with PreservesAll[Phase] { val targets: Seq[Dependency[Phase]] = Seq( Dependency[chisel3.stage.phases.Checks], - Dependency[chisel3.stage.phases.Elaborate], Dependency[chisel3.stage.phases.AddImplicitOutputFile], Dependency[chisel3.stage.phases.AddImplicitOutputAnnotationFile], Dependency[chisel3.stage.phases.MaybeAspectPhase], diff --git a/src/main/scala/chisel3/stage/phases/Emitter.scala b/src/main/scala/chisel3/stage/phases/Emitter.scala index 31e21542998..7fb9ef919e3 100644 --- a/src/main/scala/chisel3/stage/phases/Emitter.scala +++ b/src/main/scala/chisel3/stage/phases/Emitter.scala @@ -30,10 +30,9 @@ class Emitter extends Phase { Dependency[AddImplicitOutputAnnotationFile], Dependency[MaybeAspectPhase] ) - override def invalidates(phase: Phase): Boolean = phase match { - case _: Elaborate => true - case _ => false - } + override def optionalPrerequisiteOf = Seq(Dependency[Convert]) + + override def invalidates(phase: Phase): Boolean = false def transform(annotations: AnnotationSeq): AnnotationSeq = { val copts = view[ChiselOptions](annotations)