Remove last connect semantics from reset inference (#1396)

* Revert "Infer resets last connect semantics (#1291)"
* Fix handling of invalidated and undriven components of type Reset
* Run CheckTypes after InferResets
* Make reset inference bidirectional on connect
* Support AsyncResetType in RemoveValidIf
* Fix InferResets for parent constraints on child ports
* Apply suggestions from code review
* Add ScalaDoc to InferResets

Co-authored-by: Albert Magyar <albert.magyar@gmail.com>
Co-authored-by: Schuyler Eldridge <schuyler.eldridge@gmail.com>

1 files changed, 112 insertions, 14 deletions

diff --git a/src/test/scala/firrtlTests/InferResetsSpec.scala b/src/test/scala/firrtlTests/InferResetsSpec.scala
index 501dce20..0bcc459c 100644
--- a/src/test/scala/firrtlTests/InferResetsSpec.scala
+++ b/src/test/scala/firrtlTests/InferResetsSpec.scala
@@ -5,7 +5,7 @@ package firrtlTests
 import firrtl._
 import firrtl.ir._
 import firrtl.passes.{CheckHighForm, CheckTypes, CheckInitialization}
-import firrtl.transforms.InferResets
+import firrtl.transforms.{CheckCombLoops, InferResets}
 import FirrtlCheckers._
 
 // TODO
@@ -138,8 +138,8 @@ class InferResetsSpec extends FirrtlFlatSpec {
     }
   }
 
-  it should "allow last connect semantics to pick the right type for Reset" in {
-    val result =
+  it should "NOT allow last connect semantics to pick the right type for Reset" in {
+    an [InferResets.InferResetsException] shouldBe thrownBy {
       compile(s"""
         |circuit top :
         |  module top :
@@ -154,13 +154,11 @@ class InferResetsSpec extends FirrtlFlatSpec {
         |    out <= w1
         |""".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 "support last connect semantics across whens" in {
-    val result =
+  it should "NOT support last connect semantics across whens" in {
+    an [InferResets.InferResetsException] shouldBe thrownBy {
       compile(s"""
         |circuit top :
         |  module top :
@@ -182,14 +180,11 @@ class InferResetsSpec extends FirrtlFlatSpec {
         |      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 {
-    an [InferResets.DifferingDriverTypesException] shouldBe thrownBy {
+    an [InferResets.InferResetsException] shouldBe thrownBy {
       val result = compile(s"""
         |circuit top :
         |  module top :
@@ -410,5 +405,108 @@ class InferResetsSpec extends FirrtlFlatSpec {
     result should containTree { case Port(_, "childReset", Input, BoolType) => true }
     result should containTree { case Port(_, "childReset", Input, AsyncResetType) => true }
   }
-}
 
+  it should "infer based on what a component *drives* not just what drives it" in {
+    val result = compile(s"""
+      |circuit top :
+      |  module top :
+      |    input in : AsyncReset
+      |    output out : Reset
+      |    wire w : Reset
+      |    w is invalid
+      |    out <= w
+      |    out <= in
+      |""".stripMargin)
+    result should containTree { case DefWire(_, "w", AsyncResetType) => true }
+  }
+
+  it should "infer from connections, ignoring the fact that the invalidation wins" in {
+    val result = compile(s"""
+      |circuit top :
+      |  module top :
+      |    input in : AsyncReset
+      |    output out : Reset
+      |    out <= in
+      |    out is invalid
+      |""".stripMargin)
+    result should containTree { case Port(_, "out", Output, AsyncResetType) => true }
+  }
+
+  // The backwards type propagation constrains `w` to be the same as both `out0` and `out1`
+  it should "not allow an invalidated Wire to drive both a UInt<1> and an AsyncReset" in {
+    an [InferResets.InferResetsException] shouldBe thrownBy {
+      val result = compile(s"""
+        |circuit top :
+        |  module top :
+        |    input in0 : AsyncReset
+        |    input in1 : UInt<1>
+        |    output out0 : Reset
+        |    output out1 : Reset
+        |    wire w : Reset
+        |    w is invalid
+        |    out0 <= w
+        |    out1 <= w
+        |    out0 <= in0
+        |    out1 <= in1
+        |""".stripMargin
+      )
+    }
+  }
+
+  it should "not propagate type info from downstream across a cast" in {
+    val result = compile(s"""
+        |circuit top :
+        |  module top :
+        |    input in0 : AsyncReset
+        |    input in1 : UInt<1>
+        |    output out0 : Reset
+        |    output out1 : Reset
+        |    wire w : Reset
+        |    w is invalid
+        |    out0 <= asAsyncReset(w)
+        |    out1 <= w
+        |    out0 <= in0
+        |    out1 <= in1
+        |""".stripMargin
+    )
+    result should containTree { case Port(_, "out0", Output, AsyncResetType) => true }
+  }
+
+  // This tests for a bug unrelated to support or lackthereof for last connect in inference
+  it should "take into account both internal and external constraints on Module port types" in {
+    val result = compile(s"""
+      |circuit top :
+      |  module child :
+      |    input i : AsyncReset
+      |    output o : Reset
+      |    o <= i
+      |  module top :
+      |    input in : AsyncReset
+      |    output out : AsyncReset
+      |    inst c of child
+      |    c.o is invalid
+      |    c.i <= in
+      |    out <= c.o
+      |""".stripMargin)
+    result should containTree { case Port(_, "o", Output, AsyncResetType) => true }
+  }
+
+  it should "not crash on combinational loops" in {
+    a [CheckCombLoops.CombLoopException] shouldBe thrownBy {
+      val result = compile(s"""
+        |circuit top :
+        |  module top :
+        |    input in : AsyncReset
+        |    output out : Reset
+        |    wire w0 : Reset
+        |    wire w1 : Reset
+        |    w0 <= in
+        |    w0 <= w1
+        |    w1 <= w0
+        |    out <= in
+        |""".stripMargin,
+        compiler = new LowFirrtlCompiler
+      )
+    }
+  }
+}