Merge pull request #1291 from freechipsproject/infer-resets-last-connect-semantics

Infer resets last connect semantics

2 files changed, 119 insertions, 42 deletions

diff --git a/src/main/scala/firrtl/transforms/InferResets.scala b/src/main/scala/firrtl/transforms/InferResets.scala
index 70e2b76c..fbc915e2 100644
--- a/src/main/scala/firrtl/transforms/InferResets.scala
+++ b/src/main/scala/firrtl/transforms/InferResets.scala
@@ -36,7 +36,11 @@ object InferResets {
   private case class TypeDriver(tpe: Type, target: () => ReferenceTarget) extends ResetDriver {
     override def toString: String = s"TypeDriver(${tpe.serialize}, $target)"
   }
-
+  // When a [[ResetType]] is invalidated, we record the InvalidDrive
+  // If there are no types but invalid drivers, we default to BoolType
+  private case object InvalidDriver extends ResetDriver {
+    def defaultType: Type = Utils.BoolType
+  }
 
   // Type hierarchy representing the path to a leaf type in an aggregate type structure
   // Used by this [[InferResets]] to pinpoint instances of [[ResetType]] and their inferred type
@@ -84,7 +88,7 @@ class InferResets extends Transform {
 
   // Collect all drivers for circuit elements of type ResetType
   private def analyze(c: Circuit): Map[ReferenceTarget, List[ResetDriver]] = {
-    type DriverMap = mutable.HashMap[ReferenceTarget, mutable.ListBuffer[ResetDriver]]
+    type DriverMap = mutable.HashMap[ReferenceTarget, List[ResetDriver]]
     def onMod(mod: DefModule): DriverMap = {
       val instMap = mutable.Map[String, String]()
       // We need to convert submodule port targets into targets on the Module port itself
@@ -104,19 +108,18 @@ class InferResets extends Transform {
       def onStmt(map: DriverMap)(stmt: Statement): Unit = {
         // Mark driver of a ResetType leaf
         def markResetDriver(lhs: Expression, rhs: Expression): Unit = {
-          val lflip = Utils.to_flip(Utils.gender(lhs))
-          if ((lflip == Default && lhs.tpe == ResetType) ||
-              (lflip == Flip    && rhs.tpe == ResetType)) {
-            val (loc, exp) = lflip match {
-              case Default => (lhs, rhs)
-              case Flip    => (rhs, lhs)
-            }
-            val target = makeTarget(loc)
+          val con = Utils.flow(lhs) match {
+            case SinkFlow   if lhs.tpe == ResetType => Some((lhs, rhs))
+            case SourceFlow if rhs.tpe == ResetType => Some((rhs, lhs))
+            // If sink is not ResetType, do nothing
+            case _                                  => None
+          }
+          con.foreach { case (loc, exp) =>
             val driver = exp.tpe match {
               case ResetType => TargetDriver(makeTarget(exp))
               case tpe       => TypeDriver(tpe, () => makeTarget(exp))
             }
-            map.getOrElseUpdate(target, mutable.ListBuffer()) += driver
+            map(makeTarget(loc)) = driver :: Nil
           }
         }
         stmt match {
@@ -136,6 +139,16 @@ class InferResets extends Transform {
             for ((i, j) <- points) {
               markResetDriver(locs(i), exps(j))
             }
+          case IsInvalid(_, lhs) =>
+            val exprs = Utils.create_exps(lhs)
+            for (expr <- exprs) {
+              // Ignore leaves that are not of type ResetType
+              // Unlike in markResetDriver, flow is irrelevant for invalidation
+              if (expr.tpe == ResetType) {
+                val target = makeTarget(expr)
+                map(target) = InvalidDriver :: Nil
+              }
+            }
           case WDefInstance(_, inst, module, _) =>
             instMap += (inst -> module)
           case Conditionally(_, _, con, alt) =>
@@ -143,12 +156,12 @@ class InferResets extends Transform {
             val altMap = new DriverMap
             onStmt(conMap)(con)
             onStmt(altMap)(alt)
-            // Default to outerscope if not found in alt
+            // Default to outerscope if not found on either side
+            val conLookup = conMap.orElse(map).lift
             val altLookup = altMap.orElse(map).lift
             for (key <- conMap.keys ++ altMap.keys) {
-              val ds = map.getOrElseUpdate(key, mutable.ListBuffer())
-              conMap.get(key).foreach(ds ++= _)
-              altLookup(key).foreach(ds ++= _)
+              val values = conLookup(key).getOrElse(Nil) ++ altLookup(key).getOrElse(Nil)
+              map(key) = values
             }
           case other => other.foreach(onStmt(map))
         }
@@ -167,17 +180,22 @@ class InferResets extends Transform {
     val res = mutable.Map[ReferenceTarget, Type]()
     val errors = new Errors
     def rec(target: ReferenceTarget): Type = {
-      val drivers = map(target)
       res.getOrElseUpdate(target, {
-        val tpes = drivers.map {
-          case TargetDriver(t) => TypeDriver(rec(t), () => t)
-          case td: TypeDriver => td
+        val drivers = map.getOrElse(target, Nil)
+        val tpes = drivers.flatMap {
+          case TargetDriver(t) => Some(TypeDriver(rec(t), () => t))
+          case td: TypeDriver  => Some(td)
+          case InvalidDriver   => None
         }.groupBy(_.tpe)
-        if (tpes.keys.size != 1) {
-          // Multiple types of driver!
-          errors.append(DifferingDriverTypesException(target, tpes.toSeq))
+        tpes.keys.size match {
+          // This can occur if something of type Reset has no driver
+          case 0 => InvalidDriver.defaultType
+          case 1 => tpes.keys.head
+          case _ =>
+            // Multiple types of driver!
+            errors.append(DifferingDriverTypesException(target, tpes.toSeq))
+            tpes.keys.head
         }
-        tpes.keys.head
       })
     }
     for ((target, _) <- map) {
diff --git a/src/test/scala/firrtlTests/InferResetsSpec.scala b/src/test/scala/firrtlTests/InferResetsSpec.scala
index ac13033a..501dce20 100644
--- a/src/test/scala/firrtlTests/InferResetsSpec.scala
+++ b/src/test/scala/firrtlTests/InferResetsSpec.scala
@@ -4,7 +4,7 @@ package firrtlTests
 
 import firrtl._
 import firrtl.ir._
-import firrtl.passes.{CheckHighForm, CheckTypes}
+import firrtl.passes.{CheckHighForm, CheckTypes, CheckInitialization}
 import firrtl.transforms.InferResets
 import FirrtlCheckers._
 
@@ -37,7 +37,6 @@ class InferResetsSpec extends FirrtlFlatSpec {
       |    y <= asFixedPoint(r, 0)
       |    z <= asAsyncReset(r)""".stripMargin
     )
-    println(result.getEmittedCircuit)
     result should containLine ("wire r : UInt<1>")
     result should containLine ("r <= a")
     result should containLine ("v <= asUInt(r)")
@@ -125,44 +124,68 @@ class InferResetsSpec extends FirrtlFlatSpec {
     result should containTree { case Port(_, "buzz_bar_1_b", Input, AsyncResetType) => true }
   }
 
-  it should "NOT allow last connect semantics to pick the right type for Reset" in {
-    an [InferResets.DifferingDriverTypesException] shouldBe thrownBy {
+  it should "not crash if a ResetType has no drivers" in {
+    a [CheckInitialization.RefNotInitializedException] shouldBe thrownBy {
+      compile(s"""
+        |circuit test :
+        |  module test :
+        |    output out : Reset
+        |    wire w : Reset
+        |    out <= w
+        |    out <= UInt(1)
+        |""".stripMargin
+      )
+    }
+  }
+
+  it should "allow last connect semantics to pick the right type for Reset" in {
+    val result =
       compile(s"""
         |circuit top :
         |  module top :
         |    input reset0 : AsyncReset
         |    input reset1 : UInt<1>
         |    output out : Reset
+        |    wire w0 : Reset
         |    wire w1 : Reset
-        |    wire w2 : Reset
-        |    w1 <= reset0
-        |    w2 <= reset1
+        |    w0 <= reset0
+        |    w1 <= reset1
+        |    out <= w0
         |    out <= w1
-        |    out <= w2
         |""".stripMargin
       )
-    }
+    result should containTree { case DefWire(_, "w0", AsyncResetType) => true }
+    result should containTree { case DefWire(_, "w1", BoolType)       => true }
+    result should containTree { case Port(_, "out", Output, BoolType) => true }
   }
 
-  it should "NOT support last connect semantics across whens" in {
-    an [InferResets.DifferingDriverTypesException] shouldBe thrownBy {
+  it should "support last connect semantics across whens" in {
+    val result =
       compile(s"""
         |circuit top :
         |  module top :
         |    input reset0 : AsyncReset
-        |    input reset1 : UInt<1>
-        |    input en0 : UInt<1>
+        |    input reset1 : AsyncReset
+        |    input reset2 : UInt<1>
+        |    input en : UInt<1>
         |    output out : Reset
+        |    wire w0 : Reset
         |    wire w1 : Reset
         |    wire w2 : Reset
-        |    w1 <= reset0
-        |    w2 <= reset1
-        |    out <= w1
-        |    when en0 :
-        |      out <= w2
+        |    w0 <= reset0
+        |    w1 <= reset1
+        |    w2 <= reset2
+        |    out <= w2
+        |    when en :
+        |      out <= w0
+        |    else :
+        |      out <= w1
         |""".stripMargin
       )
-    }
+    result should containTree { case DefWire(_, "w0", AsyncResetType) => true }
+    result should containTree { case DefWire(_, "w1", AsyncResetType) => true }
+    result should containTree { case DefWire(_, "w2", BoolType)       => true }
+    result should containTree { case Port(_, "out", Output, AsyncResetType) => true }
   }
 
   it should "not allow different Reset Types to drive a single Reset" in {
@@ -186,6 +209,42 @@ class InferResetsSpec extends FirrtlFlatSpec {
     }
   }
 
+  it should "allow concrete reset types to overrule invalidation" in {
+    val result = compile(s"""
+      |circuit test :
+      |  module test :
+      |    input in : AsyncReset
+      |    output out : Reset
+      |    out is invalid
+      |    out <= in
+      |""".stripMargin)
+    result should containTree { case Port(_, "out", Output, AsyncResetType) => true }
+  }
+
+  it should "default to BoolType for Resets that are only invalidated" in {
+    val result = compile(s"""
+      |circuit test :
+      |  module test :
+      |    output out : Reset
+      |    out is invalid
+      |""".stripMargin)
+    result should containTree { case Port(_, "out", Output, BoolType) => true }
+  }
+
+  it should "not error if component of ResetType is invalidated and connected to an AsyncResetType" in {
+    val result = compile(s"""
+      |circuit test :
+      |  module test :
+      |    input cond : UInt<1>
+      |    input in : AsyncReset
+      |    output out : Reset
+      |    out is invalid
+      |    when cond :
+      |      out <= in
+      |""".stripMargin)
+    result should containTree { case Port(_, "out", Output, AsyncResetType) => true }
+  }
+
   it should "allow ResetType to drive AsyncResets or UInt<1>" in {
     val result1 = compile(s"""
       |circuit top :