Merge pull request #735 from freechipsproject/fix-const-prop

Fix Bugs in Constant Propagation
chipsalliance · Jan 31, 2018 · 5702511 · 5702511
2 parents 47d2bb1 + c9d3e25
commit 5702511
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 8 deletions.
diff --git a/src/main/scala/firrtl/transforms/ConstantPropagation.scala b/src/main/scala/firrtl/transforms/ConstantPropagation.scala
@@ -367,7 +367,7 @@ class ConstantPropagation extends Transform {
         // Const prop registers that are fed only a constant or a mux between and constant and the
         // register itself
         // This requires that reset has been made explicit
-        case Connect(_, lref @ WRef(lname, ltpe, RegKind, _), expr) => expr match {
+        case Connect(_, lref @ WRef(lname, ltpe, RegKind, _), expr) if !dontTouches.contains(lname) => expr match {
           case lit: Literal =>
             nodeMap(lname) = constPropExpression(pad(lit, ltpe))
           case Mux(_, tval: WRef, fval: Literal, _) if weq(lref, tval) =>
@@ -407,8 +407,13 @@ class ConstantPropagation extends Transform {
       mod.module -> children.map(i => i.name -> i.module).toMap
     })
 
-    // Module name to number of instances
-    val instCount: Map[String, Int] = iGraph.getVertices.groupBy(_.module).mapValues(_.size)
+    // This is a *relative* instance count, ie. how many there are when you visit each Module once
+    // (even if it is instantiated multiple times)
+    val instCount: Map[String, Int] = iGraph.getEdgeMap.foldLeft(Map(c.main -> 1)) {
+      case (cs, (_, values)) => values.foldLeft(cs) {
+        case (counts, value) => counts.updated(value.module, counts.getOrElse(value.module, 0) + 1)
+      }
+    }
 
     // DiGraph using Module names as nodes, destination of edge is a parent Module
     val parentGraph: DiGraph[String] = iGraph.reverse.transformNodes(_.module)

diff --git a/src/test/scala/firrtlTests/ConstantPropagationTests.scala b/src/test/scala/firrtlTests/ConstantPropagationTests.scala
@@ -67,9 +67,9 @@ s"""circuit Top :
     out <= in
   module Child :
     output out : UInt<1>
-    inst b of Bottom
-    b.in <= UInt(1)
-    out <= b.out
+    inst b0 of Bottom
+    b0.in <= UInt(1)
+    out <= b0.out
   module Top :
     input x : UInt<1>
     output z : UInt<1>
@@ -91,8 +91,8 @@ s"""circuit Top :
     out <= UInt(1)
   module Child :
     output out : UInt<1>
-    inst b of Bottom
-    b.in <= UInt(1)
+    inst b0 of Bottom
+    b0.in <= UInt(1)
     out <= UInt(1)
   module Top :
     input x : UInt<1>
@@ -712,6 +712,21 @@ class ConstantPropagationIntegrationSpec extends LowTransformSpec {
     execute(input, check, Seq(dontTouch("Top.z")))
   }
 
+  "ConstProp" should "NOT optimize across dontTouch on registers" in {
+      val input =
+        """circuit Top :
+          |  module Top :
+          |    input clk : Clock
+          |    input reset : UInt<1>
+          |    output y : UInt<1>
+          |    reg z : UInt<1>, clk
+          |    y <= z
+          |    z <= mux(reset, UInt<1>("h0"), z)""".stripMargin
+      val check = input
+    execute(input, check, Seq(dontTouch("Top.z")))
+  }
+
+
   it should "NOT optimize across dontTouch on wires" in {
       val input =
         """circuit Top :