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

Commit

Permalink
Prevent extra re-lowerings via "actual targets"
Browse files Browse the repository at this point in the history
Add a member to DependencyManager that returns the "actual targets" of
what this DependencyManager solves. This exposes an API to query not
just the targets, but the actual state that results from running the
transform. The actual targets are a superset of the targets that can
prevent unnecessary re-lowering.

Consider the situation of a dependency manager that has one transform
target, foo, and no current state. The flattened transform order that
results pulls in hundreds of other transforms that foo requires.
However, from the perspective of a user looking at the dependency
manager, it's "effect" is only running foo.

This matters when using a DependencyManager to solve a sub-problem. If
the sub-problem is too narrow, e.g., one transform foo, then the
hundreds of other transforms that may be run will be repeatedly run
over and over again because there's no API to query the effect of
solving a (targets, currentState) tuple.

This commit includes a small test that shows this issue.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
  • Loading branch information
seldridge committed Jul 29, 2020
1 parent 0de5a8d commit e49a172
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 2 deletions.
26 changes: 24 additions & 2 deletions src/main/scala/firrtl/options/DependencyManager.scala
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ trait DependencyManager[A, B <: TransformLike[A] with DependencyAPI[B]] extends
*/
def wrappers: Seq[(B) => B] = Seq.empty

/** The actual [[firrtl.options.TransformLike TransformLike]]s that will be run. This is a superset of the requested
* targets.
*/
lazy val actualTargets: Seq[Dependency[B]] =
flattenedTransformOrder
.foldLeft(_currentState.toSet) {
case (acc, tx) => acc.filterNot(a => tx.invalidates(a.getObject)) + Dependency.fromTransform(tx)
}.toSeq

/** Store of conversions between classes and objects. Objects that do not exist in the map will be lazily constructed.
*/
protected lazy val dependencyToObject: LinkedHashMap[Dependency[B], B] = {
Expand Down Expand Up @@ -224,8 +233,21 @@ trait DependencyManager[A, B <: TransformLike[A] with DependencyAPI[B]] extends
if ((prereqs -- state).nonEmpty) { Some(this.copy(prereqs.toSeq, state.toSeq)) }
else { None }
}
/* "in" is added *after* invalidation because a transform my not invalidate itself! */
((state ++ prereqs).map(dToO).filterNot(in.invalidates).map(oToD) + in, out ++ preprocessing :+ in)
/* The new state is the current state, any prerequisites, and the effect of solving a preprocessing sub-problem. The
* sub-problem uses "actual targets" as opposed to "targets" because the act of solving the sub-problem may result
* in more work than just the "targets" being run.
*/
val statex: LinkedHashSet[Dependency[B]] = state ++ prereqs ++ (
preprocessing match {
case Some(a: DependencyManager[A, B]) => a.actualTargets
case _ => Seq.empty
}
)
/* The effect of the next transform, in, is then applied:
* 1) invalidations are applied
* 2) the next transform is added to the state
*/
(statex.map(dToO).filterNot(in.invalidates).map(oToD) + in, out ++ preprocessing :+ in)
}
val postprocessing: Option[B] = {
if ((_targets -- s).nonEmpty) { Some(this.copy(_targets.toSeq, s.toSeq)) }
Expand Down
43 changes: 43 additions & 0 deletions src/test/scala/firrtlTests/options/PhaseManagerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,38 @@ object OrderingFixture {

}

object ActualTargetsFixture {

class A extends IdentityPhase {
override def invalidates(a: Phase) = false
}

class B extends IdentityPhase {
override def prerequisites = Seq(Dependency[A])
override def invalidates(a: Phase) = false
}

class C extends IdentityPhase {
override def prerequisites = Seq(Dependency[A], Dependency[B])
override def optionalPrerequisiteOf = Seq(Dependency[D])
override def invalidates(a: Phase) = a match {
case _: A | _: B => true
case _ => false
}
}

class D extends IdentityPhase {
override def prerequisites = Seq(Dependency[B])
override def invalidates(a: Phase) = false
}

class E extends IdentityPhase {
override def prerequisites = Seq(Dependency[A], Dependency[B], Dependency[D])
override def invalidates(a: Phase) = false
}

}

class PhaseManagerSpec extends AnyFlatSpec with Matchers {

def writeGraphviz(pm: PhaseManager, dir: String): Unit = {
Expand Down Expand Up @@ -688,4 +720,15 @@ class PhaseManagerSpec extends AnyFlatSpec with Matchers {
}
}

it should "use actual targets to avoid unnecessary re-lowering" in {
val f = ActualTargetsFixture

val targets = Seq(Dependency[f.A], Dependency[f.B], Dependency[f.C], Dependency[f.D], Dependency[f.E])
val order = Seq(classOf[f.A], classOf[f.B], classOf[f.C], classOf[f.A], classOf[f.B], classOf[f.D], classOf[f.E])
val foo = new PhaseManager(targets)
info(foo.prettyPrint())
(new PhaseManager(targets)).flattenedTransformOrder.map(_.getClass) should be (order)

}

}

0 comments on commit e49a172

Please sign in to comment.