Add missing invalidations to some transforms (#1541)

This adds missing invalidations to four transforms:

  - ExpandConnects
  - RemoveAccesses
  - SplitExpressions
  - VerilogMemDelays

This necessarily updates test cases which expect exact transform
orders to reflect the new order.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

6 files changed, 39 insertions, 15 deletions

diff --git a/src/main/scala/firrtl/passes/ExpandConnects.scala b/src/main/scala/firrtl/passes/ExpandConnects.scala
index f80c705c..d28e6399 100644
--- a/src/main/scala/firrtl/passes/ExpandConnects.scala
+++ b/src/main/scala/firrtl/passes/ExpandConnects.scala
@@ -2,16 +2,21 @@ package firrtl.passes
 
 import firrtl.Utils.{create_exps, flow, get_field, get_valid_points, times, to_flip, to_flow}
 import firrtl.ir._
-import firrtl.options.{PreservesAll, Dependency}
+import firrtl.options.Dependency
 import firrtl.{DuplexFlow, Flow, SinkFlow, SourceFlow, Transform, WDefInstance, WRef, WSubAccess, WSubField, WSubIndex}
 import firrtl.Mappers._
 
-object ExpandConnects extends Pass with PreservesAll[Transform] {
+object ExpandConnects extends Pass {
 
   override def prerequisites =
     Seq( Dependency(PullMuxes),
          Dependency(ReplaceAccesses) ) ++ firrtl.stage.Forms.Deduped
 
+  override def invalidates(a: Transform) = a match {
+    case ResolveFlows => true
+    case _            => false
+  }
+
   def run(c: Circuit): Circuit = {
     def expand_connects(m: Module): Module = {
       val flows = collection.mutable.LinkedHashMap[String,Flow]()
diff --git a/src/main/scala/firrtl/passes/RemoveAccesses.scala b/src/main/scala/firrtl/passes/RemoveAccesses.scala
index 176312d5..d5615260 100644
--- a/src/main/scala/firrtl/passes/RemoveAccesses.scala
+++ b/src/main/scala/firrtl/passes/RemoveAccesses.scala
@@ -23,8 +23,8 @@ object RemoveAccesses extends Pass {
          Dependency(ExpandConnects) ) ++ firrtl.stage.Forms.Deduped
 
   override def invalidates(a: Transform): Boolean = a match {
-    case Uniquify => true
-    case _        => false
+    case Uniquify | ResolveKinds | ResolveFlows => true
+    case _                                      => false
   }
 
   private def AND(e1: Expression, e2: Expression) =
diff --git a/src/main/scala/firrtl/passes/SplitExpressions.scala b/src/main/scala/firrtl/passes/SplitExpressions.scala
index 7124111b..c536cd5d 100644
--- a/src/main/scala/firrtl/passes/SplitExpressions.scala
+++ b/src/main/scala/firrtl/passes/SplitExpressions.scala
@@ -3,9 +3,9 @@
 package firrtl
 package passes
 
-import firrtl.{SystemVerilogEmitter, VerilogEmitter}
+import firrtl.{SystemVerilogEmitter, Transform, VerilogEmitter}
 import firrtl.ir._
-import firrtl.options.{Dependency, PreservesAll}
+import firrtl.options.Dependency
 import firrtl.Mappers._
 import firrtl.Utils.{kind, flow, get_info}
 
@@ -14,7 +14,7 @@ import scala.collection.mutable
 
 // Splits compound expressions into simple expressions
 //  and named intermediate nodes
-object SplitExpressions extends Pass with PreservesAll[Transform] {
+object SplitExpressions extends Pass {
 
   override def prerequisites = firrtl.stage.Forms.LowForm ++
     Seq( Dependency(firrtl.passes.RemoveValidIf),
@@ -24,6 +24,11 @@ object SplitExpressions extends Pass with PreservesAll[Transform] {
     Seq( Dependency[SystemVerilogEmitter],
          Dependency[VerilogEmitter] )
 
+  override def invalidates(a: Transform) = a match {
+    case ResolveKinds => true
+    case _            => false
+  }
+
    private def onModule(m: Module): Module = {
       val namespace = Namespace(m)
       def onStmt(s: Statement): Statement = {
diff --git a/src/main/scala/firrtl/passes/memlib/VerilogMemDelays.scala b/src/main/scala/firrtl/passes/memlib/VerilogMemDelays.scala
index 131a198b..dd644323 100644
--- a/src/main/scala/firrtl/passes/memlib/VerilogMemDelays.scala
+++ b/src/main/scala/firrtl/passes/memlib/VerilogMemDelays.scala
@@ -175,7 +175,7 @@ object VerilogMemDelays extends Pass {
          Dependency[SystemVerilogEmitter] )
 
   override def invalidates(a: Transform): Boolean = a match {
-    case _: transforms.ConstantPropagation => true
+    case _: transforms.ConstantPropagation | ResolveFlows => true
     case _ => false
   }
 
diff --git a/src/test/scala/firrtlTests/CustomTransformSpec.scala b/src/test/scala/firrtlTests/CustomTransformSpec.scala
index f1b2045e..ef1dc86d 100644
--- a/src/test/scala/firrtlTests/CustomTransformSpec.scala
+++ b/src/test/scala/firrtlTests/CustomTransformSpec.scala
@@ -167,10 +167,15 @@ class CustomTransformSpec extends FirrtlFlatSpec {
         .containsSlice(expectedSlice) should be (true)
     }
 
-    Seq( (Dependency[LowFirrtlEmitter],      Seq(Forms.LowForm.last)                 ),
-         (Dependency[MinimumVerilogEmitter], Seq(Forms.LowFormMinimumOptimized.last) ),
-         (Dependency[VerilogEmitter],        Seq(Forms.LowFormOptimized.last)        ),
-         (Dependency[SystemVerilogEmitter],  Seq(Forms.LowFormOptimized.last)        )
+    val Seq(low, lowMinOpt, lowOpt) =
+      Seq(Forms.LowForm, Forms.LowFormMinimumOptimized, Forms.LowFormOptimized)
+        .map(target => new firrtl.stage.transforms.Compiler(target))
+        .map(_.flattenedTransformOrder.map(Dependency.fromTransform(_)))
+
+    Seq( (Dependency[LowFirrtlEmitter],      Seq(low.last)      ),
+         (Dependency[MinimumVerilogEmitter], Seq(lowMinOpt.last)),
+         (Dependency[VerilogEmitter],        Seq(lowOpt.last)    ),
+         (Dependency[SystemVerilogEmitter],  Seq(lowOpt.last)   )
     ).foreach((testOrder _).tupled)
   }
 
diff --git a/src/test/scala/firrtlTests/LoweringCompilersSpec.scala b/src/test/scala/firrtlTests/LoweringCompilersSpec.scala
index ea591450..4a7a1700 100644
--- a/src/test/scala/firrtlTests/LoweringCompilersSpec.scala
+++ b/src/test/scala/firrtlTests/LoweringCompilersSpec.scala
@@ -158,8 +158,11 @@ class LoweringCompilersSpec extends FlatSpec with Matchers {
   it should "replicate the old order" in {
     val tm = new TransformManager(Forms.MidForm, Forms.Deduped)
     val patches = Seq(
+      Add(4, Seq(Dependency(firrtl.passes.ResolveFlows))),
+      Add(5, Seq(Dependency(firrtl.passes.ResolveKinds))),
       Add(6, Seq(Dependency(firrtl.passes.ResolveKinds),
-                 Dependency(firrtl.passes.InferTypes))),
+                 Dependency(firrtl.passes.InferTypes),
+                 Dependency(firrtl.passes.ResolveFlows))),
       Del(7),
       Del(8),
       Add(7, Seq(Dependency[firrtl.passes.ExpandWhensAndCheck])),
@@ -184,7 +187,11 @@ class LoweringCompilersSpec extends FlatSpec with Matchers {
 
   it should "replicate the old order" in {
     val tm = new TransformManager(Forms.LowFormMinimumOptimized, Forms.LowForm)
-    compare(legacyTransforms(new MinimumLowFirrtlOptimization), tm)
+    val patches = Seq(
+      Add(4, Seq(Dependency(firrtl.passes.ResolveFlows))),
+      Add(5, Seq(Dependency(firrtl.passes.ResolveKinds)))
+    )
+    compare(legacyTransforms(new MinimumLowFirrtlOptimization), tm, patches)
   }
 
   behavior of "LowFirrtlOptimization"
@@ -192,7 +199,9 @@ class LoweringCompilersSpec extends FlatSpec with Matchers {
   it should "replicate the old order" in {
     val tm = new TransformManager(Forms.LowFormOptimized, Forms.LowForm)
     val patches = Seq(
-      Add(7, Seq(Dependency(firrtl.passes.Legalize)))
+      Add(6, Seq(Dependency(firrtl.passes.ResolveFlows))),
+      Add(7, Seq(Dependency(firrtl.passes.Legalize))),
+      Add(8, Seq(Dependency(firrtl.passes.ResolveKinds)))
     )
     compare(legacyTransforms(new LowFirrtlOptimization), tm, patches)
   }