Replace unconnected registers with 0 in Constant Propagation (#776)

Moved from RemoveValidIf

Also Make RemoveValidIf.getGroundZero public and support Fixed

3 files changed, 59 insertions, 15 deletions

diff --git a/src/main/scala/firrtl/passes/RemoveValidIf.scala b/src/main/scala/firrtl/passes/RemoveValidIf.scala
index 7d714be7..5338a911 100644
--- a/src/main/scala/firrtl/passes/RemoveValidIf.scala
+++ b/src/main/scala/firrtl/passes/RemoveValidIf.scala
@@ -9,6 +9,23 @@ import WrappedExpression.weq
 
 /** Remove ValidIf and replace IsInvalid with a connection to zero */
 object RemoveValidIf extends Pass {
+
+  val UIntZero = Utils.zero
+  val SIntZero = SIntLiteral(BigInt(0), IntWidth(1))
+  val ClockZero = DoPrim(PrimOps.AsClock, Seq(UIntZero), Seq.empty, ClockType)
+  val FixedZero = FixedLiteral(BigInt(0), IntWidth(1), IntWidth(0))
+
+  /** Returns an [[Expression]] equal to zero for a given [[GroundType]]
+    * @note Accepts [[Type]] but dyanmically expects [[GroundType]]
+    */
+  def getGroundZero(tpe: Type): Expression = tpe match {
+    case _: UIntType => UIntZero
+    case _: SIntType => SIntZero
+    case ClockType => ClockZero
+    case _: FixedType => FixedZero
+    case other => throwInternalError(s"Unexpected type $other")
+  }
+
   // Recursive. Removes ValidIfs
   private def onExp(e: Expression): Expression = {
     e map onExp match {
@@ -16,16 +33,6 @@ object RemoveValidIf extends Pass {
       case x => x
     }
   }
-  private val UIntZero = Utils.zero
-  private val SIntZero = SIntLiteral(BigInt(0), IntWidth(1))
-  private val ClockZero = DoPrim(PrimOps.AsClock, Seq(UIntZero), Seq.empty, UIntZero.tpe)
-
-  private def getGroundZero(tpe: Type): Expression = tpe match {
-    case _: UIntType => UIntZero
-    case _: SIntType => SIntZero
-    case ClockType => ClockZero
-    case other => throwInternalError()
-  }
 
   // Recursive. Replaces IsInvalid with connecting zero
   private def onStmt(s: Statement): Statement = s map onStmt map onExp match {
@@ -33,11 +40,6 @@ object RemoveValidIf extends Pass {
       case _: AnalogType => EmptyStmt
       case tpe => Connect(info, loc, getGroundZero(tpe))
     }
-    // Register connected to itself (since reset has been made explicit) is a register with no reset
-    // and no connections, connect it to zero (to be constant propped later)
-    case Connect(info, lref: WRef, rref: WRef) if weq(lref, rref) =>
-      // We can't have an Analog reg so just get a zero
-      Connect(info, lref, getGroundZero(lref.tpe))
     case other => other
   }
 
diff --git a/src/main/scala/firrtl/transforms/ConstantPropagation.scala b/src/main/scala/firrtl/transforms/ConstantPropagation.scala
index 8217a9bd..2def130c 100644
--- a/src/main/scala/firrtl/transforms/ConstantPropagation.scala
+++ b/src/main/scala/firrtl/transforms/ConstantPropagation.scala
@@ -378,6 +378,8 @@ class ConstantPropagation extends Transform {
             nodeMap(lname) = constPropExpression(nodeMap, instMap, constSubOutputs)(pad(fval, ltpe))
           case Mux(_, tval: Literal, fval: WRef, _) if weq(lref, fval) =>
             nodeMap(lname) = constPropExpression(nodeMap, instMap, constSubOutputs)(pad(tval, ltpe))
+          case WRef(`lname`, _,_,_) => // If a register is connected to itself, propagate zero
+            nodeMap(lname) = passes.RemoveValidIf.getGroundZero(ltpe)
           case _ =>
         }
         // Mark instance inputs connected to a constant
diff --git a/src/test/scala/firrtlTests/ConstantPropagationTests.scala b/src/test/scala/firrtlTests/ConstantPropagationTests.scala
index 079b4823..ca7daa17 100644
--- a/src/test/scala/firrtlTests/ConstantPropagationTests.scala
+++ b/src/test/scala/firrtlTests/ConstantPropagationTests.scala
@@ -971,4 +971,44 @@ class ConstantPropagationIntegrationSpec extends LowTransformSpec {
           |    z <= mux(c, a, b)""".stripMargin
     execute(input, check, Seq.empty)
   }
+
+  "Registers connected only to themselves" should "be replaced with zero" in {
+      val input =
+        """circuit Top :
+          |  module Top :
+          |    input clock : Clock
+          |    output a : UInt<8>
+          |    reg ra : UInt<8>, clock
+          |    ra <= ra
+          |    a <= ra
+          |""".stripMargin
+      val check =
+        """circuit Top :
+          |  module Top :
+          |    input clock : Clock
+          |    output a : UInt<8>
+          |    a <= UInt<8>(0)
+          |""".stripMargin
+    execute(input, check, Seq.empty)
+  }
+
+  "Registers connected only to themselves from constant propagation" should "be replaced with zero" in {
+      val input =
+        """circuit Top :
+          |  module Top :
+          |    input clock : Clock
+          |    output a : UInt<8>
+          |    reg ra : UInt<8>, clock
+          |    ra <= or(ra, UInt(0))
+          |    a <= ra
+          |""".stripMargin
+      val check =
+        """circuit Top :
+          |  module Top :
+          |    input clock : Clock
+          |    output a : UInt<8>
+          |    a <= UInt<8>(0)
+          |""".stripMargin
+    execute(input, check, Seq.empty)
+  }
 }